From dd8727d3aebf1f4b552f8bad921b92107ad22936 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Wed, 6 Jun 2018 11:43:07 -0700 Subject: Apply various fixes based on PR feedback --- packages/contracts/src/utils/increase_time.ts | 2 +- packages/contracts/test/exchange/core.ts | 47 ++++++++++++++++++++------- packages/contracts/test/exchange/wrapper.ts | 18 ++++++++++ 3 files changed, 55 insertions(+), 12 deletions(-) (limited to 'packages/contracts') 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, }); -- cgit