diff options
Diffstat (limited to 'packages/contracts/test/exchange')
| -rw-r--r-- | packages/contracts/test/exchange/core.ts | 142 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/dispatcher.ts | 72 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/fill_order.ts | 2 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/internal.ts | 287 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/libs.ts | 102 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/match_orders.ts | 922 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/signature_validator.ts | 189 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/transactions.ts | 20 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/wrapper.ts | 215 |
9 files changed, 1491 insertions, 460 deletions
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index bc2bad749..fc8dc5346 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -1,20 +1,22 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetDataUtils, orderHashUtils } from '@0xproject/order-utils'; -import { RevertReason, SignedOrder } from '@0xproject/types'; -import { BigNumber } from '@0xproject/utils'; -import { Web3Wrapper } from '@0xproject/web3-wrapper'; +import { BlockchainLifecycle } from '@0x/dev-utils'; +import { assetDataUtils, orderHashUtils } from '@0x/order-utils'; +import { RevertReason, SignatureType, SignedOrder } from '@0x/types'; +import { BigNumber } from '@0x/utils'; +import { Web3Wrapper } from '@0x/web3-wrapper'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); import * as _ from 'lodash'; -import { DummyERC20TokenContract } 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'; -import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; -import { ExchangeCancelEventArgs, ExchangeContract } from '../../generated_contract_wrappers/exchange'; -import { artifacts } from '../utils/artifacts'; +import { DummyERC20TokenContract, DummyERC20TokenTransferEventArgs } from '../../generated-wrappers/dummy_erc20_token'; +import { DummyERC721TokenContract } from '../../generated-wrappers/dummy_erc721_token'; +import { DummyNoReturnERC20TokenContract } from '../../generated-wrappers/dummy_no_return_erc20_token'; +import { ERC20ProxyContract } from '../../generated-wrappers/erc20_proxy'; +import { ERC721ProxyContract } from '../../generated-wrappers/erc721_proxy'; +import { ExchangeCancelEventArgs, ExchangeContract } from '../../generated-wrappers/exchange'; +import { ReentrantERC20TokenContract } from '../../generated-wrappers/reentrant_erc20_token'; +import { TestStaticCallReceiverContract } from '../../generated-wrappers/test_static_call_receiver'; +import { artifacts } from '../../src/artifacts'; import { expectTransactionFailedAsync } from '../utils/assertions'; import { getLatestBlockTimestampAsync, increaseTimeAndMineBlockAsync } from '../utils/block_timestamp'; import { chaiSetup } from '../utils/chai_setup'; @@ -41,9 +43,12 @@ describe('Exchange core', () => { let zrxToken: DummyERC20TokenContract; let erc721Token: DummyERC721TokenContract; let noReturnErc20Token: DummyNoReturnERC20TokenContract; + let reentrantErc20Token: ReentrantERC20TokenContract; let exchange: ExchangeContract; let erc20Proxy: ERC20ProxyContract; let erc721Proxy: ERC721ProxyContract; + let maliciousWallet: TestStaticCallReceiverContract; + let maliciousValidator: TestStaticCallReceiverContract; let signedOrder: SignedOrder; let erc20Balances: ERC20BalancesByOwner; @@ -109,6 +114,18 @@ describe('Exchange core', () => { constants.AWAIT_TRANSACTION_MINED_MS, ); + maliciousWallet = maliciousValidator = await TestStaticCallReceiverContract.deployFrom0xArtifactAsync( + artifacts.TestStaticCallReceiver, + provider, + txDefaults, + ); + reentrantErc20Token = await ReentrantERC20TokenContract.deployFrom0xArtifactAsync( + artifacts.ReentrantERC20Token, + provider, + txDefaults, + exchange.address, + ); + defaultMakerAssetAddress = erc20TokenA.address; defaultTakerAssetAddress = erc20TokenB.address; @@ -135,6 +152,26 @@ describe('Exchange core', () => { signedOrder = await orderFactory.newSignedOrderAsync(); }); + const reentrancyTest = (functionNames: string[]) => { + _.forEach(functionNames, async (functionName: string, functionId: number) => { + const description = `should not allow fillOrder to reenter the Exchange contract via ${functionName}`; + it(description, async () => { + signedOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address), + }); + await web3Wrapper.awaitTransactionSuccessAsync( + await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await expectTransactionFailedAsync( + exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), + RevertReason.TransferFailed, + ); + }); + }); + }; + describe('fillOrder reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX)); + it('should throw if signature is invalid', async () => { signedOrder = await orderFactory.newSignedOrderAsync({ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), @@ -161,6 +198,85 @@ describe('Exchange core', () => { RevertReason.OrderUnfillable, ); }); + + it('should revert if `isValidSignature` tries to update state when SignatureType=Wallet', async () => { + const maliciousMakerAddress = maliciousWallet.address; + await web3Wrapper.awaitTransactionSuccessAsync( + await erc20TokenA.setBalance.sendTransactionAsync( + maliciousMakerAddress, + constants.INITIAL_ERC20_BALANCE, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await maliciousWallet.approveERC20.sendTransactionAsync( + erc20TokenA.address, + erc20Proxy.address, + constants.INITIAL_ERC20_ALLOWANCE, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + signedOrder = await orderFactory.newSignedOrderAsync({ + makerAddress: maliciousMakerAddress, + makerFee: constants.ZERO_AMOUNT, + }); + signedOrder.signature = `0x0${SignatureType.Wallet}`; + await expectTransactionFailedAsync( + exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), + RevertReason.WalletError, + ); + }); + + it('should revert if `isValidSignature` tries to update state when SignatureType=Validator', async () => { + const isApproved = true; + await web3Wrapper.awaitTransactionSuccessAsync( + await exchange.setSignatureValidatorApproval.sendTransactionAsync( + maliciousValidator.address, + isApproved, + { from: makerAddress }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + signedOrder.signature = `${maliciousValidator.address}0${SignatureType.Validator}`; + await expectTransactionFailedAsync( + exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), + 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', () => { @@ -448,7 +564,7 @@ describe('Exchange core', () => { // HACK(albrow): We need to hardcode the gas estimate here because // the Geth gas estimator doesn't work with the way we use // delegatecall and swallow errors. - gas: 490000, + gas: 600000, }); const newBalances = await erc20Wrapper.getBalancesAsync(); diff --git a/packages/contracts/test/exchange/dispatcher.ts b/packages/contracts/test/exchange/dispatcher.ts index 81871a680..3d3aa42c2 100644 --- a/packages/contracts/test/exchange/dispatcher.ts +++ b/packages/contracts/test/exchange/dispatcher.ts @@ -1,19 +1,19 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetDataUtils } from '@0xproject/order-utils'; -import { AssetProxyId, RevertReason } from '@0xproject/types'; -import { BigNumber } from '@0xproject/utils'; +import { BlockchainLifecycle } from '@0x/dev-utils'; +import { assetDataUtils } from '@0x/order-utils'; +import { AssetProxyId, RevertReason } from '@0x/types'; +import { BigNumber } from '@0x/utils'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; import * as _ from 'lodash'; -import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token'; -import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; -import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; +import { DummyERC20TokenContract } from '../../generated-wrappers/dummy_erc20_token'; +import { ERC20ProxyContract } from '../../generated-wrappers/erc20_proxy'; +import { ERC721ProxyContract } from '../../generated-wrappers/erc721_proxy'; import { TestAssetProxyDispatcherAssetProxyRegisteredEventArgs, TestAssetProxyDispatcherContract, -} from '../../generated_contract_wrappers/test_asset_proxy_dispatcher'; -import { artifacts } from '../utils/artifacts'; +} from '../../generated-wrappers/test_asset_proxy_dispatcher'; +import { artifacts } from '../../src/artifacts'; import { expectTransactionFailedAsync } from '../utils/assertions'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; @@ -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/fill_order.ts b/packages/contracts/test/exchange/fill_order.ts index b1e08324f..37efaad2b 100644 --- a/packages/contracts/test/exchange/fill_order.ts +++ b/packages/contracts/test/exchange/fill_order.ts @@ -1,4 +1,4 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; +import { BlockchainLifecycle } from '@0x/dev-utils'; import * as _ from 'lodash'; import { chaiSetup } from '../utils/chai_setup'; diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index 67d1d2d2c..109be29c6 100644 --- a/packages/contracts/test/exchange/internal.ts +++ b/packages/contracts/test/exchange/internal.ts @@ -1,14 +1,12 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { Order, RevertReason, SignedOrder } from '@0xproject/types'; -import { BigNumber } from '@0xproject/utils'; +import { BlockchainLifecycle } from '@0x/dev-utils'; +import { Order, RevertReason, SignedOrder } from '@0x/types'; +import { BigNumber } from '@0x/utils'; +import * as chai from 'chai'; 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 { TestExchangeInternalsContract } from '../../generated-wrappers/test_exchange_internals'; +import { artifacts } from '../../src/artifacts'; +import { getRevertReasonOrErrorMessageForSendTransactionAsync } from '../utils/assertions'; import { chaiSetup } from '../utils/chai_setup'; import { bytes32Values, testCombinatoriallyWithReferenceFuncAsync, uint256Values } from '../utils/combinatorial_utils'; import { constants } from '../utils/constants'; @@ -16,6 +14,8 @@ import { FillResults } from '../utils/types'; import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; chaiSetup.configure(); +const expect = chai.expect; + const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); const MAX_UINT256 = new BigNumber(2).pow(256).minus(1); @@ -43,26 +43,11 @@ const emptySignedOrder: SignedOrder = { const overflowErrorForCall = new Error(RevertReason.Uint256Overflow); -async function referenceGetPartialAmountAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, -): Promise<BigNumber> { - const invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync()); - const product = numerator.mul(target); - if (product.greaterThan(MAX_UINT256)) { - throw overflowErrorForCall; - } - if (denominator.eq(0)) { - throw invalidOpcodeErrorForCall; - } - return product.dividedToIntegerBy(denominator); -} - 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(); @@ -79,11 +64,86 @@ describe('Exchange core internal functions', () => { overflowErrorForSendTransaction = new Error( await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.Uint256Overflow), ); - invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync()); + divisionByZeroErrorForCall = new Error(RevertReason.DivisionByZero); + 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 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, + ): Promise<BigNumber> { + 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; + } + return product.dividedToIntegerBy(denominator); + } + describe('addFillResults', async () => { function makeFillResults(value: BigNumber): FillResults { return { @@ -158,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 referenceGetPartialAmountAsync( - takerAssetFilledAmount, - orderTakerAssetAmount, - otherAmount, - ), + makerAssetFilledAmount, takerAssetFilledAmount, - makerFeePaid: await referenceGetPartialAmountAsync( - takerAssetFilledAmount, - orderTakerAssetAmount, + makerFeePaid: await referenceSafeGetPartialAmountFloorAsync( + makerAssetFilledAmount, + orderMakerAssetAmount, otherAmount, ), - takerFeePaid: await referenceGetPartialAmountAsync( + takerFeePaid: await referenceSafeGetPartialAmountFloorAsync( takerAssetFilledAmount, orderTakerAssetAmount, otherAmount, @@ -193,60 +256,158 @@ describe('Exchange core internal functions', () => { ); }); - describe('getPartialAmount', async () => { - async function testGetPartialAmountAsync( + describe('getPartialAmountFloor', async () => { + async function referenceGetPartialAmountFloorAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, ): Promise<BigNumber> { - return testExchange.publicGetPartialAmount.callAsync(numerator, denominator, target); + 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, + target: BigNumber, + ): Promise<BigNumber> { + return testExchange.publicGetPartialAmountFloor.callAsync(numerator, denominator, target); } await testCombinatoriallyWithReferenceFuncAsync( - 'getPartialAmount', - referenceGetPartialAmountAsync, - testGetPartialAmountAsync, + 'getPartialAmountFloor', + referenceGetPartialAmountFloorAsync, + testGetPartialAmountFloorAsync, [uint256Values, uint256Values, uint256Values], ); }); - describe('isRoundingError', async () => { - async function referenceIsRoundingErrorAsync( + describe('getPartialAmountCeil', async () => { + async function referenceGetPartialAmountCeilAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, - ): Promise<boolean> { - const product = numerator.mul(target); + ): Promise<BigNumber> { if (denominator.eq(0)) { - throw invalidOpcodeErrorForCall; - } - const remainder = product.mod(denominator); - if (remainder.eq(0)) { - return false; + throw divisionByZeroErrorForCall; } - if (product.greaterThan(MAX_UINT256)) { + const product = numerator.mul(target); + const offset = product.add(denominator.sub(1)); + if (offset.greaterThan(MAX_UINT256)) { throw overflowErrorForCall; } - if (product.eq(0)) { - throw invalidOpcodeErrorForCall; + 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); } - const remainderTimes1000000 = remainder.mul('1000000'); - if (remainderTimes1000000.greaterThan(MAX_UINT256)) { + return result; + } + async function testGetPartialAmountCeilAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise<BigNumber> { + return testExchange.publicGetPartialAmountCeil.callAsync(numerator, denominator, target); + } + await testCombinatoriallyWithReferenceFuncAsync( + 'getPartialAmountCeil', + referenceGetPartialAmountCeilAsync, + testGetPartialAmountCeilAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('safeGetPartialAmountFloor', async () => { + async function testSafeGetPartialAmountFloorAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): 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; + } + const isRoundingError = await referenceIsRoundingErrorCeilAsync(numerator, denominator, target); + if (isRoundingError) { + throw roundingErrorForCall; + } + const product = numerator.mul(target); + const offset = product.add(denominator.sub(1)); + if (offset.greaterThan(MAX_UINT256)) { throw overflowErrorForCall; } - const errPercentageTimes1000000 = remainderTimes1000000.dividedToIntegerBy(product); - return errPercentageTimes1000000.greaterThan('1000'); + 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 result; + } + async function testSafeGetPartialAmountCeilAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise<BigNumber> { + return testExchange.publicSafeGetPartialAmountCeil.callAsync(numerator, denominator, target); + } + await testCombinatoriallyWithReferenceFuncAsync( + 'safeGetPartialAmountCeil', + referenceSafeGetPartialAmountCeilAsync, + testSafeGetPartialAmountCeilAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('isRoundingErrorFloor', async () => { + async function testIsRoundingErrorFloorAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise<boolean> { + return testExchange.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target); } - async function testIsRoundingErrorAsync( + await testCombinatoriallyWithReferenceFuncAsync( + 'isRoundingErrorFloor', + referenceIsRoundingErrorFloorAsync, + testIsRoundingErrorFloorAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('isRoundingErrorCeil', async () => { + async function testIsRoundingErrorCeilAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, ): Promise<boolean> { - return testExchange.publicIsRoundingError.callAsync(numerator, denominator, target); + return testExchange.publicIsRoundingErrorCeil.callAsync(numerator, denominator, target); } await testCombinatoriallyWithReferenceFuncAsync( - 'isRoundingError', - referenceIsRoundingErrorAsync, - testIsRoundingErrorAsync, + 'isRoundingErrorCeil', + referenceIsRoundingErrorCeilAsync, + testIsRoundingErrorCeilAsync, [uint256Values, uint256Values, uint256Values], ); }); diff --git a/packages/contracts/test/exchange/libs.ts b/packages/contracts/test/exchange/libs.ts index 5c9f9aac7..503ef0e0f 100644 --- a/packages/contracts/test/exchange/libs.ts +++ b/packages/contracts/test/exchange/libs.ts @@ -1,13 +1,13 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetDataUtils, EIP712Utils, orderHashUtils } from '@0xproject/order-utils'; -import { SignedOrder } from '@0xproject/types'; -import { BigNumber } from '@0xproject/utils'; +import { BlockchainLifecycle } from '@0x/dev-utils'; +import { assetDataUtils, orderHashUtils } from '@0x/order-utils'; +import { SignedOrder } from '@0x/types'; +import { BigNumber } from '@0x/utils'; import * as chai from 'chai'; -import { TestConstantsContract } from '../../generated_contract_wrappers/test_constants'; -import { TestLibsContract } from '../../generated_contract_wrappers/test_libs'; +import { TestConstantsContract } from '../../generated-wrappers/test_constants'; +import { TestLibsContract } from '../../generated-wrappers/test_libs'; +import { artifacts } from '../../src/artifacts'; import { addressUtils } from '../utils/address_utils'; -import { artifacts } from '../utils/artifacts'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; import { OrderFactory } from '../utils/order_factory'; @@ -71,49 +71,61 @@ describe('Exchange libs', () => { // combinatorial tests in test/exchange/internal. They test specific edge // cases that are not covered by the combinatorial tests. describe('LibMath', () => { - it('should return false if there is a rounding error of 0.1%', async () => { - const numerator = new BigNumber(20); - const denominator = new BigNumber(999); - const target = new BigNumber(50); - // rounding error = ((20*50/999) - floor(20*50/999)) / (20*50/999) = 0.1% - const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); - expect(isRoundingError).to.be.false(); - }); - it('should return false if there is a rounding of 0.09%', async () => { - const numerator = new BigNumber(20); - const denominator = new BigNumber(9991); - const target = new BigNumber(500); - // rounding error = ((20*500/9991) - floor(20*500/9991)) / (20*500/9991) = 0.09% - const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); - expect(isRoundingError).to.be.false(); + describe('isRoundingError', () => { + it('should return true if there is a rounding error of 0.1%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(999); + const target = new BigNumber(50); + // rounding error = ((20*50/999) - floor(20*50/999)) / (20*50/999) = 0.1% + const isRoundingError = await libs.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.true(); + }); + it('should return false if there is a rounding of 0.09%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(9991); + const target = new BigNumber(500); + // rounding error = ((20*500/9991) - floor(20*500/9991)) / (20*500/9991) = 0.09% + const isRoundingError = await libs.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.false(); + }); + it('should return true if there is a rounding error of 0.11%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(9989); + const target = new BigNumber(500); + // rounding error = ((20*500/9989) - floor(20*500/9989)) / (20*500/9989) = 0.011% + const isRoundingError = await libs.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.true(); + }); }); - it('should return true if there is a rounding error of 0.11%', async () => { - const numerator = new BigNumber(20); - const denominator = new BigNumber(9989); - const target = new BigNumber(500); - // rounding error = ((20*500/9989) - floor(20*500/9989)) / (20*500/9989) = 0.011% - const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); - expect(isRoundingError).to.be.true(); + describe('isRoundingErrorCeil', () => { + it('should return true if there is a rounding error of 0.1%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(1001); + const target = new BigNumber(50); + // rounding error = (ceil(20*50/1001) - (20*50/1001)) / (20*50/1001) = 0.1% + const isRoundingError = await libs.publicIsRoundingErrorCeil.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.true(); + }); + it('should return false if there is a rounding of 0.09%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(10009); + const target = new BigNumber(500); + // rounding error = (ceil(20*500/10009) - (20*500/10009)) / (20*500/10009) = 0.09% + const isRoundingError = await libs.publicIsRoundingErrorCeil.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.false(); + }); + it('should return true if there is a rounding error of 0.11%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(10011); + const target = new BigNumber(500); + // rounding error = (ceil(20*500/10011) - (20*500/10011)) / (20*500/10011) = 0.11% + const isRoundingError = await libs.publicIsRoundingErrorCeil.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.true(); + }); }); }); describe('LibOrder', () => { - describe('getOrderSchema', () => { - it('should output the correct order schema hash', async () => { - const orderSchema = await libs.getOrderSchemaHash.callAsync(); - const schemaHashBuffer = orderHashUtils._getOrderSchemaBuffer(); - const schemaHashHex = `0x${schemaHashBuffer.toString('hex')}`; - expect(schemaHashHex).to.be.equal(orderSchema); - }); - }); - describe('getDomainSeparatorSchema', () => { - it('should output the correct domain separator schema hash', async () => { - const domainSeparatorSchema = await libs.getDomainSeparatorSchemaHash.callAsync(); - const domainSchemaBuffer = EIP712Utils._getDomainSeparatorSchemaBuffer(); - const schemaHashHex = `0x${domainSchemaBuffer.toString('hex')}`; - expect(schemaHashHex).to.be.equal(domainSeparatorSchema); - }); - }); describe('getOrderHash', () => { it('should output the correct orderHash', async () => { signedOrder = await orderFactory.newSignedOrderAsync(); diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 46b3569bd..eea9992d9 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -1,17 +1,19 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetDataUtils } from '@0xproject/order-utils'; -import { RevertReason } from '@0xproject/types'; -import { BigNumber } from '@0xproject/utils'; -import { Web3Wrapper } from '@0xproject/web3-wrapper'; +import { BlockchainLifecycle } from '@0x/dev-utils'; +import { assetDataUtils } from '@0x/order-utils'; +import { RevertReason } from '@0x/types'; +import { BigNumber } from '@0x/utils'; +import { Web3Wrapper } from '@0x/web3-wrapper'; import * as chai from 'chai'; import * as _ from 'lodash'; -import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token'; -import { DummyERC721TokenContract } from '../../generated_contract_wrappers/dummy_erc721_token'; -import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; -import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; -import { ExchangeContract } from '../../generated_contract_wrappers/exchange'; -import { artifacts } from '../utils/artifacts'; +import { DummyERC20TokenContract } from '../../generated-wrappers/dummy_erc20_token'; +import { DummyERC721TokenContract } from '../../generated-wrappers/dummy_erc721_token'; +import { ERC20ProxyContract } from '../../generated-wrappers/erc20_proxy'; +import { ERC721ProxyContract } from '../../generated-wrappers/erc721_proxy'; +import { ExchangeContract } from '../../generated-wrappers/exchange'; +import { ReentrantERC20TokenContract } from '../../generated-wrappers/reentrant_erc20_token'; +import { TestExchangeInternalsContract } from '../../generated-wrappers/test_exchange_internals'; +import { artifacts } from '../../src/artifacts'; import { expectTransactionFailedAsync } from '../utils/assertions'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; @@ -20,12 +22,12 @@ import { ERC721Wrapper } from '../utils/erc721_wrapper'; import { ExchangeWrapper } from '../utils/exchange_wrapper'; import { MatchOrderTester } from '../utils/match_order_tester'; import { OrderFactory } from '../utils/order_factory'; -import { ERC20BalancesByOwner, ERC721TokenIdsByOwner, OrderInfo, OrderStatus } from '../utils/types'; +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; -const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); describe('matchOrders', () => { let makerAddressLeft: string; @@ -39,6 +41,7 @@ describe('matchOrders', () => { let erc20TokenB: DummyERC20TokenContract; let zrxToken: DummyERC20TokenContract; let erc721Token: DummyERC721TokenContract; + let reentrantErc20Token: ReentrantERC20TokenContract; let exchange: ExchangeContract; let erc20Proxy: ERC20ProxyContract; let erc721Proxy: ERC721ProxyContract; @@ -60,6 +63,8 @@ describe('matchOrders', () => { let matchOrderTester: MatchOrderTester; + let testExchange: TestExchangeInternalsContract; + before(async () => { await blockchainLifecycle.startAsync(); }); @@ -127,23 +132,46 @@ describe('matchOrders', () => { }), constants.AWAIT_TRANSACTION_MINED_MS, ); + + reentrantErc20Token = await ReentrantERC20TokenContract.deployFrom0xArtifactAsync( + artifacts.ReentrantERC20Token, + provider, + txDefaults, + exchange.address, + ); + // Set default addresses defaultERC20MakerAssetAddress = erc20TokenA.address; defaultERC20TakerAssetAddress = erc20TokenB.address; defaultERC721AssetAddress = erc721Token.address; // Create default order parameters - const defaultOrderParams = { + const defaultOrderParamsLeft = { ...constants.STATIC_ORDER_PARAMS, + makerAddress: makerAddressLeft, exchangeAddress: exchange.address, makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), + feeRecipientAddress: feeRecipientAddressLeft, + }; + const defaultOrderParamsRight = { + ...constants.STATIC_ORDER_PARAMS, + makerAddress: makerAddressRight, + exchangeAddress: exchange.address, + makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), + takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), + feeRecipientAddress: feeRecipientAddressRight, }; const privateKeyLeft = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressLeft)]; - orderFactoryLeft = new OrderFactory(privateKeyLeft, defaultOrderParams); + orderFactoryLeft = new OrderFactory(privateKeyLeft, defaultOrderParamsLeft); const privateKeyRight = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressRight)]; - orderFactoryRight = new OrderFactory(privateKeyRight, defaultOrderParams); + 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(); @@ -157,263 +185,674 @@ describe('matchOrders', () => { erc721TokenIdsByOwner = await erc721Wrapper.getBalancesAsync(); }); - it('should transfer the correct amounts when orders completely fill each other', 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(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + 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(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + 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 - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + // 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, ); - // Verify left order was fully filled - const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft); - expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); - // Verify right order was fully filled - const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight); - expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); }); - it('should transfer the correct amounts when orders completely fill each other and taker doesnt take a profit', async () => { + 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(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + 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(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(97), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(14), 0), feeRecipientAddress: feeRecipientAddressRight, }); - // Store original taker balance - const takerInitialBalances = _.cloneDeep(erc20BalancesByOwner[takerAddress][defaultERC20MakerAssetAddress]); + // 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 - let newERC20BalancesByOwner: ERC20BalancesByOwner; - let newERC721TokenIdsByOwner: ERC721TokenIdsByOwner; - // prettier-ignore - [ - newERC20BalancesByOwner, - // tslint:disable-next-line:trailing-comma - newERC721TokenIdsByOwner - ] = await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + // 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, ); - // Verify left order was fully filled - const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft); - expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); - // Verify right order was fully filled - const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight); - expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); - // Verify taker did not take a profit - expect(takerInitialBalances).to.be.deep.equal( - newERC20BalancesByOwner[takerAddress][defaultERC20MakerAssetAddress], + }); + + 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. The right maker pays a + // fee slightly lower than the right taker. + const expectedTransferAmounts = { + // Left Maker + 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(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(3), 0), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber('26.5306122448979591'), 16), // 26.531% + }; + // Match signedOrderLeft with signedOrderRight + await matchOrderTester.matchOrdersAndAssertEffectsAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + expectedTransferAmounts, ); }); - it('should transfer the correct amounts when left order is completely filled and right order is partially filled', async () => { + it('Should give left maker a better sell price when rounding', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ makerAddress: makerAddressLeft, - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(12), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(97), 0), feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ makerAddress: makerAddressRight, makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(89), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), feeRecipientAddress: feeRecipientAddressRight, }); - // Match orders - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + // 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('91.6666666666666666'), 16), // 91.6% + // 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('91.7525773195876288'), 16), // 91.75% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + }; + // Match signedOrderLeft with signedOrderRight + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); - // Verify left order was fully filled - const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft); - expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); - // Verify right order was partially filled - const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight); - expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FILLABLE); }); - it('should transfer the correct amounts when right order is completely filled and left order is partially filled', 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(50), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), + 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(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + 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), }); - // Match orders - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + // 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(16), 0), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(22), 0), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + 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(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, ); - // Verify left order was partially filled - const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft); - expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FILLABLE); - // Verify right order was fully filled - const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight); - expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); }); - it('should transfer the correct amounts when consecutive calls are used to completely fill the left order', async () => { + 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(50), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), + 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 + await matchOrderTester.matchOrdersAndAssertEffectsAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + expectedTransferAmounts, + ); + }); + + 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}`; + it(description, async () => { + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAddress: makerAddressRight, + takerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + feeRecipientAddress: feeRecipientAddressRight, + }); + await web3Wrapper.awaitTransactionSuccessAsync( + await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await expectTransactionFailedAsync( + exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress), + RevertReason.TransferFailed, + ); + }); + }); + }; + describe('matchOrders reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX)); + + it('should transfer the correct amounts when orders completely fill each other', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + }; + await matchOrderTester.matchOrdersAndAssertEffectsAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + expectedTransferAmounts, + ); + }); + + it('should transfer the correct amounts when orders completely fill each other and taker doesnt take a profit', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(0), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + }; + // Match signedOrderLeft with signedOrderRight + await matchOrderTester.matchOrdersAndAssertEffectsAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + expectedTransferAmounts, + ); + }); + + it('should transfer the correct amounts when left order is completely filled and right order is partially filled', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 16), // 50% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 16), // 50% + }; + // Match signedOrderLeft with signedOrderRight + await matchOrderTester.matchOrdersAndAssertEffectsAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + expectedTransferAmounts, + ); + }); + + it('should transfer the correct amounts when right order is completely filled and left order is partially filled', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 16), // 10% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 16), // 10% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + }; + // Match signedOrderLeft with signedOrderRight + await matchOrderTester.matchOrdersAndAssertEffectsAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + expectedTransferAmounts, + ); + }); + + it('should transfer the correct amounts when consecutive calls are used to completely fill the left order', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - feeRecipientAddress: feeRecipientAddressRight, }); // Match orders let newERC20BalancesByOwner: ERC20BalancesByOwner; let newERC721TokenIdsByOwner: ERC721TokenIdsByOwner; + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 16), // 10% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 16), // 10% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + }; // prettier-ignore [ newERC20BalancesByOwner, // tslint:disable-next-line:trailing-comma newERC721TokenIdsByOwner - ] = await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + ] = await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); - // Verify left order was partially filled - const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft); - expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FILLABLE); - // Verify right order was fully filled - const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight); - expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); // Construct second right order // Note: This order needs makerAssetAmount=90/takerAssetAmount=[anything <= 45] to fully fill the right order. // However, we use 100/50 to ensure a partial fill as we want to go down the "left fill" // branch in the contract twice for this test. const signedOrderRight2 = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18), - feeRecipientAddress: feeRecipientAddressRight, }); // Match signedOrderLeft with signedOrderRight2 const leftTakerAssetFilledAmount = signedOrderRight.makerAssetAmount; const rightTakerAssetFilledAmount = new BigNumber(0); - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + const expectedTransferAmounts2 = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(45), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 16), // 90% (10% paid earlier) + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(45), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 16), // 90% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(0), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 16), // 90% (10% paid earlier) + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 16), // 90% + }; + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight2, takerAddress, newERC20BalancesByOwner, - erc721TokenIdsByOwner, + newERC721TokenIdsByOwner, + expectedTransferAmounts2, leftTakerAssetFilledAmount, rightTakerAssetFilledAmount, ); - // Verify left order was fully filled - const leftOrderInfo2: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft); - expect(leftOrderInfo2.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); - // Verify second right order was partially filled - const rightOrderInfo2: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight2); - expect(rightOrderInfo2.orderStatus as OrderStatus).to.be.equal(OrderStatus.FILLABLE); }); it('should transfer the correct amounts when consecutive calls are used to completely fill the right order', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), - feeRecipientAddress: feeRecipientAddressRight, }); // Match orders let newERC20BalancesByOwner: ERC20BalancesByOwner; let newERC721TokenIdsByOwner: ERC721TokenIdsByOwner; + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 16), // 4% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(6), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 16), // 4% + }; // prettier-ignore [ newERC20BalancesByOwner, // tslint:disable-next-line:trailing-comma newERC721TokenIdsByOwner - ] = await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + ] = await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); - // Verify left order was partially filled - const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft); - expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); - // Verify right order was fully filled - const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight); - expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FILLABLE); + // Create second left order // Note: This order needs makerAssetAmount=96/takerAssetAmount=48 to fully fill the right order. // However, we use 100/50 to ensure a partial fill as we want to go down the "right fill" // branch in the contract twice for this test. const signedOrderLeft2 = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18), - feeRecipientAddress: feeRecipientAddressLeft, }); // Match signedOrderLeft2 with signedOrderRight const leftTakerAssetFilledAmount = new BigNumber(0); @@ -421,198 +860,257 @@ describe('matchOrders', () => { erc20BalancesByOwner[takerAddress][defaultERC20MakerAssetAddress], ); const rightTakerAssetFilledAmount = signedOrderLeft.makerAssetAmount.minus(takerAmountReceived); - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + const expectedTransferAmounts2 = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(96), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(48), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(96), 16), // 96% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(48), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(96), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(96), 16), // 96% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(0), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(96), 16), // 96% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(96), 16), // 96% + }; + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft2, signedOrderRight, takerAddress, newERC20BalancesByOwner, - erc721TokenIdsByOwner, + newERC721TokenIdsByOwner, + expectedTransferAmounts2, leftTakerAssetFilledAmount, rightTakerAssetFilledAmount, ); - // Verify second left order was partially filled - const leftOrderInfo2: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft2); - expect(leftOrderInfo2.orderStatus as OrderStatus).to.be.equal(OrderStatus.FILLABLE); - // Verify right order was fully filled - const rightOrderInfo2: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight); - expect(rightOrderInfo2.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); }); it('should transfer the correct amounts if fee recipient is the same across both matched orders', async () => { const feeRecipientAddress = feeRecipientAddressLeft; const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), feeRecipientAddress, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), feeRecipientAddress, }); // Match orders - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + }; + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); }); it('should transfer the correct amounts if taker is also the left order maker', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - feeRecipientAddress: feeRecipientAddressRight, }); // Match orders takerAddress = signedOrderLeft.makerAddress; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + }; + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); }); it('should transfer the correct amounts if taker is also the right order maker', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - feeRecipientAddress: feeRecipientAddressRight, }); // Match orders takerAddress = signedOrderRight.makerAddress; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + }; + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); }); it('should transfer the correct amounts if taker is also the left fee recipient', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - feeRecipientAddress: feeRecipientAddressRight, }); // Match orders takerAddress = feeRecipientAddressLeft; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + }; + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); }); it('should transfer the correct amounts if taker is also the right fee recipient', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - feeRecipientAddress: feeRecipientAddressRight, }); // Match orders takerAddress = feeRecipientAddressRight; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + }; + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); }); it('should transfer the correct amounts if left maker is the left fee recipient and right maker is the right fee recipient', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - feeRecipientAddress: makerAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - feeRecipientAddress: makerAddressRight, }); // Match orders - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + }; + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); }); it('Should throw if left order is not fillable', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - feeRecipientAddress: feeRecipientAddressRight, }); // Cancel left order await exchangeWrapper.cancelOrderAsync(signedOrderLeft, signedOrderLeft.makerAddress); @@ -626,18 +1124,12 @@ describe('matchOrders', () => { it('Should throw if right order is not fillable', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - feeRecipientAddress: feeRecipientAddressRight, }); // Cancel right order await exchangeWrapper.cancelOrderAsync(signedOrderRight, signedOrderRight.makerAddress); @@ -651,18 +1143,12 @@ describe('matchOrders', () => { it('should throw if there is not a positive spread', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), - feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(200), 18), - feeRecipientAddress: feeRecipientAddressRight, }); // Match orders return expectTransactionFailedAsync( @@ -674,18 +1160,13 @@ describe('matchOrders', () => { it('should throw if the left maker asset is not equal to the right taker asset ', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - feeRecipientAddress: feeRecipientAddressRight, }); // Match orders return expectTransactionFailedAsync( @@ -701,20 +1182,13 @@ describe('matchOrders', () => { it('should throw if the right maker asset is not equal to the left taker asset', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - feeRecipientAddress: feeRecipientAddressRight, }); // Match orders return expectTransactionFailedAsync( @@ -727,70 +1201,76 @@ describe('matchOrders', () => { // Create orders to match const erc721TokenToTransfer = erc721LeftMakerAssetIds[0]; const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, makerAssetData: assetDataUtils.encodeERC721AssetData(defaultERC721AssetAddress, erc721TokenToTransfer), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), makerAssetAmount: new BigNumber(1), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), takerAssetData: assetDataUtils.encodeERC721AssetData(defaultERC721AssetAddress, erc721TokenToTransfer), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: new BigNumber(1), - feeRecipientAddress: feeRecipientAddressRight, }); // Match orders - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(0), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 50% + }; + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); - // Verify left order was fully filled - const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft); - expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); - // Verify right order was fully filled - const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight); - expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); }); it('should transfer correct amounts when right order maker asset is an ERC721 token', async () => { // Create orders to match const erc721TokenToTransfer = erc721RightMakerAssetIds[0]; const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), takerAssetData: assetDataUtils.encodeERC721AssetData(defaultERC721AssetAddress, erc721TokenToTransfer), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: new BigNumber(1), - feeRecipientAddress: feeRecipientAddressLeft, }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, makerAssetData: assetDataUtils.encodeERC721AssetData(defaultERC721AssetAddress, erc721TokenToTransfer), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetAmount: new BigNumber(1), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - feeRecipientAddress: feeRecipientAddressRight, + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(8), 18), }); // Match orders - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(8), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + }; + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); - // Verify left order was fully filled - const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft); - expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); - // Verify right order was fully filled - const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight); - expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); }); }); }); // tslint:disable-line:max-file-line-count diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index 62aba45b8..756c72766 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -1,6 +1,6 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { addSignedMessagePrefix, assetDataUtils, orderHashUtils } from '@0xproject/order-utils'; -import { RevertReason, SignatureType, SignedOrder, SignerType } from '@0xproject/types'; +import { BlockchainLifecycle } from '@0x/dev-utils'; +import { assetDataUtils, orderHashUtils, signatureUtils } from '@0x/order-utils'; +import { RevertReason, SignatureType, SignedOrder } from '@0x/types'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); @@ -8,12 +8,13 @@ import ethUtil = require('ethereumjs-util'); import { TestSignatureValidatorContract, TestSignatureValidatorSignatureValidatorApprovalEventArgs, -} from '../../generated_contract_wrappers/test_signature_validator'; -import { ValidatorContract } from '../../generated_contract_wrappers/validator'; -import { WalletContract } from '../../generated_contract_wrappers/wallet'; +} from '../../generated-wrappers/test_signature_validator'; +import { TestStaticCallReceiverContract } from '../../generated-wrappers/test_static_call_receiver'; +import { ValidatorContract } from '../../generated-wrappers/validator'; +import { WalletContract } from '../../generated-wrappers/wallet'; +import { artifacts } from '../../src/artifacts'; import { addressUtils } from '../utils/address_utils'; -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 { LogDecoder } from '../utils/log_decoder'; @@ -31,6 +32,8 @@ describe('MixinSignatureValidator', () => { let signatureValidator: TestSignatureValidatorContract; let testWallet: WalletContract; let testValidator: ValidatorContract; + let maliciousWallet: TestStaticCallReceiverContract; + let maliciousValidator: TestStaticCallReceiverContract; let signerAddress: string; let signerPrivateKey: Buffer; let notSignerAddress: string; @@ -65,6 +68,11 @@ describe('MixinSignatureValidator', () => { txDefaults, signerAddress, ); + maliciousWallet = maliciousValidator = await TestStaticCallReceiverContract.deployFrom0xArtifactAsync( + artifacts.TestStaticCallReceiver, + provider, + txDefaults, + ); signatureValidatorLogDecoder = new LogDecoder(web3Wrapper); await web3Wrapper.awaitTransactionSuccessAsync( await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(testValidator.address, true, { @@ -72,6 +80,16 @@ describe('MixinSignatureValidator', () => { }), constants.AWAIT_TRANSACTION_MINED_MS, ); + await web3Wrapper.awaitTransactionSuccessAsync( + await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync( + maliciousValidator.address, + true, + { + from: signerAddress, + }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); const defaultOrderParams = { ...constants.STATIC_ORDER_PARAMS, @@ -101,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, @@ -115,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, @@ -128,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, @@ -155,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, @@ -213,7 +231,7 @@ describe('MixinSignatureValidator', () => { it('should return true when SignatureType=EthSign and signature is valid', async () => { // Create EthSign signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, SignerType.Default); + const orderHashWithEthSignPrefixHex = signatureUtils.addSignedMessagePrefix(orderHashHex); const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey); // Create 0x signature from EthSign signature @@ -236,7 +254,7 @@ describe('MixinSignatureValidator', () => { it('should return false when SignatureType=EthSign and signature is invalid', async () => { // Create EthSign signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, SignerType.Default); + const orderHashWithEthSignPrefixHex = signatureUtils.addSignedMessagePrefix(orderHashHex); const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey); // Create 0x signature from EthSign signature @@ -257,32 +275,6 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); - it('should return true when SignatureType=Caller and signer is caller', async () => { - const signature = ethUtil.toBuffer(`0x${SignatureType.Caller}`); - const signatureHex = ethUtil.bufferToHex(signature); - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - signerAddress, - signatureHex, - { from: signerAddress }, - ); - expect(isValidSignature).to.be.true(); - }); - - it('should return false when SignatureType=Caller and signer is not caller', async () => { - const signature = ethUtil.toBuffer(`0x${SignatureType.Caller}`); - const signatureHex = ethUtil.bufferToHex(signature); - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - signerAddress, - signatureHex, - { from: notSignerAddress }, - ); - expect(isValidSignature).to.be.false(); - }); - it('should return true when SignatureType=Wallet and signature is valid', async () => { // Create EIP712 signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); @@ -328,6 +320,29 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); + it('should revert when `isValidSignature` attempts to update state and SignatureType=Wallet', async () => { + // Create EIP712 signature + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashBuffer = ethUtil.toBuffer(orderHashHex); + const ecSignature = ethUtil.ecsign(orderHashBuffer, signerPrivateKey); + // Create 0x signature from EIP712 signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.Wallet}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + await expectContractCallFailedAsync( + signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + maliciousWallet.address, + signatureHex, + ), + RevertReason.WalletError, + ); + }); + it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => { const validatorAddress = ethUtil.toBuffer(`${testValidator.address}`); const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`); @@ -358,6 +373,17 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); + it('should revert when `isValidSignature` attempts to update state and SignatureType=Validator', async () => { + const validatorAddress = ethUtil.toBuffer(`${maliciousValidator.address}`); + const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`); + const signature = Buffer.concat([validatorAddress, signatureType]); + const signatureHex = ethUtil.bufferToHex(signature); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + await expectContractCallFailedAsync( + signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex), + RevertReason.ValidatorError, + ); + }); it('should return false when SignatureType=Validator, signature is valid and validator is not approved', async () => { // Set approval of signature validator to false await web3Wrapper.awaitTransactionSuccessAsync( @@ -382,53 +408,6 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); - it('should return true when SignatureType=Trezor and signature is valid', async () => { - // Create Trezor signature - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, SignerType.Trezor); - const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); - const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); - // Create 0x signature from Trezor signature - const signature = Buffer.concat([ - ethUtil.toBuffer(ecSignature.v), - ecSignature.r, - ecSignature.s, - ethUtil.toBuffer(`0x${SignatureType.Trezor}`), - ]); - const signatureHex = ethUtil.bufferToHex(signature); - // Validate signature - const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - signerAddress, - signatureHex, - ); - expect(isValidSignature).to.be.true(); - }); - - it('should return false when SignatureType=Trezor and signature is invalid', async () => { - // Create Trezor signature - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, SignerType.Trezor); - const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); - const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); - // Create 0x signature from Trezor signature - const signature = Buffer.concat([ - ethUtil.toBuffer(ecSignature.v), - ecSignature.r, - ecSignature.s, - ethUtil.toBuffer(`0x${SignatureType.Trezor}`), - ]); - const signatureHex = ethUtil.bufferToHex(signature); - // Validate signature. - // This will fail because `signerAddress` signed the message, but we're passing in `notSignerAddress` - const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - notSignerAddress, - signatureHex, - ); - expect(isValidSignature).to.be.false(); - }); - it('should return true when SignatureType=Presigned and signer has presigned hash', async () => { // Presign hash const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); @@ -462,6 +441,42 @@ describe('MixinSignatureValidator', () => { ); expect(isValidSignature).to.be.false(); }); + + it('should return true when message was signed by a Trezor One (firmware version 1.6.2)', async () => { + // messageHash translates to 0x2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b + const messageHash = ethUtil.bufferToHex(ethUtil.toBuffer('++++++++++++++++++++++++++++++++')); + const signer = '0xc28b145f10f0bcf0fc000e778615f8fd73490bad'; + const v = ethUtil.toBuffer('0x1c'); + const r = ethUtil.toBuffer('0x7b888b596ccf87f0bacab0dcb483124973f7420f169b4824d7a12534ac1e9832'); + const s = ethUtil.toBuffer('0x0c8e14f7edc01459e13965f1da56e0c23ed11e2cca932571eee1292178f90424'); + const trezorSignatureType = ethUtil.toBuffer(`0x${SignatureType.EthSign}`); + const signature = Buffer.concat([v, r, s, trezorSignatureType]); + const signatureHex = ethUtil.bufferToHex(signature); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + messageHash, + signer, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return true when message was signed by a Trezor Model T (firmware version 2.0.7)', async () => { + // messageHash translates to 0x2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b + const messageHash = ethUtil.bufferToHex(ethUtil.toBuffer('++++++++++++++++++++++++++++++++')); + const signer = '0x98ce6d9345e8ffa7d99ee0822272fae9d2c0e895'; + const v = ethUtil.toBuffer('0x1c'); + const r = ethUtil.toBuffer('0x423b71062c327f0ec4fe199b8da0f34185e59b4c1cb4cc23df86cac4a601fb3f'); + const s = ethUtil.toBuffer('0x53810d6591b5348b7ee08ee812c874b0fdfb942c9849d59512c90e295221091f'); + const trezorSignatureType = ethUtil.toBuffer(`0x${SignatureType.EthSign}`); + const signature = Buffer.concat([v, r, s, trezorSignatureType]); + const signatureHex = ethUtil.bufferToHex(signature); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + messageHash, + signer, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); }); describe('setSignatureValidatorApproval', () => { diff --git a/packages/contracts/test/exchange/transactions.ts b/packages/contracts/test/exchange/transactions.ts index 2bdd96b16..1b5eef295 100644 --- a/packages/contracts/test/exchange/transactions.ts +++ b/packages/contracts/test/exchange/transactions.ts @@ -1,16 +1,16 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetDataUtils, generatePseudoRandomSalt } from '@0xproject/order-utils'; -import { OrderWithoutExchangeAddress, RevertReason, SignedOrder } from '@0xproject/types'; -import { BigNumber } from '@0xproject/utils'; +import { BlockchainLifecycle } from '@0x/dev-utils'; +import { assetDataUtils, generatePseudoRandomSalt } from '@0x/order-utils'; +import { OrderWithoutExchangeAddress, RevertReason, SignedOrder } from '@0x/types'; +import { BigNumber } from '@0x/utils'; import * as chai from 'chai'; import * as _ from 'lodash'; -import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token'; -import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; -import { ExchangeContract } from '../../generated_contract_wrappers/exchange'; -import { ExchangeWrapperContract } from '../../generated_contract_wrappers/exchange_wrapper'; -import { WhitelistContract } from '../../generated_contract_wrappers/whitelist'; -import { artifacts } from '../utils/artifacts'; +import { DummyERC20TokenContract } from '../../generated-wrappers/dummy_erc20_token'; +import { ERC20ProxyContract } from '../../generated-wrappers/erc20_proxy'; +import { ExchangeContract } from '../../generated-wrappers/exchange'; +import { ExchangeWrapperContract } from '../../generated-wrappers/exchange_wrapper'; +import { WhitelistContract } from '../../generated-wrappers/whitelist'; +import { artifacts } from '../../src/artifacts'; import { expectTransactionFailedAsync } from '../utils/assertions'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts index d48441dca..6b660aac5 100644 --- a/packages/contracts/test/exchange/wrapper.ts +++ b/packages/contracts/test/exchange/wrapper.ts @@ -1,17 +1,18 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetDataUtils, orderHashUtils } from '@0xproject/order-utils'; -import { RevertReason, SignedOrder } from '@0xproject/types'; -import { BigNumber } from '@0xproject/utils'; -import { Web3Wrapper } from '@0xproject/web3-wrapper'; +import { BlockchainLifecycle } from '@0x/dev-utils'; +import { assetDataUtils, orderHashUtils } from '@0x/order-utils'; +import { RevertReason, SignedOrder } from '@0x/types'; +import { BigNumber } from '@0x/utils'; +import { Web3Wrapper } from '@0x/web3-wrapper'; import * as chai from 'chai'; import * as _ from 'lodash'; -import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token'; -import { DummyERC721TokenContract } from '../../generated_contract_wrappers/dummy_erc721_token'; -import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; -import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; -import { ExchangeContract } from '../../generated_contract_wrappers/exchange'; -import { artifacts } from '../utils/artifacts'; +import { DummyERC20TokenContract } from '../../generated-wrappers/dummy_erc20_token'; +import { DummyERC721TokenContract } from '../../generated-wrappers/dummy_erc721_token'; +import { ERC20ProxyContract } from '../../generated-wrappers/erc20_proxy'; +import { ERC721ProxyContract } from '../../generated-wrappers/erc721_proxy'; +import { ExchangeContract } from '../../generated-wrappers/exchange'; +import { ReentrantERC20TokenContract } from '../../generated-wrappers/reentrant_erc20_token'; +import { artifacts } from '../../src/artifacts'; import { expectTransactionFailedAsync } from '../utils/assertions'; import { getLatestBlockTimestampAsync, increaseTimeAndMineBlockAsync } from '../utils/block_timestamp'; import { chaiSetup } from '../utils/chai_setup'; @@ -40,6 +41,7 @@ describe('Exchange wrappers', () => { let exchange: ExchangeContract; let erc20Proxy: ERC20ProxyContract; let erc721Proxy: ERC721ProxyContract; + let reentrantErc20Token: ReentrantERC20TokenContract; let exchangeWrapper: ExchangeWrapper; let erc20Wrapper: ERC20Wrapper; @@ -104,6 +106,13 @@ describe('Exchange wrappers', () => { constants.AWAIT_TRANSACTION_MINED_MS, ); + reentrantErc20Token = await ReentrantERC20TokenContract.deployFrom0xArtifactAsync( + artifacts.ReentrantERC20Token, + provider, + txDefaults, + exchange.address, + ); + defaultMakerAssetAddress = erc20TokenA.address; defaultTakerAssetAddress = erc20TokenB.address; @@ -126,6 +135,26 @@ describe('Exchange wrappers', () => { await blockchainLifecycle.revertAsync(); }); describe('fillOrKillOrder', () => { + const reentrancyTest = (functionNames: string[]) => { + _.forEach(functionNames, async (functionName: string, functionId: number) => { + const description = `should not allow fillOrKillOrder to reenter the Exchange contract via ${functionName}`; + it(description, async () => { + const signedOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address), + }); + await web3Wrapper.awaitTransactionSuccessAsync( + await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await expectTransactionFailedAsync( + exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress), + RevertReason.TransferFailed, + ); + }); + }); + }; + describe('fillOrKillOrder reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX)); + it('should transfer the correct amounts', async () => { const signedOrder = await orderFactory.newSignedOrderAsync({ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), @@ -197,6 +226,25 @@ describe('Exchange wrappers', () => { }); describe('fillOrderNoThrow', () => { + const reentrancyTest = (functionNames: string[]) => { + _.forEach(functionNames, async (functionName: string, functionId: number) => { + const description = `should not allow fillOrderNoThrow to reenter the Exchange contract via ${functionName}`; + it(description, async () => { + const signedOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address), + }); + await web3Wrapper.awaitTransactionSuccessAsync( + await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(erc20Balances).to.deep.equal(newBalances); + }); + }); + }; + describe('fillOrderNoThrow reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX)); + it('should transfer the correct amounts', async () => { const signedOrder = await orderFactory.newSignedOrderAsync({ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), @@ -397,6 +445,26 @@ describe('Exchange wrappers', () => { }); describe('batchFillOrders', () => { + const reentrancyTest = (functionNames: string[]) => { + _.forEach(functionNames, async (functionName: string, functionId: number) => { + const description = `should not allow batchFillOrders to reenter the Exchange contract via ${functionName}`; + it(description, async () => { + const signedOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address), + }); + await web3Wrapper.awaitTransactionSuccessAsync( + await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await expectTransactionFailedAsync( + exchangeWrapper.batchFillOrdersAsync([signedOrder], takerAddress), + RevertReason.TransferFailed, + ); + }); + }); + }; + describe('batchFillOrders reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX)); + it('should transfer the correct amounts', async () => { const takerAssetFillAmounts: BigNumber[] = []; const makerAssetAddress = erc20TokenA.address; @@ -446,6 +514,26 @@ describe('Exchange wrappers', () => { }); describe('batchFillOrKillOrders', () => { + const reentrancyTest = (functionNames: string[]) => { + _.forEach(functionNames, async (functionName: string, functionId: number) => { + const description = `should not allow batchFillOrKillOrders to reenter the Exchange contract via ${functionName}`; + it(description, async () => { + const signedOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address), + }); + await web3Wrapper.awaitTransactionSuccessAsync( + await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await expectTransactionFailedAsync( + exchangeWrapper.batchFillOrKillOrdersAsync([signedOrder], takerAddress), + RevertReason.TransferFailed, + ); + }); + }); + }; + describe('batchFillOrKillOrders reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX)); + it('should transfer the correct amounts', async () => { const takerAssetFillAmounts: BigNumber[] = []; const makerAssetAddress = erc20TokenA.address; @@ -512,6 +600,25 @@ describe('Exchange wrappers', () => { }); describe('batchFillOrdersNoThrow', async () => { + const reentrancyTest = (functionNames: string[]) => { + _.forEach(functionNames, async (functionName: string, functionId: number) => { + const description = `should not allow batchFillOrdersNoThrow to reenter the Exchange contract via ${functionName}`; + it(description, async () => { + const signedOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address), + }); + await web3Wrapper.awaitTransactionSuccessAsync( + await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await exchangeWrapper.batchFillOrdersNoThrowAsync([signedOrder], takerAddress); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(erc20Balances).to.deep.equal(newBalances); + }); + }); + }; + describe('batchFillOrdersNoThrow reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX)); + it('should transfer the correct amounts', async () => { const takerAssetFillAmounts: BigNumber[] = []; const makerAssetAddress = erc20TokenA.address; @@ -625,6 +732,28 @@ describe('Exchange wrappers', () => { }); describe('marketSellOrders', () => { + const reentrancyTest = (functionNames: string[]) => { + _.forEach(functionNames, async (functionName: string, functionId: number) => { + const description = `should not allow marketSellOrders to reenter the Exchange contract via ${functionName}`; + it(description, async () => { + const signedOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address), + }); + await web3Wrapper.awaitTransactionSuccessAsync( + await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await expectTransactionFailedAsync( + exchangeWrapper.marketSellOrdersAsync([signedOrder], takerAddress, { + takerAssetFillAmount: signedOrder.takerAssetAmount, + }), + RevertReason.TransferFailed, + ); + }); + }); + }; + describe('marketSellOrders reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX)); + it('should stop when the entire takerAssetFillAmount is filled', async () => { const takerAssetFillAmount = signedOrders[0].takerAssetAmount.plus( signedOrders[1].takerAssetAmount.div(2), @@ -717,6 +846,27 @@ describe('Exchange wrappers', () => { }); describe('marketSellOrdersNoThrow', () => { + const reentrancyTest = (functionNames: string[]) => { + _.forEach(functionNames, async (functionName: string, functionId: number) => { + const description = `should not allow marketSellOrdersNoThrow to reenter the Exchange contract via ${functionName}`; + it(description, async () => { + const signedOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address), + }); + await web3Wrapper.awaitTransactionSuccessAsync( + await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await exchangeWrapper.marketSellOrdersNoThrowAsync([signedOrder], takerAddress, { + takerAssetFillAmount: signedOrder.takerAssetAmount, + }); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(erc20Balances).to.deep.equal(newBalances); + }); + }); + }; + describe('marketSellOrdersNoThrow reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX)); + it('should stop when the entire takerAssetFillAmount is filled', async () => { const takerAssetFillAmount = signedOrders[0].takerAssetAmount.plus( signedOrders[1].takerAssetAmount.div(2), @@ -843,6 +993,28 @@ describe('Exchange wrappers', () => { }); describe('marketBuyOrders', () => { + const reentrancyTest = (functionNames: string[]) => { + _.forEach(functionNames, async (functionName: string, functionId: number) => { + const description = `should not allow marketBuyOrders to reenter the Exchange contract via ${functionName}`; + it(description, async () => { + const signedOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address), + }); + await web3Wrapper.awaitTransactionSuccessAsync( + await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await expectTransactionFailedAsync( + exchangeWrapper.marketBuyOrdersAsync([signedOrder], takerAddress, { + makerAssetFillAmount: signedOrder.makerAssetAmount, + }), + RevertReason.TransferFailed, + ); + }); + }); + }; + describe('marketBuyOrders reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX)); + it('should stop when the entire makerAssetFillAmount is filled', async () => { const makerAssetFillAmount = signedOrders[0].makerAssetAmount.plus( signedOrders[1].makerAssetAmount.div(2), @@ -933,6 +1105,27 @@ describe('Exchange wrappers', () => { }); describe('marketBuyOrdersNoThrow', () => { + const reentrancyTest = (functionNames: string[]) => { + _.forEach(functionNames, async (functionName: string, functionId: number) => { + const description = `should not allow marketBuyOrdersNoThrow to reenter the Exchange contract via ${functionName}`; + it(description, async () => { + const signedOrder = await orderFactory.newSignedOrderAsync({ + makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address), + }); + await web3Wrapper.awaitTransactionSuccessAsync( + await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await exchangeWrapper.marketBuyOrdersNoThrowAsync([signedOrder], takerAddress, { + makerAssetFillAmount: signedOrder.makerAssetAmount, + }); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(erc20Balances).to.deep.equal(newBalances); + }); + }); + }; + describe('marketBuyOrdersNoThrow reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX)); + it('should stop when the entire makerAssetFillAmount is filled', async () => { const makerAssetFillAmount = signedOrders[0].makerAssetAmount.plus( signedOrders[1].makerAssetAmount.div(2), |