Apply various fixes based on PR feedback

5 files changed, 79 insertions, 20 deletions

diff --git a/packages/contracts/src/utils/increase_time.ts b/packages/contracts/src/utils/increase_time.ts
index 5336a180d..4565d8dbc 100644
--- a/packages/contracts/src/utils/increase_time.ts
+++ b/packages/contracts/src/utils/increase_time.ts
@@ -8,7 +8,7 @@ let firstAccount: string | undefined;
 /**
  * Increases time by the given number of seconds and then mines a block so that
  * the current block timestamp has the offset applied.
- * @param seconds the Promise which is expected to reject
+ * @param seconds the number of seconds by which to incrase the time offset.
  * @returns a new Promise which will resolve with the new total time offset or
  * reject if the time could not be increased.
  */
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index 803a07b57..866e9e3a5 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -416,7 +416,9 @@ describe('Exchange core', () => {
                 makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
                 takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(200), 18),
             });
-            return expectRevertOrAlwaysFailingTransactionAsync(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress));
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+            );
         });
 
         it('should throw if signature is invalid', async () => {
@@ -431,7 +433,9 @@ describe('Exchange core', () => {
             const invalidSigBuff = Buffer.concat([v, invalidR, invalidS, signatureType]);
             const invalidSigHex = `0x${invalidSigBuff.toString('hex')}`;
             signedOrder.signature = invalidSigHex;
-            return expectRevertOrAlwaysFailingTransactionAsync(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress));
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+            );
         });
 
         it('should throw if makerAssetAmount is 0', async () => {
@@ -439,7 +443,9 @@ describe('Exchange core', () => {
                 makerAssetAmount: new BigNumber(0),
             });
 
-            return expectRevertOrAlwaysFailingTransactionAsync(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress));
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+            );
         });
 
         it('should throw if takerAssetAmount is 0', async () => {
@@ -447,7 +453,9 @@ describe('Exchange core', () => {
                 takerAssetAmount: new BigNumber(0),
             });
 
-            return expectRevertOrAlwaysFailingTransactionAsync(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress));
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+            );
         });
 
         it('should throw if takerAssetFillAmount is 0', async () => {
@@ -465,14 +473,18 @@ describe('Exchange core', () => {
                 makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18),
             });
 
-            return expectRevertOrAlwaysFailingTransactionAsync(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress));
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+            );
         });
 
         it('should throw if taker erc20Balances are too low to fill order', async () => {
             signedOrder = orderFactory.newSignedOrder({
                 takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18),
             });
-            return expectRevertOrAlwaysFailingTransactionAsync(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress));
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+            );
         });
 
         it('should throw if maker allowances are too low to fill order', async () => {
@@ -482,7 +494,9 @@ describe('Exchange core', () => {
                 }),
                 constants.AWAIT_TRANSACTION_MINED_MS,
             );
-            return expectRevertOrAlwaysFailingTransactionAsync(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress));
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+            );
         });
 
         it('should throw if taker allowances are too low to fill order', async () => {
@@ -492,7 +506,9 @@ describe('Exchange core', () => {
                 }),
                 constants.AWAIT_TRANSACTION_MINED_MS,
             );
-            return expectRevertOrAlwaysFailingTransactionAsync(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress));
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+            );
         });
 
         it('should throw if an order is expired', async () => {
@@ -520,7 +536,9 @@ describe('Exchange core', () => {
         });
 
         it('should throw if not sent by maker', async () => {
-            return expectRevertOrAlwaysFailingTransactionAsync(exchangeWrapper.cancelOrderAsync(signedOrder, takerAddress));
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                exchangeWrapper.cancelOrderAsync(signedOrder, takerAddress),
+            );
         });
 
         it('should throw if makerAssetAmount is 0', async () => {
@@ -528,7 +546,9 @@ describe('Exchange core', () => {
                 makerAssetAmount: new BigNumber(0),
             });
 
-            return expectRevertOrAlwaysFailingTransactionAsync(exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress));
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress),
+            );
         });
 
         it('should throw if takerAssetAmount is 0', async () => {
@@ -536,7 +556,9 @@ describe('Exchange core', () => {
                 takerAssetAmount: new BigNumber(0),
             });
 
-            return expectRevertOrAlwaysFailingTransactionAsync(exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress));
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress),
+            );
         });
 
         it('should be able to cancel a full order', async () => {
@@ -648,6 +670,9 @@ describe('Exchange core', () => {
                 }),
             ];
             await exchangeWrapper.batchFillOrdersNoThrowAsync(signedOrders, takerAddress, {
+                // HACK(albrow): We need to hardcode the gas estimate here because
+                // the Geth gas estimator doesn't work with the way we use
+                // delegatecall and swallow errors.
                 gas: 490000,
             });
 
diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts
index c47f3a4e4..8ef966be8 100644
--- a/packages/contracts/test/exchange/wrapper.ts
+++ b/packages/contracts/test/exchange/wrapper.ts
@@ -201,6 +201,9 @@ describe('Exchange wrappers', () => {
 
             await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress, {
                 takerAssetFillAmount,
+                // HACK(albrow): We need to hardcode the gas estimate here because
+                // the Geth gas estimator doesn't work with the way we use
+                // delegatecall and swallow errors.
                 gas: 250000,
             });
 
@@ -365,6 +368,9 @@ describe('Exchange wrappers', () => {
             const takerAssetFillAmount = signedOrder.takerAssetAmount;
             await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress, {
                 takerAssetFillAmount,
+                // HACK(albrow): We need to hardcode the gas estimate here because
+                // the Geth gas estimator doesn't work with the way we use
+                // delegatecall and swallow errors.
                 gas: 270000,
             });
             // Verify post-conditions
@@ -541,6 +547,9 @@ describe('Exchange wrappers', () => {
 
                 await exchangeWrapper.batchFillOrdersNoThrowAsync(signedOrders, takerAddress, {
                     takerAssetFillAmounts,
+                    // HACK(albrow): We need to hardcode the gas estimate here because
+                    // the Geth gas estimator doesn't work with the way we use
+                    // delegatecall and swallow errors.
                     gas: 600000,
                 });
 
@@ -598,6 +607,9 @@ describe('Exchange wrappers', () => {
                 const newOrders = [invalidOrder, ...validOrders];
                 await exchangeWrapper.batchFillOrdersNoThrowAsync(newOrders, takerAddress, {
                     takerAssetFillAmounts,
+                    // HACK(albrow): We need to hardcode the gas estimate here because
+                    // the Geth gas estimator doesn't work with the way we use
+                    // delegatecall and swallow errors.
                     gas: 450000,
                 });
 
@@ -761,6 +773,9 @@ describe('Exchange wrappers', () => {
                 });
                 await exchangeWrapper.marketSellOrdersNoThrowAsync(signedOrders, takerAddress, {
                     takerAssetFillAmount,
+                    // HACK(albrow): We need to hardcode the gas estimate here because
+                    // the Geth gas estimator doesn't work with the way we use
+                    // delegatecall and swallow errors.
                     gas: 600000,
                 });
 
@@ -940,6 +955,9 @@ describe('Exchange wrappers', () => {
                 });
                 await exchangeWrapper.marketSellOrdersNoThrowAsync(signedOrders, takerAddress, {
                     takerAssetFillAmount,
+                    // HACK(albrow): We need to hardcode the gas estimate here because
+                    // the Geth gas estimator doesn't work with the way we use
+                    // delegatecall and swallow errors.
                     gas: 600000,
                 });
 
diff --git a/packages/devnet/run.sh b/packages/devnet/run.sh
index 2c6d5fad1..85d223c61 100755
--- a/packages/devnet/run.sh
+++ b/packages/devnet/run.sh
@@ -4,9 +4,23 @@ set -e
 mkdir -p /var/log
 
 # Start Geth in background and redirect output to log file
-/geth --verbosity 5 --datadir node0/ --syncmode 'full' --nat none --nodiscover --port 30310 --txpool.journal '' \
-    --rpc --rpcaddr '0.0.0.0' --rpcport 8501 --rpcapi 'personal,db,eth,net,web3,txpool,miner,debug' \
-    --networkid 50 --gasprice '2000000000' --targetgaslimit '0x4c4b400000' --mine --etherbase '0xe8816898d851d5b61b7f950627d04d794c07ca37' \
+/geth \
+    --verbosity 5 \
+    --datadir node0/ \
+    --syncmode 'full' \
+    --nat none \
+    --nodiscover \
+    --port 30310 \
+    --txpool.journal '' \
+    --rpc \
+    --rpcaddr '0.0.0.0' \
+    --rpcport 8501 \
+    --rpcapi 'personal,db,eth,net,web3,txpool,miner,debug' \
+    --networkid 50 \
+    --gasprice '2000000000' \
+    --targetgaslimit '0x4c4b400000' \
+    --mine \
+    --etherbase '0xe8816898d851d5b61b7f950627d04d794c07ca37' \
     --unlock '0xe8816898d851d5b61b7f950627d04d794c07ca37,0x5409ed021d9299bf6814279a6a1411a7e866a631,0x6ecbe1db9ef729cbe972c83fb886247691fb6beb,0xe36ea790bc9d7ab70c55260c66d52b1eca985f84,0xe834ec434daba538cd1b9fe1582052b880bd7e63,0x78dc5d2d739606d31509c31d654056a45185ecb6,0xa8dda8d7f5310e4a9e24f8eba77e091ac264f872,0x06cef8e666768cc40cc78cf93d9611019ddcb628,0x4404ac8bd8f9618d27ad2f1485aa1b2cfd82482d,0x7457d5e02197480db681d3fdf256c7aca21bdc12,0x91c987bf62d25945db517bdaa840a6c661374402' \
     --password=node0/password.txt \
     > /var/log/geth &
@@ -15,10 +29,10 @@ mkdir -p /var/log
 sleep 10
 
 # Send some transactions.
-# Note(albrow): 🐉 We have to do this so that debug.setHead works correctly.
+# HACK(albrow): 🐉 We have to do this so that debug.setHead works correctly.
 # (Geth does not seem to like debug.setHead(0), so by sending some transactions
 # we increase the current block number beyond 0). Additionally, some tests seem
-# to break when there are fewuer than 3 blocks in the chain. (We have no idea
+# to break when there are fewer than 3 blocks in the chain. (We have no idea
 # why, but it was consistently reproducible).
 /geth --datadir node0/ attach --exec 'eth.sendTransaction({"from": "0x5409ED021D9299bf6814279A6A1411A7e866A631", "to": "0x84bd1cfa409cb0bb9b23b8b1a33515b4ac00a0af", "value": "0x1"})'
 /geth --datadir node0/ attach --exec 'eth.sendTransaction({"from": "0x5409ED021D9299bf6814279A6A1411A7e866A631", "to": "0x84bd1cfa409cb0bb9b23b8b1a33515b4ac00a0af", "value": "0x1"})'
diff --git a/packages/web3-wrapper/src/web3_wrapper.ts b/packages/web3-wrapper/src/web3_wrapper.ts
index 6c9fa980e..559bf3ea9 100644
--- a/packages/web3-wrapper/src/web3_wrapper.ts
+++ b/packages/web3-wrapper/src/web3_wrapper.ts
@@ -422,9 +422,11 @@ export class Web3Wrapper {
         return receipt;
     }
     /**
-     * Sets the current head of the local chain by block number. Note, this is a
-     * destructive action and may severely damage your chain. Use with extreme
-     * caution.
+     * Calls the 'debug_setHead' JSON RPC method, which sets the current head of
+     * the local chain by block number. Note, this is a destructive action and
+     * may severely damage your chain. Use with extreme caution. As of now, this
+     * is only supported by Geth. It sill throw if the 'debug_setHead' method is
+     * not supported.
      * @param  blockNumber The block number to reset to.
      */
     public async setHeadAsync(blockNumber: number): Promise<void> {