Merge pull request #385 from 0xProject/feature/0x_config_validation

Better validate ZeroExConfig on private networks

10 files changed, 92 insertions, 31 deletions

diff --git a/packages/0x.js/CHANGELOG.md b/packages/0x.js/CHANGELOG.md
index 0af474c74..57cd381ee 100644
--- a/packages/0x.js/CHANGELOG.md
+++ b/packages/0x.js/CHANGELOG.md
@@ -1,5 +1,9 @@
 # CHANGELOG
 
+## v0.33.0 - _TBD, 2018_
+
+    * Improve validation to force passing contract addresses on private networks (#385)
+
 ## v0.32.2 - _February 9, 2018_
 
     * Fix publishing issue where .npmignore was not properly excluding undesired content (#389)
diff --git a/packages/0x.js/src/0x.ts b/packages/0x.js/src/0x.ts
index 6cfa65cc2..c578478d8 100644
--- a/packages/0x.js/src/0x.ts
+++ b/packages/0x.js/src/0x.ts
@@ -13,6 +13,8 @@ import { TokenTransferProxyWrapper } from './contract_wrappers/token_transfer_pr
 import { TokenWrapper } from './contract_wrappers/token_wrapper';
 import { OrderStateWatcher } from './order_watcher/order_state_watcher';
 import { zeroExConfigSchema } from './schemas/zero_ex_config_schema';
+import { zeroExPrivateNetworkConfigSchema } from './schemas/zero_ex_private_network_config_schema';
+import { zeroExPublicNetworkConfigSchema } from './schemas/zero_ex_public_network_config_schema';
 import { ECSignature, Order, SignedOrder, Web3Provider, ZeroExConfig, ZeroExError } from './types';
 import { assert } from './utils/assert';
 import { constants } from './utils/constants';
@@ -163,7 +165,10 @@ export class ZeroEx {
      */
     constructor(provider: Web3Provider, config: ZeroExConfig) {
         assert.isWeb3Provider('provider', provider);
-        assert.doesConformToSchema('config', config, zeroExConfigSchema);
+        assert.doesConformToSchema('config', config, zeroExConfigSchema, [
+            zeroExPrivateNetworkConfigSchema,
+            zeroExPublicNetworkConfigSchema,
+        ]);
         const artifactJSONs = _.values(artifacts);
         const abiArrays = _.map(artifactJSONs, artifact => artifact.abi);
         this._abiDecoder = new AbiDecoder(abiArrays);
diff --git a/packages/0x.js/src/schemas/zero_ex_config_schema.ts b/packages/0x.js/src/schemas/zero_ex_config_schema.ts
index 546b1c2d0..a9c3c64fc 100644
--- a/packages/0x.js/src/schemas/zero_ex_config_schema.ts
+++ b/packages/0x.js/src/schemas/zero_ex_config_schema.ts
@@ -1,27 +1,5 @@
 export const zeroExConfigSchema = {
     id: '/ZeroExConfig',
-    properties: {
-        networkId: {
-            type: 'number',
-            minimum: 0,
-        },
-        gasPrice: { $ref: '/Number' },
-        exchangeContractAddress: { $ref: '/Address' },
-        tokenRegistryContractAddress: { $ref: '/Address' },
-        orderWatcherConfig: {
-            type: 'object',
-            properties: {
-                pollingIntervalMs: {
-                    type: 'number',
-                    minimum: 0,
-                },
-                numConfirmations: {
-                    type: 'number',
-                    minimum: 0,
-                },
-            },
-        },
-    },
+    oneOf: [{ $ref: '/ZeroExPrivateNetworkConfig' }, { $ref: '/ZeroExPublicNetworkConfig' }],
     type: 'object',
-    required: ['networkId'],
 };
diff --git a/packages/0x.js/src/schemas/zero_ex_private_network_config_schema.ts b/packages/0x.js/src/schemas/zero_ex_private_network_config_schema.ts
new file mode 100644
index 000000000..f7f649a6d
--- /dev/null
+++ b/packages/0x.js/src/schemas/zero_ex_private_network_config_schema.ts
@@ -0,0 +1,35 @@
+export const zeroExPrivateNetworkConfigSchema = {
+    id: '/ZeroExPrivateNetworkConfig',
+    properties: {
+        networkId: {
+            type: 'number',
+            minimum: 1,
+        },
+        gasPrice: { $ref: '/Number' },
+        zrxContractAddress: { $ref: '/Address' },
+        exchangeContractAddress: { $ref: '/Address' },
+        tokenRegistryContractAddress: { $ref: '/Address' },
+        tokenTransferProxyContractAddress: { $ref: '/Address' },
+        orderWatcherConfig: {
+            type: 'object',
+            properties: {
+                pollingIntervalMs: {
+                    type: 'number',
+                    minimum: 0,
+                },
+                numConfirmations: {
+                    type: 'number',
+                    minimum: 0,
+                },
+            },
+        },
+    },
+    type: 'object',
+    required: [
+        'networkId',
+        'zrxContractAddress',
+        'exchangeContractAddress',
+        'tokenRegistryContractAddress',
+        'tokenTransferProxyContractAddress',
+    ],
+};
diff --git a/packages/0x.js/src/schemas/zero_ex_public_network_config_schema.ts b/packages/0x.js/src/schemas/zero_ex_public_network_config_schema.ts
new file mode 100644
index 000000000..9da31481a
--- /dev/null
+++ b/packages/0x.js/src/schemas/zero_ex_public_network_config_schema.ts
@@ -0,0 +1,29 @@
+export const zeroExPublicNetworkConfigSchema = {
+    id: '/ZeroExPublicNetworkConfig',
+    properties: {
+        networkId: {
+            type: 'number',
+            enum: [1, 3, 4, 42, 50],
+        },
+        gasPrice: { $ref: '/Number' },
+        zrxContractAddress: { $ref: '/Address' },
+        exchangeContractAddress: { $ref: '/Address' },
+        tokenRegistryContractAddress: { $ref: '/Address' },
+        tokenTransferProxyContractAddress: { $ref: '/Address' },
+        orderWatcherConfig: {
+            type: 'object',
+            properties: {
+                pollingIntervalMs: {
+                    type: 'number',
+                    minimum: 0,
+                },
+                numConfirmations: {
+                    type: 'number',
+                    minimum: 0,
+                },
+            },
+        },
+    },
+    type: 'object',
+    required: ['networkId'],
+};
diff --git a/packages/0x.js/src/types.ts b/packages/0x.js/src/types.ts
index ab97f7775..0a3037258 100644
--- a/packages/0x.js/src/types.ts
+++ b/packages/0x.js/src/types.ts
@@ -196,7 +196,7 @@ export interface OrderStateWatcherConfig {
 }
 
 /*
- * networkId: The id of the underlying ethereum network your provider is connected to. (1-mainnet, 42-kovan, 50-testrpc)
+ * networkId: The id of the underlying ethereum network your provider is connected to. (1-mainnet, 3-ropsten, 4-rinkeby, 42-kovan, 50-testrpc)
  * gasPrice: Gas price to use with every transaction
  * exchangeContractAddress: The address of an exchange contract to use
  * zrxContractAddress: The address of the ZRX contract to use
diff --git a/packages/0x.js/test/ether_token_wrapper_test.ts b/packages/0x.js/test/ether_token_wrapper_test.ts
index da49ec467..72086dff0 100644
--- a/packages/0x.js/test/ether_token_wrapper_test.ts
+++ b/packages/0x.js/test/ether_token_wrapper_test.ts
@@ -75,11 +75,14 @@ describe('EtherTokenWrapper', () => {
             const contractAddressIfExists = zeroEx.etherToken.getContractAddressIfExists();
             expect(contractAddressIfExists).to.not.be.undefined();
         });
-        it('should return undefined if connected to an unknown network', () => {
+        it('should throw if connected to a private network and contract addresses are not specified', () => {
             const UNKNOWN_NETWORK_NETWORK_ID = 10;
-            const unknownNetworkZeroEx = new ZeroEx(web3.currentProvider, { networkId: UNKNOWN_NETWORK_NETWORK_ID });
-            const contractAddressIfExists = unknownNetworkZeroEx.etherToken.getContractAddressIfExists();
-            expect(contractAddressIfExists).to.be.undefined();
+            expect(
+                () =>
+                    new ZeroEx(web3.currentProvider, {
+                        networkId: UNKNOWN_NETWORK_NETWORK_ID,
+                    } as any),
+            ).to.throw();
         });
     });
     describe('#depositAsync', () => {
diff --git a/packages/0x.js/test/expiration_watcher_test.ts b/packages/0x.js/test/expiration_watcher_test.ts
index b49dee8e5..7f79e3802 100644
--- a/packages/0x.js/test/expiration_watcher_test.ts
+++ b/packages/0x.js/test/expiration_watcher_test.ts
@@ -9,10 +9,10 @@ import * as Web3 from 'web3';
 import { ZeroEx } from '../src/0x';
 import { ExpirationWatcher } from '../src/order_watcher/expiration_watcher';
 import { DoneCallback, Token } from '../src/types';
-import { constants } from '../src/utils/constants';
 import { utils } from '../src/utils/utils';
 
 import { chaiSetup } from './utils/chai_setup';
+import { constants } from './utils/constants';
 import { FillScenarios } from './utils/fill_scenarios';
 import { reportNoErrorCallbackErrors } from './utils/report_callback_errors';
 import { TokenUtils } from './utils/token_utils';
diff --git a/packages/assert/CHANGELOG.md b/packages/assert/CHANGELOG.md
index 23c2c5e56..f512f7b10 100644
--- a/packages/assert/CHANGELOG.md
+++ b/packages/assert/CHANGELOG.md
@@ -1,5 +1,9 @@
 # CHANGELOG
 
+## v0.1.0 - _TBD, 2018_
+
+    * Add an optional parameter `subSchemas` to `doesConformToSchema` method (#385)
+
 ## v0.0.18 - _February 9, 2017_
 
     * Fix publishing issue where .npmignore was not properly excluding undesired content (#389)
diff --git a/packages/assert/src/index.ts b/packages/assert/src/index.ts
index 7ad574ec7..4d090e493 100644
--- a/packages/assert/src/index.ts
+++ b/packages/assert/src/index.ts
@@ -66,8 +66,11 @@ export const assert = {
         const isWeb3Provider = _.isFunction(value.send) || _.isFunction(value.sendAsync);
         this.assert(isWeb3Provider, this.typeAssertionMessage(variableName, 'Web3.Provider', value));
     },
-    doesConformToSchema(variableName: string, value: any, schema: Schema): void {
+    doesConformToSchema(variableName: string, value: any, schema: Schema, subSchemas?: Schema[]): void {
         const schemaValidator = new SchemaValidator();
+        if (!_.isUndefined(subSchemas)) {
+            _.map(subSchemas, schemaValidator.addSchema.bind(schemaValidator));
+        }
         const validationResult = schemaValidator.validate(value, schema);
         const hasValidationErrors = validationResult.errors.length > 0;
         const msg = `Expected ${variableName} to conform to schema ${schema.id}