Improve error handling for unknown network ids in contract-wrappers

9 files changed, 31 insertions, 18 deletions

diff --git a/packages/contract-wrappers/src/contract_wrappers.ts b/packages/contract-wrappers/src/contract_wrappers.ts
index 34eb1f303..8cb322912 100644
--- a/packages/contract-wrappers/src/contract_wrappers.ts
+++ b/packages/contract-wrappers/src/contract_wrappers.ts
@@ -27,6 +27,8 @@ import { contractWrappersPublicNetworkConfigSchema } from './schemas/contract_wr
 import { ContractWrappersConfig } from './types';
 import { assert } from './utils/assert';
 import { constants } from './utils/constants';
+import { _getDefaultContractAddresses } from './utils/contract_addresses';
+
 /**
  * The ContractWrappers class contains smart contract wrappers helpful when building on 0x protocol.
  */
@@ -102,7 +104,7 @@ export class ContractWrappers {
             ? constants.DEFAULT_BLOCK_POLLING_INTERVAL
             : config.blockPollingIntervalMs;
         const contractAddresses = _.isUndefined(config.contractAddresses)
-            ? getContractAddressesForNetwork(config.networkId)
+            ? _getDefaultContractAddresses(config.networkId)
             : config.contractAddresses;
         this.erc20Proxy = new ERC20ProxyWrapper(this._web3Wrapper, config.networkId, contractAddresses.erc20Proxy);
         this.erc721Proxy = new ERC721ProxyWrapper(this._web3Wrapper, config.networkId, contractAddresses.erc721Proxy);
diff --git a/packages/contract-wrappers/src/contract_wrappers/contract_wrapper.ts b/packages/contract-wrappers/src/contract_wrappers/contract_wrapper.ts
index 3d37e446c..72f5aa629 100644
--- a/packages/contract-wrappers/src/contract_wrappers/contract_wrapper.ts
+++ b/packages/contract-wrappers/src/contract_wrappers/contract_wrapper.ts
@@ -1,4 +1,4 @@
-import { ContractAddresses, getContractAddressesForNetwork } from '@0xproject/contract-addresses';
+import { ContractAddresses, getContractAddressesForNetwork, NetworkId } from '@0xproject/contract-addresses';
 import { AbiDecoder, intervalUtils, logUtils } from '@0xproject/utils';
 import { marshaller, Web3Wrapper } from '@0xproject/web3-wrapper';
 import {
@@ -112,10 +112,6 @@ export abstract class ContractWrapper {
         const logWithDecodedArgs = abiDecoder.tryToDecodeLogOrNoop(log);
         return logWithDecodedArgs;
     }
-    protected _getDefaultContractAddresses(): ContractAddresses {
-        // TODO(albrow): Figure out better error handling here.
-        return getContractAddressesForNetwork(this._networkId);
-    }
     private _onLogStateChanged<ArgsType extends ContractEventArgs>(isRemoved: boolean, rawLog: RawLogEntry): void {
         const log: LogEntry = marshaller.unmarshalLog(rawLog);
         _.forEach(this._filters, (filter: FilterObject, filterToken: string) => {
diff --git a/packages/contract-wrappers/src/contract_wrappers/erc20_proxy_wrapper.ts b/packages/contract-wrappers/src/contract_wrappers/erc20_proxy_wrapper.ts
index 459019877..369e27e19 100644
--- a/packages/contract-wrappers/src/contract_wrappers/erc20_proxy_wrapper.ts
+++ b/packages/contract-wrappers/src/contract_wrappers/erc20_proxy_wrapper.ts
@@ -6,6 +6,7 @@ import { ContractAbi } from 'ethereum-types';
 import * as _ from 'lodash';
 
 import { assert } from '../utils/assert';
+import { _getDefaultContractAddresses } from '../utils/contract_addresses';
 
 import { ContractWrapper } from './contract_wrapper';
 
@@ -25,7 +26,7 @@ export class ERC20ProxyWrapper extends ContractWrapper {
      */
     constructor(web3Wrapper: Web3Wrapper, networkId: number, address?: string) {
         super(web3Wrapper, networkId);
-        this.address = _.isUndefined(address) ? this._getDefaultContractAddresses().erc20Proxy : address;
+        this.address = _.isUndefined(address) ? _getDefaultContractAddresses(networkId).erc20Proxy : address;
     }
     /**
      * Get the 4 bytes ID of this asset proxy
diff --git a/packages/contract-wrappers/src/contract_wrappers/erc721_proxy_wrapper.ts b/packages/contract-wrappers/src/contract_wrappers/erc721_proxy_wrapper.ts
index 9cf56dec1..0c61d939f 100644
--- a/packages/contract-wrappers/src/contract_wrappers/erc721_proxy_wrapper.ts
+++ b/packages/contract-wrappers/src/contract_wrappers/erc721_proxy_wrapper.ts
@@ -6,6 +6,7 @@ import { ContractAbi } from 'ethereum-types';
 import * as _ from 'lodash';
 
 import { assert } from '../utils/assert';
+import { _getDefaultContractAddresses } from '../utils/contract_addresses';
 
 import { ContractWrapper } from './contract_wrapper';
 
@@ -25,7 +26,7 @@ export class ERC721ProxyWrapper extends ContractWrapper {
      */
     constructor(web3Wrapper: Web3Wrapper, networkId: number, address?: string) {
         super(web3Wrapper, networkId);
-        this.address = _.isUndefined(address) ? this._getDefaultContractAddresses().erc721Proxy : address;
+        this.address = _.isUndefined(address) ? _getDefaultContractAddresses(networkId).erc721Proxy : address;
     }
     /**
      * Get the 4 bytes ID of this asset proxy
diff --git a/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts b/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
index 67c0351e2..a9809e0e7 100644
--- a/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
+++ b/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
@@ -29,6 +29,7 @@ import {
     ValidateOrderFillableOpts,
 } from '../types';
 import { assert } from '../utils/assert';
+import { _getDefaultContractAddresses } from '../utils/contract_addresses';
 import { decorators } from '../utils/decorators';
 import { TransactionEncoder } from '../utils/transaction_encoder';
 
@@ -72,9 +73,9 @@ export class ExchangeWrapper extends ContractWrapper {
         super(web3Wrapper, networkId, blockPollingIntervalMs);
         this._erc20TokenWrapper = erc20TokenWrapper;
         this._erc721TokenWrapper = erc721TokenWrapper;
-        this.address = _.isUndefined(address) ? this._getDefaultContractAddresses().exchange : address;
+        this.address = _.isUndefined(address) ? _getDefaultContractAddresses(networkId).exchange : address;
         this.zrxTokenAddress = _.isUndefined(zrxTokenAddress)
-            ? this._getDefaultContractAddresses().zrxToken
+            ? _getDefaultContractAddresses(networkId).zrxToken
             : zrxTokenAddress;
     }
     /**
diff --git a/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts b/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts
index fe4d0cbda..fd7a9a362 100644
--- a/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts
+++ b/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts
@@ -7,12 +7,12 @@ import { Web3Wrapper } from '@0xproject/web3-wrapper';
 import { ContractAbi } from 'ethereum-types';
 import * as _ from 'lodash';
 
-import { orderTxOptsSchema } from '../schemas/order_tx_opts_schema';
 import { txOptsSchema } from '../schemas/tx_opts_schema';
 import { OrderTransactionOpts } from '../types';
 import { assert } from '../utils/assert';
 import { calldataOptimizationUtils } from '../utils/calldata_optimization_utils';
 import { constants } from '../utils/constants';
+import { _getDefaultContractAddresses } from '../utils/contract_addresses';
 import { decorators } from '../utils/decorators';
 import { utils } from '../utils/utils';
 
@@ -49,12 +49,12 @@ export class ForwarderWrapper extends ContractWrapper {
         etherTokenAddress?: string,
     ) {
         super(web3Wrapper, networkId);
-        this.address = _.isUndefined(address) ? this._getDefaultContractAddresses().exchange : address;
+        this.address = _.isUndefined(address) ? _getDefaultContractAddresses(networkId).exchange : address;
         this.zrxTokenAddress = _.isUndefined(zrxTokenAddress)
-            ? this._getDefaultContractAddresses().zrxToken
+            ? _getDefaultContractAddresses(networkId).zrxToken
             : zrxTokenAddress;
         this.etherTokenAddress = _.isUndefined(etherTokenAddress)
-            ? this._getDefaultContractAddresses().etherToken
+            ? _getDefaultContractAddresses(networkId).etherToken
             : etherTokenAddress;
     }
     /**
diff --git a/packages/contract-wrappers/src/contract_wrappers/order_validator_wrapper.ts b/packages/contract-wrappers/src/contract_wrappers/order_validator_wrapper.ts
index 16179447b..4b6196eff 100644
--- a/packages/contract-wrappers/src/contract_wrappers/order_validator_wrapper.ts
+++ b/packages/contract-wrappers/src/contract_wrappers/order_validator_wrapper.ts
@@ -9,6 +9,7 @@ import * as _ from 'lodash';
 
 import { BalanceAndAllowance, OrderAndTraderInfo, TraderInfo } from '../types';
 import { assert } from '../utils/assert';
+import { _getDefaultContractAddresses } from '../utils/contract_addresses';
 
 import { ContractWrapper } from './contract_wrapper';
 
@@ -28,7 +29,7 @@ export class OrderValidatorWrapper extends ContractWrapper {
      */
     constructor(web3Wrapper: Web3Wrapper, networkId: number, address?: string) {
         super(web3Wrapper, networkId);
-        this.address = _.isUndefined(address) ? this._getDefaultContractAddresses().exchange : address;
+        this.address = _.isUndefined(address) ? _getDefaultContractAddresses(networkId).exchange : address;
     }
     /**
      * Get an object conforming to OrderAndTraderInfo containing on-chain information of the provided order and address
diff --git a/packages/contract-wrappers/src/utils/contract_addresses.ts b/packages/contract-wrappers/src/utils/contract_addresses.ts
new file mode 100644
index 000000000..c32907c24
--- /dev/null
+++ b/packages/contract-wrappers/src/utils/contract_addresses.ts
@@ -0,0 +1,13 @@
+import { ContractAddresses, getContractAddressesForNetwork, NetworkId } from '@0xproject/contract-addresses';
+import * as _ from 'lodash';
+
+// Returns the default contract addresses for the given networkId or throws with
+// a context-specific error message if the networkId is not recognized.
+export function _getDefaultContractAddresses(networkId: number): ContractAddresses {
+    if (!(networkId in NetworkId)) {
+        throw new Error(
+            `No default contract addresses found for the given network id (${networkId}). If you want to use ContractWrappers on this network, you must manually pass in the contract address(es) to the constructor.`,
+        );
+    }
+    return getContractAddressesForNetwork(networkId);
+}
diff --git a/packages/contract-wrappers/test/exchange_wrapper_test.ts b/packages/contract-wrappers/test/exchange_wrapper_test.ts
index 695574466..7e29c76a1 100644
--- a/packages/contract-wrappers/test/exchange_wrapper_test.ts
+++ b/packages/contract-wrappers/test/exchange_wrapper_test.ts
@@ -28,13 +28,11 @@ describe('ExchangeWrapper', () => {
     let exchangeContractAddress: string;
     let makerTokenAddress: string;
     let takerTokenAddress: string;
-    let coinbase: string;
     let makerAddress: string;
     let anotherMakerAddress: string;
     let takerAddress: string;
     let makerAssetData: string;
     let takerAssetData: string;
-    let feeRecipient: string;
     let txHash: string;
     const fillableAmount = new BigNumber(5);
     const takerTokenFillAmount = new BigNumber(5);
@@ -61,7 +59,7 @@ describe('ExchangeWrapper', () => {
             contractWrappers.erc20Proxy.address,
             contractWrappers.erc721Proxy.address,
         );
-        [coinbase, makerAddress, takerAddress, feeRecipient, anotherMakerAddress] = userAddresses;
+        [, makerAddress, takerAddress, , anotherMakerAddress] = userAddresses;
         [makerTokenAddress, takerTokenAddress] = tokenUtils.getDummyERC20TokenAddresses();
         [makerAssetData, takerAssetData] = [
             assetDataUtils.encodeERC20AssetData(makerTokenAddress),