From 577a8dd005715ba0fd22a5118d99ccc87af0782c Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Mon, 4 Jun 2018 17:51:50 -0700 Subject: Fix some more test cases, especially those that call increaseTime --- packages/contracts/src/utils/increase_time.ts | 26 +++++++ packages/contracts/test/asset_proxy_owner.ts | 19 +++-- packages/contracts/test/exchange/wrapper.ts | 83 ++++++++++++---------- .../contracts/test/multi_sig_with_time_lock.ts | 11 +-- 4 files changed, 84 insertions(+), 55 deletions(-) create mode 100644 packages/contracts/src/utils/increase_time.ts diff --git a/packages/contracts/src/utils/increase_time.ts b/packages/contracts/src/utils/increase_time.ts new file mode 100644 index 000000000..726a759f3 --- /dev/null +++ b/packages/contracts/src/utils/increase_time.ts @@ -0,0 +1,26 @@ +import * as _ from 'lodash'; + +import { constants } from './constants'; +import { web3Wrapper } from './web3_wrapper'; + +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. +export async function increaseTimeAndMineBlockAsync(seconds: number): Promise { + if (_.isUndefined(firstAccount)) { + const accounts = await web3Wrapper.getAvailableAddressesAsync(); + firstAccount = accounts[0]; + } + + const offset = await web3Wrapper.increaseTimeAsync(seconds); + // Note: we need to send a transaction after increasing time so + // that a block is actually mined. The contract looks at the + // last mined block for the timestamp. + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ from: firstAccount, to: firstAccount, value: 0 }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + + return offset; +} diff --git a/packages/contracts/test/asset_proxy_owner.ts b/packages/contracts/test/asset_proxy_owner.ts index 318fc65e5..c7d4e08ed 100644 --- a/packages/contracts/test/asset_proxy_owner.ts +++ b/packages/contracts/test/asset_proxy_owner.ts @@ -18,6 +18,7 @@ import { artifacts } from '../src/utils/artifacts'; import { expectRevertOrAlwaysFailingTransaction } from '../src/utils/assertions'; import { chaiSetup } from '../src/utils/chai_setup'; import { constants } from '../src/utils/constants'; +import { increaseTimeAndMineBlockAsync } from '../src/utils/increase_time'; import { MultiSigWrapper } from '../src/utils/multi_sig_wrapper'; import { provider, txDefaults, web3Wrapper } from '../src/utils/web3_wrapper'; @@ -29,7 +30,7 @@ describe('AssetProxyOwner', () => { let owners: string[]; let authorized: string; const REQUIRED_APPROVALS = new BigNumber(2); - const SECONDS_TIME_LOCKED = new BigNumber(1000000); + const SECONDS_TIME_LOCKED = new BigNumber(1000); let erc20Proxy: MixinAuthorizableContract; let erc721Proxy: MixinAuthorizableContract; @@ -147,8 +148,7 @@ describe('AssetProxyOwner', () => { ); }); - // TODO(albrow): gas required exceeds allowance or always failing transaction - it.skip('should register an address if called by multisig after timelock', async () => { + it('should register an address if called by multisig after timelock', async () => { const addressToRegister = erc20Proxy.address; const isRegistered = true; const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( @@ -160,11 +160,12 @@ describe('AssetProxyOwner', () => { registerAssetProxyData, owners[0], ); + const log = submitTxRes.logs[0] as LogWithDecodedArgs; const txId = log.args.transactionId; const confirmTxRes = await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - await web3Wrapper.increaseTimeAsync(SECONDS_TIME_LOCKED.toNumber()); + await increaseTimeAndMineBlockAsync(SECONDS_TIME_LOCKED.toNumber()); const executeTxRes = await multiSigWrapper.executeTransactionAsync(txId, owners[0]); const registerLog = executeTxRes.logs[0] as LogWithDecodedArgs; @@ -175,8 +176,7 @@ describe('AssetProxyOwner', () => { expect(isAssetProxyRegistered).to.equal(isRegistered); }); - // TODO(albrow): gas required exceeds allowance or always failing transaction - it.skip('should fail if registering a null address', async () => { + it('should fail if registering a null address', async () => { const addressToRegister = constants.NULL_ADDRESS; const isRegistered = true; const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( @@ -192,7 +192,7 @@ describe('AssetProxyOwner', () => { const txId = log.args.transactionId; await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - await web3Wrapper.increaseTimeAsync(SECONDS_TIME_LOCKED.toNumber()); + await increaseTimeAndMineBlockAsync(SECONDS_TIME_LOCKED.toNumber()); const executeTxRes = await multiSigWrapper.executeTransactionAsync(txId, owners[0]); const failureLog = executeTxRes.logs[0] as LogWithDecodedArgs; @@ -203,8 +203,7 @@ describe('AssetProxyOwner', () => { }); }); - // TODO(albrow): gas required exceeds allowance or always failing transaction - describe.skip('executeRemoveAuthorizedAddress', () => { + describe('executeRemoveAuthorizedAddress', () => { before('authorize both proxies and register erc20 proxy', async () => { // Only register ERC20 proxy const addressToRegister = erc20Proxy.address; @@ -245,7 +244,7 @@ describe('AssetProxyOwner', () => { await multiSigWrapper.confirmTransactionAsync(erc20AddAuthorizedAddressTxId, owners[1]); await multiSigWrapper.confirmTransactionAsync(erc721AddAuthorizedAddressTxId, owners[1]); - await web3Wrapper.increaseTimeAsync(SECONDS_TIME_LOCKED.toNumber()); + await increaseTimeAndMineBlockAsync(SECONDS_TIME_LOCKED.toNumber()); await multiSigWrapper.executeTransactionAsync(registerAssetProxyTxId, owners[0]); await multiSigWrapper.executeTransactionAsync(erc20AddAuthorizedAddressTxId, owners[0]); await multiSigWrapper.executeTransactionAsync(erc721AddAuthorizedAddressTxId, owners[0]); diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts index 305ce7222..9df08ed86 100644 --- a/packages/contracts/test/exchange/wrapper.ts +++ b/packages/contracts/test/exchange/wrapper.ts @@ -62,7 +62,6 @@ describe('Exchange wrappers', () => { await blockchainLifecycle.revertAsync(); }); before(async () => { - console.log('before running'); const accounts = await web3Wrapper.getAvailableAddressesAsync(); const usedAddresses = ([owner, makerAddress, takerAddress, feeRecipientAddress] = accounts); @@ -116,13 +115,10 @@ describe('Exchange wrappers', () => { }; const privateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)]; orderFactory = new OrderFactory(privateKey, defaultOrderParams); - console.log('before finished running'); }); beforeEach(async () => { - console.log('beforeEach running'); await blockchainLifecycle.startAsync(); erc20Balances = await erc20Wrapper.getBalancesAsync(); - console.log('beforeEach finished'); }); afterEach(async () => { await blockchainLifecycle.revertAsync(); @@ -203,48 +199,64 @@ describe('Exchange wrappers', () => { // -10000000000000000000000 // +9950000000000000000000 + // + // We think this is failing due to a problem in the fillOrderNoThrow + // function in the smart contract. (There's a lot of assembly). it.skip('should transfer the correct amounts', async () => { const signedOrder = orderFactory.newSignedOrder({ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(200), 18), }); const takerAssetFillAmount = signedOrder.takerAssetAmount.div(2); + + console.log('maker balance: ', erc20Balances[makerAddress][defaultMakerAssetAddress].toString()); await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress, { takerAssetFillAmount, }); const newBalances = await erc20Wrapper.getBalancesAsync(); - const makerAssetFilledAmount = takerAssetFillAmount - .times(signedOrder.makerAssetAmount) - .dividedToIntegerBy(signedOrder.takerAssetAmount); - const makerFee = signedOrder.makerFee - .times(makerAssetFilledAmount) - .dividedToIntegerBy(signedOrder.makerAssetAmount); - const takerFee = signedOrder.takerFee - .times(makerAssetFilledAmount) - .dividedToIntegerBy(signedOrder.makerAssetAmount); - expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( - erc20Balances[makerAddress][defaultMakerAssetAddress].minus(makerAssetFilledAmount), - ); - expect(newBalances[makerAddress][defaultTakerAssetAddress]).to.be.bignumber.equal( - erc20Balances[makerAddress][defaultTakerAssetAddress].add(takerAssetFillAmount), - ); - expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( - erc20Balances[makerAddress][zrxToken.address].minus(makerFee), - ); - expect(newBalances[takerAddress][defaultTakerAssetAddress]).to.be.bignumber.equal( - erc20Balances[takerAddress][defaultTakerAssetAddress].minus(takerAssetFillAmount), - ); - expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( - erc20Balances[takerAddress][defaultMakerAssetAddress].add(makerAssetFilledAmount), - ); - expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( - erc20Balances[takerAddress][zrxToken.address].minus(takerFee), - ); - expect(newBalances[feeRecipientAddress][zrxToken.address]).to.be.bignumber.equal( - erc20Balances[feeRecipientAddress][zrxToken.address].add(makerFee.add(takerFee)), - ); + console.log('new maker balance: ', newBalances[makerAddress][defaultMakerAssetAddress].toString()); + + // const makerAssetFilledAmount = takerAssetFillAmount + // .times(signedOrder.makerAssetAmount) + // .dividedToIntegerBy(signedOrder.takerAssetAmount); + // const makerFee = signedOrder.makerFee + // .times(makerAssetFilledAmount) + // .dividedToIntegerBy(signedOrder.makerAssetAmount); + // const takerFee = signedOrder.takerFee + // .times(makerAssetFilledAmount) + // .dividedToIntegerBy(signedOrder.makerAssetAmount); + // console.log('makerAssetFilledAmount: ', makerAssetFilledAmount); + // console.log('makerFee: ', makerFee); + // console.log('takerFee: ', takerFee); + // expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + // erc20Balances[makerAddress][defaultMakerAssetAddress].minus(makerAssetFilledAmount), + // ); + // console.log(1); + // expect(newBalances[makerAddress][defaultTakerAssetAddress]).to.be.bignumber.equal( + // erc20Balances[makerAddress][defaultTakerAssetAddress].add(takerAssetFillAmount), + // ); + // console.log(2); + // expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( + // erc20Balances[makerAddress][zrxToken.address].minus(makerFee), + // ); + // console.log(3); + // expect(newBalances[takerAddress][defaultTakerAssetAddress]).to.be.bignumber.equal( + // erc20Balances[takerAddress][defaultTakerAssetAddress].minus(takerAssetFillAmount), + // ); + // console.log(4); + // expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + // erc20Balances[takerAddress][defaultMakerAssetAddress].add(makerAssetFilledAmount), + // ); + // console.log(5); + // expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( + // erc20Balances[takerAddress][zrxToken.address].minus(takerFee), + // ); + // console.log(6); + // expect(newBalances[feeRecipientAddress][zrxToken.address]).to.be.bignumber.equal( + // erc20Balances[feeRecipientAddress][zrxToken.address].add(makerFee.add(takerFee)), + // ); }); it('should not change erc20Balances if maker erc20Balances are too low to fill order', async () => { @@ -725,8 +737,7 @@ describe('Exchange wrappers', () => { ); }); - // TODO(albrow): failing similar to above - it.skip('should fill all signedOrders if cannot fill entire takerAssetFillAmount', async () => { + it('should fill all signedOrders if cannot fill entire takerAssetFillAmount', async () => { const takerAssetFillAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18); _.forEach(signedOrders, signedOrder => { erc20Balances[makerAddress][defaultMakerAssetAddress] = erc20Balances[makerAddress][ diff --git a/packages/contracts/test/multi_sig_with_time_lock.ts b/packages/contracts/test/multi_sig_with_time_lock.ts index 090ce91f5..b606f31ce 100644 --- a/packages/contracts/test/multi_sig_with_time_lock.ts +++ b/packages/contracts/test/multi_sig_with_time_lock.ts @@ -15,6 +15,7 @@ import { artifacts } from '../src/utils/artifacts'; import { expectRevertOrAlwaysFailingTransaction } from '../src/utils/assertions'; import { chaiSetup } from '../src/utils/chai_setup'; import { constants } from '../src/utils/constants'; +import { increaseTimeAndMineBlockAsync } from '../src/utils/increase_time'; import { MultiSigWrapper } from '../src/utils/multi_sig_wrapper'; import { provider, txDefaults, web3Wrapper } from '../src/utils/web3_wrapper'; @@ -155,16 +156,8 @@ describe('MultiSigWalletWithTimeLock', () => { ); }); - // TODO(albrow): increaseTimeAsync not supported it('should execute if it has enough confirmations and is past the time lock', async () => { - await web3Wrapper.increaseTimeAsync(SECONDS_TIME_LOCKED.toNumber()); - // Note: we need to send a transaction after increasing time so - // that a block is actually mined. The contract looks at the - // last mined block for the timestamp. - await web3Wrapper.awaitTransactionSuccessAsync( - await web3Wrapper.sendTransactionAsync({ from: owners[0], to: owners[1], value: 1 }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); + await increaseTimeAndMineBlockAsync(SECONDS_TIME_LOCKED.toNumber()); await web3Wrapper.awaitTransactionSuccessAsync( await multiSig.executeTransaction.sendTransactionAsync(txId, { from: owners[0] }), constants.AWAIT_TRANSACTION_MINED_MS, -- cgit v1.2.3