diff options
Diffstat (limited to 'packages/contracts/test')
| -rw-r--r-- | packages/contracts/test/asset_proxy/authorizable.ts | 4 | ||||
| -rw-r--r-- | packages/contracts/test/asset_proxy/proxies.ts | 237 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/core.ts | 39 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/dispatcher.ts | 54 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/internal.ts | 201 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/match_orders.ts | 314 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/signature_validator.ts | 14 | ||||
| -rw-r--r-- | packages/contracts/test/libraries/lib_bytes.ts | 95 | ||||
| -rw-r--r-- | packages/contracts/test/multisig/asset_proxy_owner.ts | 45 | ||||
| -rw-r--r-- | packages/contracts/test/tokens/unlimited_allowance_token.ts | 8 | ||||
| -rw-r--r-- | packages/contracts/test/utils/artifacts.ts | 2 | ||||
| -rw-r--r-- | packages/contracts/test/utils/assertions.ts | 17 | ||||
| -rw-r--r-- | packages/contracts/test/utils/constants.ts | 1 | ||||
| -rw-r--r-- | packages/contracts/test/utils/web3_wrapper.ts | 4 |
14 files changed, 855 insertions, 180 deletions
diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts index e99c6cee3..0c0da46b3 100644 --- a/packages/contracts/test/asset_proxy/authorizable.ts +++ b/packages/contracts/test/asset_proxy/authorizable.ts @@ -2,6 +2,7 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { RevertReason } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; +import * as _ from 'lodash'; import { MixinAuthorizableContract } from '../../generated_contract_wrappers/mixin_authorizable'; import { artifacts } from '../utils/artifacts'; @@ -28,8 +29,7 @@ describe('Authorizable', () => { }); before(async () => { const accounts = await web3Wrapper.getAvailableAddressesAsync(); - owner = address = accounts[0]; - notOwner = accounts[1]; + [owner, address, notOwner] = _.slice(accounts, 0, 3); authorizable = await MixinAuthorizableContract.deployFrom0xArtifactAsync( artifacts.MixinAuthorizable, provider, diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index 6031e554d..4d70031d1 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -8,6 +8,8 @@ import * as _ from 'lodash'; import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token'; import { DummyERC721ReceiverContract } from '../../generated_contract_wrappers/dummy_erc721_receiver'; import { DummyERC721TokenContract } from '../../generated_contract_wrappers/dummy_erc721_token'; +import { DummyMultipleReturnERC20TokenContract } from '../../generated_contract_wrappers/dummy_multiple_return_erc20_token'; +import { DummyNoReturnERC20TokenContract } from '../../generated_contract_wrappers/dummy_no_return_erc20_token'; import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; import { IAssetProxyContract } from '../../generated_contract_wrappers/i_asset_proxy'; @@ -42,6 +44,8 @@ describe('Asset Transfer Proxies', () => { let erc721Receiver: DummyERC721ReceiverContract; let erc20Proxy: ERC20ProxyContract; let erc721Proxy: ERC721ProxyContract; + let noReturnErc20Token: DummyNoReturnERC20TokenContract; + let multipleReturnErc20Token: DummyMultipleReturnERC20TokenContract; let erc20Wrapper: ERC20Wrapper; let erc721Wrapper: ERC721Wrapper; @@ -91,6 +95,51 @@ describe('Asset Transfer Proxies', () => { provider, txDefaults, ); + noReturnErc20Token = await DummyNoReturnERC20TokenContract.deployFrom0xArtifactAsync( + artifacts.DummyNoReturnERC20Token, + provider, + txDefaults, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + constants.DUMMY_TOKEN_DECIMALS, + constants.DUMMY_TOKEN_TOTAL_SUPPLY, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await noReturnErc20Token.setBalance.sendTransactionAsync(makerAddress, constants.INITIAL_ERC20_BALANCE), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await noReturnErc20Token.approve.sendTransactionAsync( + erc20Proxy.address, + constants.INITIAL_ERC20_ALLOWANCE, + { from: makerAddress }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + multipleReturnErc20Token = await DummyMultipleReturnERC20TokenContract.deployFrom0xArtifactAsync( + artifacts.DummyMultipleReturnERC20Token, + provider, + txDefaults, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + constants.DUMMY_TOKEN_DECIMALS, + constants.DUMMY_TOKEN_TOTAL_SUPPLY, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await multipleReturnErc20Token.setBalance.sendTransactionAsync( + makerAddress, + constants.INITIAL_ERC20_BALANCE, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await multipleReturnErc20Token.approve.sendTransactionAsync( + erc20Proxy.address, + constants.INITIAL_ERC20_ALLOWANCE, + { from: makerAddress }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); }); beforeEach(async () => { await blockchainLifecycle.startAsync(); @@ -141,6 +190,65 @@ describe('Asset Transfer Proxies', () => { ); }); + it('should successfully transfer tokens that do not return a value', async () => { + // Construct ERC20 asset data + const encodedAssetData = assetDataUtils.encodeERC20AssetData(noReturnErc20Token.address); + // Perform a transfer from makerAddress to takerAddress + const initialMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress); + const initialTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress); + const amount = new BigNumber(10); + const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( + encodedAssetData, + makerAddress, + takerAddress, + amount, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + to: erc20Proxy.address, + data, + from: exchangeAddress, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Verify transfer was successful + const newMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress); + const newTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress); + expect(newMakerBalance).to.be.bignumber.equal(initialMakerBalance.minus(amount)); + expect(newTakerBalance).to.be.bignumber.equal(initialTakerBalance.plus(amount)); + }); + + it('should successfully transfer tokens and ignore extra assetData', async () => { + // Construct ERC20 asset data + const extraData = '0102030405060708'; + const encodedAssetData = `${assetDataUtils.encodeERC20AssetData(zrxToken.address)}${extraData}`; + // Perform a transfer from makerAddress to takerAddress + const erc20Balances = await erc20Wrapper.getBalancesAsync(); + const amount = new BigNumber(10); + const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( + encodedAssetData, + makerAddress, + takerAddress, + amount, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + to: erc20Proxy.address, + data, + from: exchangeAddress, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Verify transfer was successful + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][zrxToken.address].minus(amount), + ); + expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[takerAddress][zrxToken.address].add(amount), + ); + }); + it('should do nothing if transferring 0 amount of a token', async () => { // Construct ERC20 asset data const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address); @@ -189,6 +297,7 @@ describe('Asset Transfer Proxies', () => { }), constants.AWAIT_TRANSACTION_MINED_MS, ); + const erc20Balances = await erc20Wrapper.getBalancesAsync(); // Perform a transfer; expect this to fail. await expectTransactionFailedAsync( web3Wrapper.sendTransactionAsync({ @@ -198,6 +307,43 @@ describe('Asset Transfer Proxies', () => { }), RevertReason.TransferFailed, ); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(newBalances).to.deep.equal(erc20Balances); + }); + + it('should throw if allowances are too low and token does not return a value', async () => { + // Construct ERC20 asset data + const encodedAssetData = assetDataUtils.encodeERC20AssetData(noReturnErc20Token.address); + // Create allowance less than transfer amount. Set allowance on proxy. + const allowance = new BigNumber(0); + const amount = new BigNumber(10); + const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( + encodedAssetData, + makerAddress, + takerAddress, + amount, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await noReturnErc20Token.approve.sendTransactionAsync(erc20Proxy.address, allowance, { + from: makerAddress, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const initialMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress); + const initialTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress); + // Perform a transfer; expect this to fail. + await expectTransactionFailedAsync( + web3Wrapper.sendTransactionAsync({ + to: erc20Proxy.address, + data, + from: exchangeAddress, + }), + RevertReason.TransferFailed, + ); + const newMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress); + const newTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress); + expect(newMakerBalance).to.be.bignumber.equal(initialMakerBalance); + expect(newTakerBalance).to.be.bignumber.equal(initialTakerBalance); }); it('should throw if requesting address is not authorized', async () => { @@ -211,6 +357,7 @@ describe('Asset Transfer Proxies', () => { takerAddress, amount, ); + const erc20Balances = await erc20Wrapper.getBalancesAsync(); await expectTransactionFailedAsync( web3Wrapper.sendTransactionAsync({ to: erc20Proxy.address, @@ -219,6 +366,35 @@ describe('Asset Transfer Proxies', () => { }), RevertReason.SenderNotAuthorized, ); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(newBalances).to.deep.equal(erc20Balances); + }); + + it('should throw if token returns more than 32 bytes', async () => { + // Construct ERC20 asset data + const encodedAssetData = assetDataUtils.encodeERC20AssetData(multipleReturnErc20Token.address); + const amount = new BigNumber(10); + const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( + encodedAssetData, + makerAddress, + takerAddress, + amount, + ); + const initialMakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(makerAddress); + const initialTakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(takerAddress); + // Perform a transfer; expect this to fail. + await expectTransactionFailedAsync( + web3Wrapper.sendTransactionAsync({ + to: erc20Proxy.address, + data, + from: exchangeAddress, + }), + RevertReason.TransferFailed, + ); + const newMakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(makerAddress); + const newTakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(takerAddress); + expect(newMakerBalance).to.be.bignumber.equal(initialMakerBalance); + expect(newTakerBalance).to.be.bignumber.equal(initialTakerBalance); }); }); @@ -247,7 +423,38 @@ describe('Asset Transfer Proxies', () => { const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); - expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); + expect(ownerMakerAsset).to.be.equal(makerAddress); + // Perform a transfer from makerAddress to takerAddress + const amount = new BigNumber(1); + const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( + encodedAssetData, + makerAddress, + takerAddress, + amount, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + to: erc721Proxy.address, + data, + from: exchangeAddress, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Verify transfer was successful + const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(newOwnerMakerAsset).to.be.bignumber.equal(takerAddress); + }); + + it('should successfully transfer tokens and ignore extra assetData', async () => { + // Construct ERC721 asset data + const extraData = '0102030405060708'; + const encodedAssetData = `${assetDataUtils.encodeERC721AssetData( + erc721Token.address, + erc721MakerTokenId, + )}${extraData}`; + // Verify pre-condition + const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(ownerMakerAsset).to.be.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(1); const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( @@ -274,7 +481,7 @@ describe('Asset Transfer Proxies', () => { const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); - expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); + expect(ownerMakerAsset).to.be.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(1); const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( @@ -304,7 +511,7 @@ describe('Asset Transfer Proxies', () => { const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); - expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); + expect(ownerMakerAsset).to.be.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(0); const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( @@ -313,7 +520,7 @@ describe('Asset Transfer Proxies', () => { takerAddress, amount, ); - return expectTransactionFailedAsync( + await expectTransactionFailedAsync( web3Wrapper.sendTransactionAsync({ to: erc721Proxy.address, data, @@ -321,6 +528,8 @@ describe('Asset Transfer Proxies', () => { }), RevertReason.InvalidAmount, ); + const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(newOwner).to.be.equal(ownerMakerAsset); }); it('should throw if transferring > 1 amount of a token', async () => { @@ -328,7 +537,7 @@ describe('Asset Transfer Proxies', () => { const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); - expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); + expect(ownerMakerAsset).to.be.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(500); const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( @@ -337,7 +546,7 @@ describe('Asset Transfer Proxies', () => { takerAddress, amount, ); - return expectTransactionFailedAsync( + await expectTransactionFailedAsync( web3Wrapper.sendTransactionAsync({ to: erc721Proxy.address, data, @@ -345,11 +554,16 @@ describe('Asset Transfer Proxies', () => { }), RevertReason.InvalidAmount, ); + const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(newOwner).to.be.equal(ownerMakerAsset); }); it('should throw if allowances are too low', async () => { // Construct ERC721 asset data const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); + // Verify pre-condition + const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(ownerMakerAsset).to.be.equal(makerAddress); // Remove transfer approval for makerAddress. await web3Wrapper.awaitTransactionSuccessAsync( await erc721Token.approve.sendTransactionAsync(constants.NULL_ADDRESS, erc721MakerTokenId, { @@ -365,7 +579,7 @@ describe('Asset Transfer Proxies', () => { takerAddress, amount, ); - return expectTransactionFailedAsync( + await expectTransactionFailedAsync( web3Wrapper.sendTransactionAsync({ to: erc721Proxy.address, data, @@ -373,11 +587,16 @@ describe('Asset Transfer Proxies', () => { }), RevertReason.TransferFailed, ); + const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(newOwner).to.be.equal(ownerMakerAsset); }); it('should throw if requesting address is not authorized', async () => { // Construct ERC721 asset data const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); + // Verify pre-condition + const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(ownerMakerAsset).to.be.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(1); const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( @@ -386,7 +605,7 @@ describe('Asset Transfer Proxies', () => { takerAddress, amount, ); - return expectTransactionFailedAsync( + await expectTransactionFailedAsync( web3Wrapper.sendTransactionAsync({ to: erc721Proxy.address, data, @@ -394,6 +613,8 @@ describe('Asset Transfer Proxies', () => { }), RevertReason.SenderNotAuthorized, ); + const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(newOwner).to.be.equal(ownerMakerAsset); }); }); diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index 3bb71b58f..acb99eed1 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -8,7 +8,10 @@ import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); import * as _ from 'lodash'; -import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token'; +import { + DummyERC20TokenContract, + DummyERC20TokenTransferEventArgs, +} from '../../generated_contract_wrappers/dummy_erc20_token'; import { DummyERC721TokenContract } from '../../generated_contract_wrappers/dummy_erc721_token'; import { DummyNoReturnERC20TokenContract } from '../../generated_contract_wrappers/dummy_no_return_erc20_token'; import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; @@ -243,6 +246,40 @@ describe('Exchange core', () => { RevertReason.ValidatorError, ); }); + + it('should not emit transfer events for transfers where from == to', async () => { + const txReceipt = await exchangeWrapper.fillOrderAsync(signedOrder, makerAddress); + const logs = txReceipt.logs; + const transferLogs = _.filter( + logs, + log => (log as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).event === 'Transfer', + ); + expect(transferLogs.length).to.be.equal(2); + expect((transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).address).to.be.equal( + zrxToken.address, + ); + expect((transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._from).to.be.equal( + makerAddress, + ); + expect((transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._to).to.be.equal( + feeRecipientAddress, + ); + expect( + (transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._value, + ).to.be.bignumber.equal(signedOrder.makerFee); + expect((transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).address).to.be.equal( + zrxToken.address, + ); + expect((transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._from).to.be.equal( + makerAddress, + ); + expect((transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._to).to.be.equal( + feeRecipientAddress, + ); + expect( + (transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._value, + ).to.be.bignumber.equal(signedOrder.takerFee); + }); }); describe('Testing exchange of ERC20 tokens with no return values', () => { diff --git a/packages/contracts/test/exchange/dispatcher.ts b/packages/contracts/test/exchange/dispatcher.ts index 81871a680..a8ae897a8 100644 --- a/packages/contracts/test/exchange/dispatcher.ts +++ b/packages/contracts/test/exchange/dispatcher.ts @@ -205,6 +205,60 @@ describe('AssetProxyDispatcher', () => { ); }); + it('should not dispatch a transfer if amount == 0', async () => { + // Register ERC20 proxy + await web3Wrapper.awaitTransactionSuccessAsync( + await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Construct metadata for ERC20 proxy + const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address); + + // Perform a transfer from makerAddress to takerAddress + const erc20Balances = await erc20Wrapper.getBalancesAsync(); + const amount = constants.ZERO_AMOUNT; + const txReceipt = await web3Wrapper.awaitTransactionSuccessAsync( + await assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync( + encodedAssetData, + makerAddress, + takerAddress, + amount, + { from: owner }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + expect(txReceipt.logs.length).to.be.equal(0); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(newBalances).to.deep.equal(erc20Balances); + }); + + it('should not dispatch a transfer if from == to', async () => { + // Register ERC20 proxy + await web3Wrapper.awaitTransactionSuccessAsync( + await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Construct metadata for ERC20 proxy + const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address); + + // Perform a transfer from makerAddress to takerAddress + const erc20Balances = await erc20Wrapper.getBalancesAsync(); + const amount = new BigNumber(10); + const txReceipt = await web3Wrapper.awaitTransactionSuccessAsync( + await assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync( + encodedAssetData, + makerAddress, + makerAddress, + amount, + { from: owner }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + expect(txReceipt.logs.length).to.be.equal(0); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(newBalances).to.deep.equal(erc20Balances); + }); + it('should throw if dispatching to unregistered proxy', async () => { // Construct metadata for ERC20 proxy const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address); diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index de381fca3..156e086af 100644 --- a/packages/contracts/test/exchange/internal.ts +++ b/packages/contracts/test/exchange/internal.ts @@ -6,10 +6,7 @@ import * as _ from 'lodash'; import { TestExchangeInternalsContract } from '../../generated_contract_wrappers/test_exchange_internals'; import { artifacts } from '../utils/artifacts'; -import { - getInvalidOpcodeErrorMessageForCallAsync, - getRevertReasonOrErrorMessageForSendTransactionAsync, -} from '../utils/assertions'; +import { getRevertReasonOrErrorMessageForSendTransactionAsync } from '../utils/assertions'; import { chaiSetup } from '../utils/chai_setup'; import { bytes32Values, testCombinatoriallyWithReferenceFuncAsync, uint256Values } from '../utils/combinatorial_utils'; import { constants } from '../utils/constants'; @@ -48,9 +45,9 @@ const overflowErrorForCall = new Error(RevertReason.Uint256Overflow); describe('Exchange core internal functions', () => { let testExchange: TestExchangeInternalsContract; - let invalidOpcodeErrorForCall: Error | undefined; let overflowErrorForSendTransaction: Error | undefined; let divisionByZeroErrorForCall: Error | undefined; + let roundingErrorForCall: Error | undefined; before(async () => { await blockchainLifecycle.startAsync(); @@ -68,12 +65,67 @@ describe('Exchange core internal functions', () => { await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.Uint256Overflow), ); divisionByZeroErrorForCall = new Error(RevertReason.DivisionByZero); - invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync()); + roundingErrorForCall = new Error(RevertReason.RoundingError); }); // Note(albrow): Don't forget to add beforeEach and afterEach calls to reset // the blockchain state for any tests which modify it! - async function referenceGetPartialAmountFloorAsync( + async function referenceIsRoundingErrorFloorAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise<boolean> { + if (denominator.eq(0)) { + throw divisionByZeroErrorForCall; + } + if (numerator.eq(0)) { + return false; + } + if (target.eq(0)) { + return false; + } + const product = numerator.mul(target); + const remainder = product.mod(denominator); + const remainderTimes1000 = remainder.mul('1000'); + const isError = remainderTimes1000.gte(product); + if (product.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + if (remainderTimes1000.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + return isError; + } + + async function referenceIsRoundingErrorCeilAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise<boolean> { + if (denominator.eq(0)) { + throw divisionByZeroErrorForCall; + } + if (numerator.eq(0)) { + return false; + } + if (target.eq(0)) { + return false; + } + const product = numerator.mul(target); + const remainder = product.mod(denominator); + const error = denominator.sub(remainder).mod(denominator); + const errorTimes1000 = error.mul('1000'); + const isError = errorTimes1000.gte(product); + if (product.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + if (errorTimes1000.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + return isError; + } + + async function referenceSafeGetPartialAmountFloorAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, @@ -81,6 +133,10 @@ describe('Exchange core internal functions', () => { if (denominator.eq(0)) { throw divisionByZeroErrorForCall; } + const isRoundingError = await referenceIsRoundingErrorFloorAsync(numerator, denominator, target); + if (isRoundingError) { + throw roundingErrorForCall; + } const product = numerator.mul(target); if (product.greaterThan(MAX_UINT256)) { throw overflowErrorForCall; @@ -162,19 +218,22 @@ describe('Exchange core internal functions', () => { // in any mathematical operation in either the reference TypeScript // implementation or the Solidity implementation of // calculateFillResults. + const makerAssetFilledAmount = await referenceSafeGetPartialAmountFloorAsync( + takerAssetFilledAmount, + orderTakerAssetAmount, + otherAmount, + ); + const order = makeOrder(otherAmount, orderTakerAssetAmount, otherAmount, otherAmount); + const orderMakerAssetAmount = order.makerAssetAmount; return { - makerAssetFilledAmount: await referenceGetPartialAmountFloorAsync( - takerAssetFilledAmount, - orderTakerAssetAmount, - otherAmount, - ), + makerAssetFilledAmount, takerAssetFilledAmount, - makerFeePaid: await referenceGetPartialAmountFloorAsync( - takerAssetFilledAmount, - orderTakerAssetAmount, + makerFeePaid: await referenceSafeGetPartialAmountFloorAsync( + makerAssetFilledAmount, + orderMakerAssetAmount, otherAmount, ), - takerFeePaid: await referenceGetPartialAmountFloorAsync( + takerFeePaid: await referenceSafeGetPartialAmountFloorAsync( takerAssetFilledAmount, orderTakerAssetAmount, otherAmount, @@ -198,6 +257,20 @@ describe('Exchange core internal functions', () => { }); describe('getPartialAmountFloor', async () => { + async function referenceGetPartialAmountFloorAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise<BigNumber> { + if (denominator.eq(0)) { + throw divisionByZeroErrorForCall; + } + const product = numerator.mul(target); + if (product.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + return product.dividedToIntegerBy(denominator); + } async function testGetPartialAmountFloorAsync( numerator: BigNumber, denominator: BigNumber, @@ -206,7 +279,7 @@ describe('Exchange core internal functions', () => { return testExchange.publicGetPartialAmountFloor.callAsync(numerator, denominator, target); } await testCombinatoriallyWithReferenceFuncAsync( - 'getPartialAmount', + 'getPartialAmountFloor', referenceGetPartialAmountFloorAsync, testGetPartialAmountFloorAsync, [uint256Values, uint256Values, uint256Values], @@ -250,76 +323,80 @@ describe('Exchange core internal functions', () => { ); }); - describe('isRoundingError', async () => { - async function referenceIsRoundingErrorAsync( + describe('safeGetPartialAmountFloor', async () => { + async function testSafeGetPartialAmountFloorAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, - ): Promise<boolean> { + ): Promise<BigNumber> { + return testExchange.publicSafeGetPartialAmountFloor.callAsync(numerator, denominator, target); + } + await testCombinatoriallyWithReferenceFuncAsync( + 'safeGetPartialAmountFloor', + referenceSafeGetPartialAmountFloorAsync, + testSafeGetPartialAmountFloorAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('safeGetPartialAmountCeil', async () => { + async function referenceSafeGetPartialAmountCeilAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise<BigNumber> { if (denominator.eq(0)) { throw divisionByZeroErrorForCall; } - if (numerator.eq(0)) { - return false; - } - if (target.eq(0)) { - return false; + const isRoundingError = await referenceIsRoundingErrorCeilAsync(numerator, denominator, target); + if (isRoundingError) { + throw roundingErrorForCall; } const product = numerator.mul(target); - const remainder = product.mod(denominator); - const remainderTimes1000 = remainder.mul('1000'); - const isError = remainderTimes1000.gt(product); - if (product.greaterThan(MAX_UINT256)) { + const offset = product.add(denominator.sub(1)); + if (offset.greaterThan(MAX_UINT256)) { throw overflowErrorForCall; } - if (remainderTimes1000.greaterThan(MAX_UINT256)) { - throw overflowErrorForCall; + const result = offset.dividedToIntegerBy(denominator); + if (product.mod(denominator).eq(0)) { + expect(result.mul(denominator)).to.be.bignumber.eq(product); + } else { + expect(result.mul(denominator)).to.be.bignumber.gt(product); } - return isError; + return result; } - async function testIsRoundingErrorAsync( + async function testSafeGetPartialAmountCeilAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, - ): Promise<boolean> { - return testExchange.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target); + ): Promise<BigNumber> { + return testExchange.publicSafeGetPartialAmountCeil.callAsync(numerator, denominator, target); } await testCombinatoriallyWithReferenceFuncAsync( - 'isRoundingError', - referenceIsRoundingErrorAsync, - testIsRoundingErrorAsync, + 'safeGetPartialAmountCeil', + referenceSafeGetPartialAmountCeilAsync, + testSafeGetPartialAmountCeilAsync, [uint256Values, uint256Values, uint256Values], ); }); - describe('isRoundingErrorCeil', async () => { - async function referenceIsRoundingErrorAsync( + describe('isRoundingErrorFloor', async () => { + async function testIsRoundingErrorFloorAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, ): Promise<boolean> { - if (denominator.eq(0)) { - throw divisionByZeroErrorForCall; - } - if (numerator.eq(0)) { - return false; - } - if (target.eq(0)) { - return false; - } - const product = numerator.mul(target); - const remainder = product.mod(denominator); - const error = denominator.sub(remainder).mod(denominator); - const errorTimes1000 = error.mul('1000'); - const isError = errorTimes1000.gt(product); - if (product.greaterThan(MAX_UINT256)) { - throw overflowErrorForCall; - } - if (errorTimes1000.greaterThan(MAX_UINT256)) { - throw overflowErrorForCall; - } - return isError; + return testExchange.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target); } + await testCombinatoriallyWithReferenceFuncAsync( + 'isRoundingErrorFloor', + referenceIsRoundingErrorFloorAsync, + testIsRoundingErrorFloorAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('isRoundingErrorCeil', async () => { async function testIsRoundingErrorCeilAsync( numerator: BigNumber, denominator: BigNumber, @@ -329,7 +406,7 @@ describe('Exchange core internal functions', () => { } await testCombinatoriallyWithReferenceFuncAsync( 'isRoundingErrorCeil', - referenceIsRoundingErrorAsync, + referenceIsRoundingErrorCeilAsync, testIsRoundingErrorCeilAsync, [uint256Values, uint256Values, uint256Values], ); diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 8732e20ba..c6e7b494f 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -3,6 +3,7 @@ import { assetDataUtils } from '@0xproject/order-utils'; import { RevertReason } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; +import * as chai from 'chai'; import * as _ from 'lodash'; import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token'; @@ -11,8 +12,10 @@ import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_prox import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; import { ExchangeContract } from '../../generated_contract_wrappers/exchange'; import { ReentrantERC20TokenContract } from '../../generated_contract_wrappers/reentrant_erc20_token'; +import { TestExchangeInternalsContract } from '../../generated_contract_wrappers/test_exchange_internals'; import { artifacts } from '../utils/artifacts'; import { expectTransactionFailedAsync } from '../utils/assertions'; +import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; import { ERC20Wrapper } from '../utils/erc20_wrapper'; import { ERC721Wrapper } from '../utils/erc721_wrapper'; @@ -23,6 +26,8 @@ import { ERC20BalancesByOwner, ERC721TokenIdsByOwner } from '../utils/types'; import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); +chaiSetup.configure(); +const expect = chai.expect; describe('matchOrders', () => { let makerAddressLeft: string; @@ -58,6 +63,8 @@ describe('matchOrders', () => { let matchOrderTester: MatchOrderTester; + let testExchange: TestExchangeInternalsContract; + before(async () => { await blockchainLifecycle.startAsync(); }); @@ -160,6 +167,11 @@ describe('matchOrders', () => { orderFactoryRight = new OrderFactory(privateKeyRight, defaultOrderParamsRight); // Set match order tester matchOrderTester = new MatchOrderTester(exchangeWrapper, erc20Wrapper, erc721Wrapper, zrxToken.address); + testExchange = await TestExchangeInternalsContract.deployFrom0xArtifactAsync( + artifacts.TestExchangeInternals, + provider, + txDefaults, + ); }); beforeEach(async () => { await blockchainLifecycle.startAsync(); @@ -173,39 +185,170 @@ describe('matchOrders', () => { erc721TokenIdsByOwner = await erc721Wrapper.getBalancesAsync(); }); - it('Should give right maker a better price when correct price is not integral', async () => { + it('Should transfer correct amounts when right order is fully filled and values pass isRoundingErrorFloor but fail isRoundingErrorCeil', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAddress: makerAddressLeft, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(17), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(98), 0), + feeRecipientAddress: feeRecipientAddressLeft, + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAddress: makerAddressRight, + makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), + takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(13), 0), + feeRecipientAddress: feeRecipientAddressRight, + }); + // Assert is rounding error ceil & not rounding error floor + // These assertions are taken from MixinMatchOrders::calculateMatchedFillResults + // The rounding error is derived computating how much the left maker will sell. + const numerator = signedOrderLeft.makerAssetAmount; + const denominator = signedOrderLeft.takerAssetAmount; + const target = signedOrderRight.makerAssetAmount; + const isRoundingErrorCeil = await testExchange.publicIsRoundingErrorCeil.callAsync( + numerator, + denominator, + target, + ); + expect(isRoundingErrorCeil).to.be.true(); + const isRoundingErrorFloor = await testExchange.publicIsRoundingErrorFloor.callAsync( + numerator, + denominator, + target, + ); + expect(isRoundingErrorFloor).to.be.false(); + // Match signedOrderLeft with signedOrderRight + // Note that the left maker received a slightly better sell price. + // This is intentional; see note in MixinMatchOrders.calculateMatchedFillResults. + // Because the left maker received a slightly more favorable sell price, the fee + // paid by the left taker is slightly higher than that paid by the left maker. + // Fees can be thought of as a tax paid by the seller, derived from the sale price. + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(13), 0), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 0), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber('76.4705882352941176'), 16), // 76.47% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 0), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(13), 0), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(0), 0), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber('76.5306122448979591'), 16), // 76.53% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + }; + await matchOrderTester.matchOrdersAndAssertEffectsAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + expectedTransferAmounts, + ); + }); + + it('Should transfer correct amounts when left order is fully filled and values pass isRoundingErrorCeil but fail isRoundingErrorFloor', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ makerAddress: makerAddressLeft, - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2000), 0), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1001), 0), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(15), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 0), feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ makerAddress: makerAddressRight, makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10000), 0), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(3000), 0), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(97), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(14), 0), + feeRecipientAddress: feeRecipientAddressRight, + }); + // Assert is rounding error floor & not rounding error ceil + // These assertions are taken from MixinMatchOrders::calculateMatchedFillResults + // The rounding error is derived computating how much the right maker will buy. + const numerator = signedOrderRight.takerAssetAmount; + const denominator = signedOrderRight.makerAssetAmount; + const target = signedOrderLeft.takerAssetAmount; + const isRoundingErrorFloor = await testExchange.publicIsRoundingErrorFloor.callAsync( + numerator, + denominator, + target, + ); + expect(isRoundingErrorFloor).to.be.true(); + const isRoundingErrorCeil = await testExchange.publicIsRoundingErrorCeil.callAsync( + numerator, + denominator, + target, + ); + expect(isRoundingErrorCeil).to.be.false(); + // Match signedOrderLeft with signedOrderRight + // Note that the right maker received a slightly better purchase price. + // This is intentional; see note in MixinMatchOrders.calculateMatchedFillResults. + // Because the right maker received a slightly more favorable buy price, the fee + // paid by the right taker is slightly higher than that paid by the right maker. + // Fees can be thought of as a tax paid by the seller, derived from the sale price. + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(15), 0), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 0), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 0), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(13), 0), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber('92.7835051546391752'), 16), // 92.78% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 0), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber('92.8571428571428571'), 16), // 92.85% + }; + await matchOrderTester.matchOrdersAndAssertEffectsAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + expectedTransferAmounts, + ); + }); + + it('Should give right maker a better buy price when rounding', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAddress: makerAddressLeft, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(16), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(22), 0), + feeRecipientAddress: feeRecipientAddressLeft, + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAddress: makerAddressRight, + makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), + takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(83), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(49), 0), feeRecipientAddress: feeRecipientAddressRight, }); // Note: + // The correct price buy price for the right maker would yield (49/83) * 22 = 12.988 units + // of the left maker asset. This gets rounded up to 13, giving the right maker a better price. + // Note: // The maker/taker fee percentage paid on the right order differs because - // they received different sale prices. Similarly, the right maker pays a - // slightly higher lower than the right taker. + // they received different sale prices. The right maker pays a + // fee slightly lower than the right taker. const expectedTransferAmounts = { // Left Maker - amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2000), 0), - amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1001), 0), + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(16), 0), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(22), 0), feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Right Maker - amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1001), 0), - amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(301), 0), - feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10.01), 16), // 10.01% + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(22), 0), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(13), 0), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber('26.5060240963855421'), 16), // 26.506% // Taker - amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1699), 0), + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber('10.0333333333333333'), 16), // 10.03% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber('26.5306122448979591'), 16), // 26.531% }; // Match signedOrderLeft with signedOrderRight await matchOrderTester.matchOrdersAndAssertEffectsAsync( @@ -218,7 +361,7 @@ describe('matchOrders', () => { ); }); - it('Should give left maker a better price when correct price is not integral', async () => { + it('Should give left maker a better sell price when rounding', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ makerAddress: makerAddressLeft, @@ -236,7 +379,8 @@ describe('matchOrders', () => { }); // Note: // The maker/taker fee percentage paid on the left order differs because - // they received different sale prices. + // they received different sale prices. The left maker pays a fee + // slightly lower than the left taker. const expectedTransferAmounts = { // Left Maker amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(11), 0), @@ -262,42 +406,87 @@ describe('matchOrders', () => { ); }); - it('Should transfer correct amounts when right order fill amount deviates from amount derived by `Exchange.fillOrder`', async () => { + it('Should give right maker and right taker a favorable fee price when rounding', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ makerAddress: makerAddressLeft, - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 0), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(16), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(22), 0), feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ makerAddress: makerAddressRight, makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 0), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 0), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(83), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(49), 0), feeRecipientAddress: feeRecipientAddressRight, + makerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(10000), 0), + takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(10000), 0), }); - // TODO: These values will change after implementation of rounding up has been merged + // Note: + // The maker/taker fee percentage paid on the right order differs because + // they received different sale prices. The right maker pays a + // fee slightly lower than the right taker. const expectedTransferAmounts = { // Left Maker - amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 0), - amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(16), 0), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(22), 0), feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Right Maker - // Note: - // For order [4,2] valid fill amounts through `Exchange.fillOrder` would be [2, 1] or [4, 2] - // In this case we have fill amounts of [3, 1] which is attainable through - // `Exchange.matchOrders` but not `Exchange.fillOrder` - // Note: - // The right maker fee differs from the right taker fee because their exchange rate differs. - // The right maker always receives the better exchange and fee price. - amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), - amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 0), - feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 16), // 75% + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(22), 0), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(13), 0), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2650), 0), // 2650.6 rounded down tro 2650 // Taker - amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(8), 0), + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(2653), 0), // 2653.1 rounded down to 2653 + }; + // Match signedOrderLeft with signedOrderRight + await matchOrderTester.matchOrdersAndAssertEffectsAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + expectedTransferAmounts, + ); + }); + + it('Should give left maker and left taker a favorable fee price when rounding', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAddress: makerAddressLeft, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(12), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(97), 0), + feeRecipientAddress: feeRecipientAddressLeft, + makerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(10000), 0), + takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(10000), 0), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAddress: makerAddressRight, + makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), + takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(89), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), + feeRecipientAddress: feeRecipientAddressRight, + }); + // Note: + // The maker/taker fee percentage paid on the left order differs because + // they received different sale prices. The left maker pays a + // fee slightly lower than the left taker. + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(11), 0), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(89), 0), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(9166), 0), // 9166.6 rounded down to 9166 + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(89), 0), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 0), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(9175), 0), // 9175.2 rounded down to 9175 feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; // Match signedOrderLeft with signedOrderRight @@ -311,6 +500,61 @@ describe('matchOrders', () => { ); }); + it('Should transfer correct amounts when right order fill amount deviates from amount derived by `Exchange.fillOrder`', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAddress: makerAddressLeft, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1005), 0), + feeRecipientAddress: feeRecipientAddressLeft, + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAddress: makerAddressRight, + makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), + takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2126), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1063), 0), + feeRecipientAddress: feeRecipientAddressRight, + }); + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 0), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1005), 0), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + // Notes: + // i. + // The left order is fully filled by the right order, so the right maker must sell 1005 units of their asset to the left maker. + // By selling 1005 units, the right maker should theoretically receive 502.5 units of the left maker's asset. + // Since the transfer amount must be an integer, this value must be rounded down to 502 or up to 503. + // ii. + // If the right order were filled via `Exchange.fillOrder` the respective fill amounts would be [1004, 502] or [1006, 503]. + // It follows that we cannot trigger a sale of 1005 units of the right maker's asset through `Exchange.fillOrder`. + // iii. + // For an optimal match, the algorithm must choose either [1005, 502] or [1005, 503] as fill amounts for the right order. + // The algorithm favors the right maker when the exchange rate must be rounded, so the final fill for the right order is [1005, 503]. + // iv. + // The right maker fee differs from the right taker fee because their exchange rate differs. + // The right maker always receives the better exchange and fee price. + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1005), 0), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(503), 0), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber('47.2718720602069614'), 16), // 47.27% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(497), 0), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber('47.3189087488240827'), 16), // 47.31% + }; + // Match signedOrderLeft with signedOrderRight + await matchOrderTester.matchOrdersAndAssertEffectsAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + expectedTransferAmounts, + ); + }); + const reentrancyTest = (functionNames: string[]) => { _.forEach(functionNames, async (functionName: string, functionId: number) => { const description = `should not allow matchOrders to reenter the Exchange contract via ${functionName}`; diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index da2febfd8..5cc62e777 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -14,7 +14,7 @@ import { ValidatorContract } from '../../generated_contract_wrappers/validator'; import { WalletContract } from '../../generated_contract_wrappers/wallet'; import { addressUtils } from '../utils/address_utils'; import { artifacts } from '../utils/artifacts'; -import { expectContractCallFailed, expectContractCallFailedWithoutReasonAsync } from '../utils/assertions'; +import { expectContractCallFailedAsync } from '../utils/assertions'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; import { LogDecoder } from '../utils/log_decoder'; @@ -119,7 +119,7 @@ describe('MixinSignatureValidator', () => { it('should revert when signature is empty', async () => { const emptySignature = '0x'; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectContractCallFailed( + return expectContractCallFailedAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, @@ -133,7 +133,7 @@ describe('MixinSignatureValidator', () => { const unsupportedSignatureType = SignatureType.NSignatureTypes; const unsupportedSignatureHex = '0x' + Buffer.from([unsupportedSignatureType]).toString('hex'); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectContractCallFailed( + return expectContractCallFailedAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, @@ -146,7 +146,7 @@ describe('MixinSignatureValidator', () => { it('should revert when SignatureType=Illegal', async () => { const unsupportedSignatureHex = '0x' + Buffer.from([SignatureType.Illegal]).toString('hex'); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectContractCallFailed( + return expectContractCallFailedAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, @@ -173,7 +173,7 @@ describe('MixinSignatureValidator', () => { const signatureBuffer = Buffer.concat([fillerData, signatureType]); const signatureHex = ethUtil.bufferToHex(signatureBuffer); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectContractCallFailed( + return expectContractCallFailedAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, @@ -339,7 +339,7 @@ describe('MixinSignatureValidator', () => { ethUtil.toBuffer(`0x${SignatureType.Wallet}`), ]); const signatureHex = ethUtil.bufferToHex(signature); - await expectContractCallFailed( + await expectContractCallFailedAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, maliciousWallet.address, @@ -385,7 +385,7 @@ describe('MixinSignatureValidator', () => { const signature = Buffer.concat([validatorAddress, signatureType]); const signatureHex = ethUtil.bufferToHex(signature); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - await expectContractCallFailed( + await expectContractCallFailedAsync( signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex), RevertReason.ValidatorError, ); diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index 1c497a226..13640a761 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -9,7 +9,7 @@ import * as _ from 'lodash'; import { TestLibBytesContract } from '../../generated_contract_wrappers/test_lib_bytes'; import { artifacts } from '../utils/artifacts'; -import { expectContractCallFailed } from '../utils/assertions'; +import { expectContractCallFailedAsync } from '../utils/assertions'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; import { typeEncodingUtils } from '../utils/type_encoding_utils'; @@ -41,6 +41,8 @@ describe('LibBytes', () => { const testBytes32B = '0x534877abd8443578526845cdfef020047528759477fedef87346527659aced32'; const testUint256 = new BigNumber(testBytes32, 16); const testUint256B = new BigNumber(testBytes32B, 16); + const testBytes4 = '0xabcdef12'; + const testByte = '0xab'; let shortData: string; let shortTestBytes: string; let shortTestBytesAsBuffer: Buffer; @@ -101,34 +103,47 @@ describe('LibBytes', () => { describe('popLastByte', () => { it('should revert if length is 0', async () => { - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicPopLastByte.callAsync(constants.NULL_BYTES), RevertReason.LibBytesGreaterThanZeroLengthRequired, ); }); - it('should pop the last byte from the input and return it', async () => { + it('should pop the last byte from the input and return it when array holds more than 1 byte', async () => { const [newBytes, poppedByte] = await libBytes.publicPopLastByte.callAsync(byteArrayLongerThan32Bytes); const expectedNewBytes = byteArrayLongerThan32Bytes.slice(0, -2); const expectedPoppedByte = `0x${byteArrayLongerThan32Bytes.slice(-2)}`; expect(newBytes).to.equal(expectedNewBytes); expect(poppedByte).to.equal(expectedPoppedByte); }); + it('should pop the last byte from the input and return it when array is exactly 1 byte', async () => { + const [newBytes, poppedByte] = await libBytes.publicPopLastByte.callAsync(testByte); + const expectedNewBytes = '0x'; + expect(newBytes).to.equal(expectedNewBytes); + return expect(poppedByte).to.be.equal(testByte); + }); }); describe('popLast20Bytes', () => { it('should revert if length is less than 20', async () => { - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicPopLast20Bytes.callAsync(byteArrayShorterThan20Bytes), RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); }); - it('should pop the last 20 bytes from the input and return it', async () => { + it('should pop the last 20 bytes from the input and return it when array holds more than 20 bytes', async () => { const [newBytes, poppedAddress] = await libBytes.publicPopLast20Bytes.callAsync(byteArrayLongerThan32Bytes); const expectedNewBytes = byteArrayLongerThan32Bytes.slice(0, -40); const expectedPoppedAddress = `0x${byteArrayLongerThan32Bytes.slice(-40)}`; expect(newBytes).to.equal(expectedNewBytes); expect(poppedAddress).to.equal(expectedPoppedAddress); }); + it('should pop the last 20 bytes from the input and return it when array is exactly 20 bytes', async () => { + const [newBytes, poppedAddress] = await libBytes.publicPopLast20Bytes.callAsync(testAddress); + const expectedNewBytes = '0x'; + const expectedPoppedAddress = testAddress; + expect(newBytes).to.equal(expectedNewBytes); + expect(poppedAddress).to.equal(expectedPoppedAddress); + }); }); describe('equals', () => { @@ -185,7 +200,7 @@ describe('LibBytes', () => { describe('deepCopyBytes', () => { it('should revert if dest is shorter than source', async () => { - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicDeepCopyBytes.callAsync(byteArrayShorterThan32Bytes, byteArrayLongerThan32Bytes), RevertReason.LibBytesGreaterOrEqualToSourceBytesLengthRequired, ); @@ -238,7 +253,7 @@ describe('LibBytes', () => { it('should fail if the byte array is too short to hold an address', async () => { const shortByteArray = '0xabcdef'; const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadAddress.callAsync(shortByteArray, offset), RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); @@ -246,7 +261,7 @@ describe('LibBytes', () => { it('should fail if the length between the offset and end of the byte array is too short to hold an address', async () => { const byteArray = testAddress; const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadAddress.callAsync(byteArray, badOffset), RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); @@ -282,7 +297,7 @@ describe('LibBytes', () => { }); it('should fail if the byte array is too short to hold an address', async () => { const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteAddress.callAsync(byteArrayShorterThan20Bytes, offset, testAddress), RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); @@ -290,7 +305,7 @@ describe('LibBytes', () => { it('should fail if the length between the offset and end of the byte array is too short to hold an address', async () => { const byteArray = byteArrayLongerThan32Bytes; const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteAddress.callAsync(byteArray, badOffset, testAddress), RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); @@ -303,7 +318,7 @@ describe('LibBytes', () => { const bytes32 = await libBytes.publicReadBytes32.callAsync(testBytes32, testBytes32Offset); return expect(bytes32).to.be.equal(testBytes32); }); - it('should successfully read bytes32 when it is offset in the array)', async () => { + it('should successfully read bytes32 when it is offset in the array', async () => { const bytes32ByteArrayBuffer = ethUtil.toBuffer(testBytes32); const prefixByteArrayBuffer = ethUtil.toBuffer('0xabcdef'); const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, bytes32ByteArrayBuffer]); @@ -314,14 +329,14 @@ describe('LibBytes', () => { }); it('should fail if the byte array is too short to hold a bytes32', async () => { const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytes32.callAsync(byteArrayShorterThan32Bytes, offset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(testBytes32).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytes32.callAsync(testBytes32, badOffset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -357,7 +372,7 @@ describe('LibBytes', () => { }); it('should fail if the byte array is too short to hold a bytes32', async () => { const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteBytes32.callAsync(byteArrayShorterThan32Bytes, offset, testBytes32), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -365,7 +380,7 @@ describe('LibBytes', () => { it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32', async () => { const byteArray = byteArrayLongerThan32Bytes; const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteBytes32.callAsync(byteArray, badOffset, testBytes32), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -393,7 +408,7 @@ describe('LibBytes', () => { }); it('should fail if the byte array is too short to hold a uint256', async () => { const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadUint256.callAsync(byteArrayShorterThan32Bytes, offset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -403,7 +418,7 @@ describe('LibBytes', () => { const testUint256AsBuffer = ethUtil.toBuffer(formattedTestUint256); const byteArray = ethUtil.bufferToHex(testUint256AsBuffer); const badOffset = new BigNumber(testUint256AsBuffer.byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadUint256.callAsync(byteArray, badOffset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -443,7 +458,7 @@ describe('LibBytes', () => { }); it('should fail if the byte array is too short to hold a uint256', async () => { const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteUint256.callAsync(byteArrayShorterThan32Bytes, offset, testUint256), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -451,7 +466,7 @@ describe('LibBytes', () => { it('should fail if the length between the offset and end of the byte array is too short to hold a uint256', async () => { const byteArray = byteArrayLongerThan32Bytes; const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteUint256.callAsync(byteArray, badOffset, testUint256), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -462,8 +477,9 @@ describe('LibBytes', () => { // AssertionError: expected promise to be rejected with an error including 'revert' but it was fulfilled with '0x08c379a0' it('should revert if byte array has a length < 4', async () => { const byteArrayLessThan4Bytes = '0x010101'; - return expectContractCallFailed( - libBytes.publicReadBytes4.callAsync(byteArrayLessThan4Bytes, new BigNumber(0)), + const offset = new BigNumber(0); + return expectContractCallFailedAsync( + libBytes.publicReadBytes4.callAsync(byteArrayLessThan4Bytes, offset), RevertReason.LibBytesGreaterOrEqualTo4LengthRequired, ); }); @@ -472,6 +488,27 @@ describe('LibBytes', () => { const expectedFirst4Bytes = byteArrayLongerThan32Bytes.slice(0, 10); expect(first4Bytes).to.equal(expectedFirst4Bytes); }); + it('should successfully read bytes4 when the bytes4 takes up the whole array', async () => { + const testBytes4Offset = new BigNumber(0); + const bytes4 = await libBytes.publicReadBytes4.callAsync(testBytes4, testBytes4Offset); + return expect(bytes4).to.be.equal(testBytes4); + }); + it('should successfully read bytes4 when it is offset in the array', async () => { + const bytes4ByteArrayBuffer = ethUtil.toBuffer(testBytes4); + const prefixByteArrayBuffer = ethUtil.toBuffer('0xabcdef'); + const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, bytes4ByteArrayBuffer]); + const combinedByteArray = ethUtil.bufferToHex(combinedByteArrayBuffer); + const testBytes4Offset = new BigNumber(prefixByteArrayBuffer.byteLength); + const bytes4 = await libBytes.publicReadBytes4.callAsync(combinedByteArray, testBytes4Offset); + return expect(bytes4).to.be.equal(testBytes4); + }); + it('should fail if the length between the offset and end of the byte array is too short to hold a bytes4', async () => { + const badOffset = new BigNumber(ethUtil.toBuffer(testBytes4).byteLength); + return expectContractCallFailedAsync( + libBytes.publicReadBytes4.callAsync(testBytes4, badOffset), + RevertReason.LibBytesGreaterOrEqualTo4LengthRequired, + ); + }); }); describe('readBytesWithLength', () => { @@ -517,28 +554,28 @@ describe('LibBytes', () => { it('should fail if the byte array is too short to hold the length of a nested byte array', async () => { // The length of the nested array is 32 bytes. By storing less than 32 bytes, a length cannot be read. const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytesWithLength.callAsync(byteArrayShorterThan32Bytes, offset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); it('should fail if we store a nested byte array length, without a nested byte array', async () => { const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytesWithLength.callAsync(testBytes32, offset), RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(byteArrayShorterThan32Bytes).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytesWithLength.callAsync(byteArrayShorterThan32Bytes, badOffset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold the nested byte array', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(testBytes32).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytesWithLength.callAsync(testBytes32, badOffset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -546,7 +583,7 @@ describe('LibBytes', () => { }); describe('writeBytesWithLength', () => { - it('should successfully write short, nested array of bytes when it takes up the whole array)', async () => { + it('should successfully write short, nested array of bytes when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex(new Buffer(shortTestBytesAsBuffer.byteLength)); const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( @@ -650,15 +687,15 @@ describe('LibBytes', () => { it('should fail if the byte array is too short to hold the length of a nested byte array', async () => { const offset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex(new Buffer(1)); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, offset, longData), RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired, ); }); - it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array)', async () => { + it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array', async () => { const emptyByteArray = ethUtil.bufferToHex(new Buffer(shortTestBytesAsBuffer.byteLength)); const badOffset = new BigNumber(ethUtil.toBuffer(shortTestBytesAsBuffer).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, badOffset, shortData), RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired, ); diff --git a/packages/contracts/test/multisig/asset_proxy_owner.ts b/packages/contracts/test/multisig/asset_proxy_owner.ts index 9515941ff..bb2b3b1a3 100644 --- a/packages/contracts/test/multisig/asset_proxy_owner.ts +++ b/packages/contracts/test/multisig/asset_proxy_owner.ts @@ -1,4 +1,5 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; +import { RevertReason } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; @@ -14,9 +15,11 @@ import { MixinAuthorizableContract } from '../../generated_contract_wrappers/mix import { TestAssetProxyOwnerContract } from '../../generated_contract_wrappers/test_asset_proxy_owner'; import { artifacts } from '../utils/artifacts'; import { - expectContractCallFailedWithoutReasonAsync, - expectContractCreationFailedWithoutReason, + expectContractCallFailedAsync, + expectContractCreationFailedAsync, + expectTransactionFailedAsync, expectTransactionFailedWithoutReasonAsync, + sendTransactionResult, } from '../utils/assertions'; import { increaseTimeAndMineBlockAsync } from '../utils/block_timestamp'; import { chaiSetup } from '../utils/chai_setup'; @@ -109,8 +112,8 @@ describe('AssetProxyOwner', () => { }); it('should throw if a null address is included in assetProxyContracts', async () => { const assetProxyContractAddresses = [erc20Proxy.address, constants.NULL_ADDRESS]; - return expectContractCreationFailedWithoutReason( - AssetProxyOwnerContract.deployFrom0xArtifactAsync( + return expectContractCreationFailedAsync( + (AssetProxyOwnerContract.deployFrom0xArtifactAsync( artifacts.AssetProxyOwner, provider, txDefaults, @@ -118,7 +121,8 @@ describe('AssetProxyOwner', () => { assetProxyContractAddresses, REQUIRED_APPROVALS, SECONDS_TIME_LOCKED, - ), + ) as any) as sendTransactionResult, + RevertReason.InvalidAssetProxy, ); }); }); @@ -148,25 +152,6 @@ describe('AssetProxyOwner', () => { }); }); - describe('readBytes4', () => { - it('should revert if byte array has a length < 4', async () => { - const byteArrayLessThan4Bytes = '0x010101'; - return expectContractCallFailedWithoutReasonAsync( - testAssetProxyOwner.publicReadBytes4.callAsync(byteArrayLessThan4Bytes, new BigNumber(0)), - ); - }); - it('should return the first 4 bytes of a byte array of arbitrary length', async () => { - const byteArrayLongerThan32Bytes = - '0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef'; - const first4Bytes = await testAssetProxyOwner.publicReadBytes4.callAsync( - byteArrayLongerThan32Bytes, - new BigNumber(0), - ); - const expectedFirst4Bytes = byteArrayLongerThan32Bytes.slice(0, 10); - expect(first4Bytes).to.equal(expectedFirst4Bytes); - }); - }); - describe('registerAssetProxy', () => { it('should throw if not called by multisig', async () => { const isRegistered = true; @@ -300,8 +285,9 @@ describe('AssetProxyOwner', () => { ); const log = submitTxRes.logs[0] as LogWithDecodedArgs<AssetProxyOwnerSubmissionEventArgs>; const txId = log.args.transactionId; - return expectContractCallFailedWithoutReasonAsync( + return expectContractCallFailedAsync( testAssetProxyOwner.testValidRemoveAuthorizedAddressAtIndexTx.callAsync(txId), + RevertReason.InvalidFunctionSelector, ); }); @@ -335,8 +321,9 @@ describe('AssetProxyOwner', () => { ); const log = submitTxRes.logs[0] as LogWithDecodedArgs<AssetProxyOwnerSubmissionEventArgs>; const txId = log.args.transactionId; - return expectContractCallFailedWithoutReasonAsync( + return expectContractCallFailedAsync( testAssetProxyOwner.testValidRemoveAuthorizedAddressAtIndexTx.callAsync(txId), + RevertReason.UnregisteredAssetProxy, ); }); }); @@ -377,10 +364,11 @@ describe('AssetProxyOwner', () => { await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - return expectTransactionFailedWithoutReasonAsync( + return expectTransactionFailedAsync( testAssetProxyOwner.executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, { from: owners[1], }), + RevertReason.UnregisteredAssetProxy, ); }); @@ -399,10 +387,11 @@ describe('AssetProxyOwner', () => { await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - return expectTransactionFailedWithoutReasonAsync( + return expectTransactionFailedAsync( testAssetProxyOwner.executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, { from: owners[1], }), + RevertReason.InvalidFunctionSelector, ); }); diff --git a/packages/contracts/test/tokens/unlimited_allowance_token.ts b/packages/contracts/test/tokens/unlimited_allowance_token.ts index f2725b408..63680fe9b 100644 --- a/packages/contracts/test/tokens/unlimited_allowance_token.ts +++ b/packages/contracts/test/tokens/unlimited_allowance_token.ts @@ -5,7 +5,7 @@ import * as chai from 'chai'; import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token'; import { artifacts } from '../utils/artifacts'; -import { expectContractCallFailed } from '../utils/assertions'; +import { expectContractCallFailedAsync } from '../utils/assertions'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; @@ -54,7 +54,7 @@ describe('UnlimitedAllowanceToken', () => { it('should throw if owner has insufficient balance', async () => { const ownerBalance = await token.balanceOf.callAsync(owner); const amountToTransfer = ownerBalance.plus(1); - return expectContractCallFailed( + return expectContractCallFailedAsync( token.transfer.callAsync(spender, amountToTransfer, { from: owner }), RevertReason.Erc20InsufficientBalance, ); @@ -93,7 +93,7 @@ describe('UnlimitedAllowanceToken', () => { await token.approve.sendTransactionAsync(spender, amountToTransfer, { from: owner }), constants.AWAIT_TRANSACTION_MINED_MS, ); - return expectContractCallFailed( + return expectContractCallFailedAsync( token.transferFrom.callAsync(owner, spender, amountToTransfer, { from: spender, }), @@ -109,7 +109,7 @@ describe('UnlimitedAllowanceToken', () => { const isSpenderAllowanceInsufficient = spenderAllowance.cmp(amountToTransfer) < 0; expect(isSpenderAllowanceInsufficient).to.be.true(); - return expectContractCallFailed( + return expectContractCallFailedAsync( token.transferFrom.callAsync(owner, spender, amountToTransfer, { from: spender, }), diff --git a/packages/contracts/test/utils/artifacts.ts b/packages/contracts/test/utils/artifacts.ts index 5ddb5cc7f..53f2a4e4e 100644 --- a/packages/contracts/test/utils/artifacts.ts +++ b/packages/contracts/test/utils/artifacts.ts @@ -4,6 +4,7 @@ import * as AssetProxyOwner from '../../artifacts/AssetProxyOwner.json'; import * as DummyERC20Token from '../../artifacts/DummyERC20Token.json'; import * as DummyERC721Receiver from '../../artifacts/DummyERC721Receiver.json'; import * as DummyERC721Token from '../../artifacts/DummyERC721Token.json'; +import * as DummyMultipleReturnERC20Token from '../../artifacts/DummyMultipleReturnERC20Token.json'; import * as DummyNoReturnERC20Token from '../../artifacts/DummyNoReturnERC20Token.json'; import * as ERC20Proxy from '../../artifacts/ERC20Proxy.json'; import * as ERC721Proxy from '../../artifacts/ERC721Proxy.json'; @@ -37,6 +38,7 @@ export const artifacts = { DummyERC20Token: (DummyERC20Token as any) as ContractArtifact, DummyERC721Receiver: (DummyERC721Receiver as any) as ContractArtifact, DummyERC721Token: (DummyERC721Token as any) as ContractArtifact, + DummyMultipleReturnERC20Token: (DummyMultipleReturnERC20Token as any) as ContractArtifact, DummyNoReturnERC20Token: (DummyNoReturnERC20Token as any) as ContractArtifact, ERC20Proxy: (ERC20Proxy as any) as ContractArtifact, ERC721Proxy: (ERC721Proxy as any) as ContractArtifact, diff --git a/packages/contracts/test/utils/assertions.ts b/packages/contracts/test/utils/assertions.ts index 61df800c8..3361a751a 100644 --- a/packages/contracts/test/utils/assertions.ts +++ b/packages/contracts/test/utils/assertions.ts @@ -159,7 +159,7 @@ export async function expectTransactionFailedWithoutReasonAsync(p: sendTransacti * @returns a new Promise which will reject if the conditions are not met and * otherwise resolve with no value. */ -export async function expectContractCallFailed<T>(p: Promise<T>, reason: RevertReason): Promise<void> { +export async function expectContractCallFailedAsync<T>(p: Promise<T>, reason: RevertReason): Promise<void> { return expect(p).to.be.rejectedWith(reason); } @@ -180,7 +180,20 @@ export async function expectContractCallFailedWithoutReasonAsync<T>(p: Promise<T * @returns a new Promise which will reject if the conditions are not met and * otherwise resolve with no value. */ -export async function expectContractCreationFailedWithoutReason<T>(p: Promise<T>): Promise<void> { +export async function expectContractCreationFailedAsync<T>( + p: sendTransactionResult, + reason: RevertReason, +): Promise<void> { + return expectTransactionFailedAsync(p, reason); +} + +/** + * Resolves if the contract creation/deployment fails without a revert reason. + * @param p a Promise resulting from a contract creation/deployment + * @returns a new Promise which will reject if the conditions are not met and + * otherwise resolve with no value. + */ +export async function expectContractCreationFailedWithoutReasonAsync<T>(p: Promise<T>): Promise<void> { const errMessage = await _getTransactionFailedErrorMessageAsync(); return expect(p).to.be.rejectedWith(errMessage); } diff --git a/packages/contracts/test/utils/constants.ts b/packages/contracts/test/utils/constants.ts index ee4378d2e..b9ba8ccb9 100644 --- a/packages/contracts/test/utils/constants.ts +++ b/packages/contracts/test/utils/constants.ts @@ -60,6 +60,7 @@ export const constants = { 'MARKET_SELL_ORDERS', 'MATCH_ORDERS', 'CANCEL_ORDER', + 'BATCH_CANCEL_ORDERS', 'CANCEL_ORDERS_UP_TO', 'SET_SIGNATURE_VALIDATOR_APPROVAL', ], diff --git a/packages/contracts/test/utils/web3_wrapper.ts b/packages/contracts/test/utils/web3_wrapper.ts index acb3103b7..d1cd3d387 100644 --- a/packages/contracts/test/utils/web3_wrapper.ts +++ b/packages/contracts/test/utils/web3_wrapper.ts @@ -1,5 +1,5 @@ import { devConstants, env, EnvVars, web3Factory } from '@0xproject/dev-utils'; -import { prependSubprovider } from '@0xproject/subproviders'; +import { prependSubprovider, Web3ProviderEngine } from '@0xproject/subproviders'; import { logUtils } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as _ from 'lodash'; @@ -47,7 +47,7 @@ const ganacheConfigs = { }; const providerConfigs = testProvider === ProviderType.Ganache ? ganacheConfigs : gethConfigs; -export const provider = web3Factory.getRpcProvider(providerConfigs); +export const provider: Web3ProviderEngine = web3Factory.getRpcProvider(providerConfigs); const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); const isProfilerEnabled = env.parseBoolean(EnvVars.SolidityProfiler); const isRevertTraceEnabled = env.parseBoolean(EnvVars.SolidityRevertTrace); |