From 407f63ef2055e8cd01af010a2e6d3a6c548c9793 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 20 Aug 2018 13:13:38 -0700 Subject: Removed calculateFillResults from matchOrders workflow. Eliminates compounded rounding errors. --- .../2.0.0/protocol/Exchange/MixinMatchOrders.sol | 67 ++++++++++------------ 1 file changed, 29 insertions(+), 38 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index 25220a673..2012aa6db 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -109,7 +109,7 @@ contract MixinMatchOrders is rightOrderInfo.orderTakerAssetFilledAmount, matchedFillResults.right ); - + // Settle matched orders. Succeeds or throws. settleMatchedOrders( leftOrder, @@ -169,52 +169,43 @@ contract MixinMatchOrders is // The amount saved by the left maker goes to the taker. // Either the left or right order will be fully filled; possibly both. // The left order is fully filled iff the right order can sell more than left can buy. - // That is: the amount required to fill the left order is less than or equal to - // the amount we can spend from the right order: - // <= * - // <= * / - // * <= * + + // Derive maker asset amounts for left & right orders, given store taker assert amounts uint256 leftTakerAssetAmountRemaining = safeSub(leftOrder.takerAssetAmount, leftOrderTakerAssetFilledAmount); + uint256 leftMakerAssetAmountRemaining = getPartialAmountFloor( + leftOrder.makerAssetAmount, + leftOrder.takerAssetAmount, + leftTakerAssetAmountRemaining + ); uint256 rightTakerAssetAmountRemaining = safeSub(rightOrder.takerAssetAmount, rightOrderTakerAssetFilledAmount); - uint256 leftTakerAssetFilledAmount; - uint256 rightTakerAssetFilledAmount; - if ( - safeMul(leftTakerAssetAmountRemaining, rightOrder.takerAssetAmount) <= - safeMul(rightTakerAssetAmountRemaining, rightOrder.makerAssetAmount) - ) { - // Left order will be fully filled: maximally fill left - leftTakerAssetFilledAmount = leftTakerAssetAmountRemaining; + uint256 rightMakerAssetAmountRemaining = getPartialAmountFloor( + rightOrder.makerAssetAmount, + rightOrder.takerAssetAmount, + rightTakerAssetAmountRemaining + ); - // The right order receives an amount proportional to how much was spent. - rightTakerAssetFilledAmount = getPartialAmountFloor( - rightOrder.takerAssetAmount, - rightOrder.makerAssetAmount, - leftTakerAssetFilledAmount + if (leftTakerAssetAmountRemaining >= rightMakerAssetAmountRemaining) { + // Case 1: Right order is fully filled: maximally fill right + matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining; + matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining; + matchedFillResults.left.makerAssetFilledAmount = getPartialAmountFloor( + leftOrder.makerAssetAmount, + leftOrder.takerAssetAmount, + matchedFillResults.right.makerAssetFilledAmount ); + matchedFillResults.left.takerAssetFilledAmount = matchedFillResults.right.makerAssetFilledAmount; } else { - // Right order will be fully filled: maximally fill right - rightTakerAssetFilledAmount = rightTakerAssetAmountRemaining; - - // The left order receives an amount proportional to how much was spent. - leftTakerAssetFilledAmount = getPartialAmountFloor( - rightOrder.makerAssetAmount, + // Case 2: Left order is fully filled: maximall fill left + matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining; + matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining; + matchedFillResults.right.makerAssetFilledAmount = matchedFillResults.left.takerAssetFilledAmount; + matchedFillResults.right.takerAssetFilledAmount = getPartialAmountCeil( rightOrder.takerAssetAmount, - rightTakerAssetFilledAmount + rightOrder.makerAssetAmount, + matchedFillResults.left.takerAssetFilledAmount ); } - // Calculate fill results for left order - matchedFillResults.left = calculateFillResults( - leftOrder, - leftTakerAssetFilledAmount - ); - - // Calculate fill results for right order - matchedFillResults.right = calculateFillResults( - rightOrder, - rightTakerAssetFilledAmount - ); - // Calculate amount given to taker matchedFillResults.leftMakerAssetSpreadAmount = safeSub( matchedFillResults.left.makerAssetFilledAmount, -- cgit v1.2.3 From 057891b342cf639b493adcf0a136f7ee421aaaf9 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 20 Aug 2018 13:22:21 -0700 Subject: Added fees to matchOrders (previously in calculateFillResults --- .../2.0.0/protocol/Exchange/MixinMatchOrders.sol | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'packages/contracts') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index 2012aa6db..58f131819 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -212,6 +212,30 @@ contract MixinMatchOrders is matchedFillResults.right.takerAssetFilledAmount ); + // Compute fees for left order + matchedFillResults.left.makerFeePaid = getPartialAmount( + matchedFillResults.left.takerAssetFilledAmount, + leftOrder.takerAssetAmount, + leftOrder.makerFee + ); + matchedFillResults.left.takerFeePaid = getPartialAmount( + matchedFillResults.left.takerAssetFilledAmount, + leftOrder.takerAssetAmount, + leftOrder.takerFee + ); + + // Compute fees for right order + matchedFillResults.right.makerFeePaid = getPartialAmount( + matchedFillResults.right.takerAssetFilledAmount, + rightOrder.takerAssetAmount, + rightOrder.makerFee + ); + matchedFillResults.right.takerFeePaid = getPartialAmount( + matchedFillResults.right.takerAssetFilledAmount, + rightOrder.takerAssetAmount, + rightOrder.takerFee + ); + // Return fill results return matchedFillResults; } -- cgit v1.2.3 From 0ecdf1e2132141b199fa929ec1c6ca9979f06302 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 20 Aug 2018 14:02:40 -0700 Subject: All existing tests pass. --- packages/contracts/test/exchange/match_orders.ts | 105 ++++++++++++++++++++- .../contracts/test/utils/match_order_tester.ts | 53 +++++++---- 2 files changed, 137 insertions(+), 21 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 8cd873a85..78f33ec7c 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -28,7 +28,7 @@ chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -describe('matchOrders', () => { +describe.only('matchOrders', () => { let makerAddressLeft: string; let makerAddressRight: string; let owner: string; @@ -176,6 +176,109 @@ describe('matchOrders', () => { erc20BalancesByOwner = await erc20Wrapper.getBalancesAsync(); erc721TokenIdsByOwner = await erc721Wrapper.getBalancesAsync(); }); + + + /* + it.only('should transfer the correct amounts when orders completely fill each other', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAddress: makerAddressLeft, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2000), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1001), 0), + feeRecipientAddress: feeRecipientAddressLeft, + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAddress: makerAddressRight, + makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), + takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10000), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(3000), 0), + feeRecipientAddress: feeRecipientAddressRight, + }); + // Match signedOrderLeft with signedOrderRight + await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + ); + // // 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.only('Jacobs Example', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAddress: makerAddressLeft, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), + feeRecipientAddress: feeRecipientAddressLeft, + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAddress: makerAddressRight, + makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), + takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 0), + feeRecipientAddress: feeRecipientAddressRight, + }); + // Match signedOrderLeft with signedOrderRight + await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + ); + // // Verify left order was fully filled + const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft); + //expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); + console.log("*** LEFT ORDER INFO ***\n", JSON.stringify(leftOrderInfo), "\n***************"); + // Verify right order was fully filled + const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight); + // expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); + console.log("*** RIGHT ORDER INFO ***\n", JSON.stringify(rightOrderInfo), "\n***************"); + }); + + + /* + it.only('Jacobs Example', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAddress: makerAddressLeft, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), + feeRecipientAddress: feeRecipientAddressLeft, + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAddress: makerAddressRight, + makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), + takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), + feeRecipientAddress: feeRecipientAddressRight, + }); + // Match signedOrderLeft with signedOrderRight + await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + ); + // // 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); + });*/ const reentrancyTest = (functionNames: string[]) => { _.forEach(functionNames, async (functionName: string, functionId: number) => { diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index fa2eabc12..ded429322 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -9,6 +9,7 @@ import { ERC20Wrapper } from './erc20_wrapper'; import { ERC721Wrapper } from './erc721_wrapper'; import { ExchangeWrapper } from './exchange_wrapper'; import { ERC20BalancesByOwner, ERC721TokenIdsByOwner, TransferAmountsByMatchOrders as TransferAmounts } from './types'; +import { TransactionReceiptWithDecodedLogs } from '../../../../node_modules/ethereum-types'; chaiSetup.configure(); const expect = chai.expect; @@ -108,7 +109,7 @@ export class MatchOrderTester { : new BigNumber(0); expect(expectedOrderFilledAmountRight).to.be.bignumber.equal(orderTakerAssetFilledAmountRight); // Match left & right orders - await this._exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress); + const transactionReceipt = await this._exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress); const newERC20BalancesByOwner = await this._erc20Wrapper.getBalancesAsync(); const newERC721TokenIdsByOwner = await this._erc721Wrapper.getBalancesAsync(); // Calculate expected balance changes @@ -117,6 +118,8 @@ export class MatchOrderTester { signedOrderRight, orderTakerAssetFilledAmountLeft, orderTakerAssetFilledAmountRight, + transactionReceipt, + takerAddress ); let expectedERC20BalancesByOwner: ERC20BalancesByOwner; let expectedERC721TokenIdsByOwner: ERC721TokenIdsByOwner; @@ -135,6 +138,7 @@ export class MatchOrderTester { expectedERC721TokenIdsByOwner, newERC721TokenIdsByOwner, ); + expect(didExpectedBalancesMatchRealBalances).to.be.true(); return [newERC20BalancesByOwner, newERC721TokenIdsByOwner]; } @@ -150,26 +154,35 @@ export class MatchOrderTester { signedOrderRight: SignedOrder, orderTakerAssetFilledAmountLeft: BigNumber, orderTakerAssetFilledAmountRight: BigNumber, + transactionReceipt: TransactionReceiptWithDecodedLogs, + takerAddress: string ): Promise { - let amountBoughtByLeftMaker = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( - orderHashUtils.getOrderHashHex(signedOrderLeft), - ); - amountBoughtByLeftMaker = amountBoughtByLeftMaker.minus(orderTakerAssetFilledAmountLeft); - const amountSoldByLeftMaker = amountBoughtByLeftMaker - .times(signedOrderLeft.makerAssetAmount) - .dividedToIntegerBy(signedOrderLeft.takerAssetAmount); - const amountReceivedByRightMaker = amountBoughtByLeftMaker - .times(signedOrderRight.takerAssetAmount) - .dividedToIntegerBy(signedOrderRight.makerAssetAmount); - const amountReceivedByTaker = amountSoldByLeftMaker.minus(amountReceivedByRightMaker); - let amountBoughtByRightMaker = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( - orderHashUtils.getOrderHashHex(signedOrderRight), - ); - amountBoughtByRightMaker = amountBoughtByRightMaker.minus(orderTakerAssetFilledAmountRight); - const amountSoldByRightMaker = amountBoughtByRightMaker - .times(signedOrderRight.makerAssetAmount) - .dividedToIntegerBy(signedOrderRight.takerAssetAmount); - const amountReceivedByLeftMaker = amountSoldByRightMaker; + // Parse logs + expect(transactionReceipt.logs.length).to.be.equal(2); + // First log is for left fill + const leftLog = ((transactionReceipt.logs[0] as any) as {args: { makerAddress: string, takerAddress: string, makerAssetFilledAmount: string, takerAssetFilledAmount: string, makerFeePaid: string, takerFeePaid: string}}); + expect(leftLog.args.makerAddress).to.be.equal(signedOrderLeft.makerAddress); + expect(leftLog.args.takerAddress).to.be.equal(takerAddress); + const amountBoughtByLeftMaker = new BigNumber(leftLog.args.takerAssetFilledAmount); + const amountSoldByLeftMaker = new BigNumber(leftLog.args.makerAssetFilledAmount); + // Second log is for right fill + const rightLog = ((transactionReceipt.logs[1] as any) as {args: { makerAddress: string, takerAddress: string, makerAssetFilledAmount: string, takerAssetFilledAmount: string, makerFeePaid: string, takerFeePaid: string}}); + expect(rightLog.args.makerAddress).to.be.equal(signedOrderRight.makerAddress); + expect(rightLog.args.takerAddress).to.be.equal(takerAddress); + const amountBoughtByRightMaker = new BigNumber(rightLog.args.takerAssetFilledAmount); + const amountSoldByRightMaker = new BigNumber(rightLog.args.makerAssetFilledAmount); + // Determine amount received by taker + const amountReceivedByTaker = amountSoldByLeftMaker.sub(amountBoughtByRightMaker); + + const amountReceivedByLeftMaker = amountBoughtByLeftMaker; + const amountReceivedByRightMaker = amountBoughtByRightMaker; + + console.log("Amount bought by left maker = ", JSON.stringify(amountBoughtByLeftMaker)); + console.log("Amount sold by left maker = ", JSON.stringify(amountSoldByLeftMaker)); + console.log("Amount bought by right maker = ", JSON.stringify(amountBoughtByRightMaker)); + console.log("Amount sold by right maker = ", JSON.stringify(amountSoldByRightMaker)); + console.log("Amount received by taker = ", JSON.stringify(amountReceivedByTaker)); + //const amountReceivedByLeftMaker = amountSoldByRightMaker; const feePaidByLeftMaker = signedOrderLeft.makerFee .times(amountSoldByLeftMaker) .dividedToIntegerBy(signedOrderLeft.makerAssetAmount); -- cgit v1.2.3 From a32b201afe6c90a6bd06b88a5d512408cd3cb032 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 20 Aug 2018 17:00:08 -0700 Subject: Rounding for fees in match orders addressed, plus example --- .../2.0.0/protocol/Exchange/MixinMatchOrders.sol | 8 +-- .../contracts/test/utils/match_order_tester.ts | 84 ++++++++++++++++++++-- 2 files changed, 83 insertions(+), 9 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index 58f131819..b3f376eb8 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -214,8 +214,8 @@ contract MixinMatchOrders is // Compute fees for left order matchedFillResults.left.makerFeePaid = getPartialAmount( - matchedFillResults.left.takerAssetFilledAmount, - leftOrder.takerAssetAmount, + matchedFillResults.left.makerAssetFilledAmount, + leftOrder.makerAssetAmount, leftOrder.makerFee ); matchedFillResults.left.takerFeePaid = getPartialAmount( @@ -226,8 +226,8 @@ contract MixinMatchOrders is // Compute fees for right order matchedFillResults.right.makerFeePaid = getPartialAmount( - matchedFillResults.right.takerAssetFilledAmount, - rightOrder.takerAssetAmount, + matchedFillResults.right.makerAssetFilledAmount, + rightOrder.makerAssetAmount, rightOrder.makerFee ); matchedFillResults.right.takerFeePaid = getPartialAmount( diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index ded429322..5160b47d3 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -139,7 +139,78 @@ export class MatchOrderTester { newERC721TokenIdsByOwner, ); + // Compute actual transfer amounts + let actualTransferAmounts = {}; + const makerAssetProxyIdLeft = assetDataUtils.decodeAssetProxyId(signedOrderLeft.makerAssetData); + if (makerAssetProxyIdLeft === AssetProxyId.ERC20) { + const erc20AssetData = assetDataUtils.decodeERC20AssetData(signedOrderLeft.makerAssetData); + const makerAssetAddressLeft = erc20AssetData.tokenAddress; + actualTransferAmounts.amountSoldByLeftMaker = erc20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sub(newERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft]); + actualTransferAmounts.amountBoughtByRightMaker = newERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sub(erc20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft]); + actualTransferAmounts.amountReceivedByRightMaker = actualTransferAmounts.amountBoughtByRightMaker; + actualTransferAmounts.amountReceivedByTaker = newERC20BalancesByOwner[takerAddress][makerAssetAddressLeft].sub(erc20BalancesByOwner[takerAddress][makerAssetAddressLeft]); + } else if(makerAssetProxyIdLeft === AssetProxyId.ERC721) { + } + const makerAssetProxyIdRight = assetDataUtils.decodeAssetProxyId(signedOrderRight.makerAssetData); + if (makerAssetProxyIdRight === AssetProxyId.ERC20) { + const erc20AssetData = assetDataUtils.decodeERC20AssetData(signedOrderRight.makerAssetData); + const makerAssetAddressRight = erc20AssetData.tokenAddress; + actualTransferAmounts.amountSoldByRightMaker = erc20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight].sub(newERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]); + actualTransferAmounts.amountBoughtByLeftMaker = newERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sub(erc20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight]); + actualTransferAmounts.amountReceivedByLeftMaker = actualTransferAmounts.amountBoughtByLeftMaker; + } else if(makerAssetProxyIdRight === AssetProxyId.ERC721) { + } + // Fees + actualTransferAmounts.feePaidByLeftMaker = erc20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress].sub(newERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress]); + actualTransferAmounts.feePaidByRightMaker = erc20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress].sub(newERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress]); + actualTransferAmounts.totalFeePaidByTaker = erc20BalancesByOwner[takerAddress][this._feeTokenAddress].sub(newERC20BalancesByOwner[takerAddress][this._feeTokenAddress]); + + console.log("amountBoughtByLeftMaker"); + expect(expectedTransferAmounts.amountBoughtByLeftMaker).to.be.bignumber.equal(actualTransferAmounts.amountBoughtByLeftMaker); + console.log("amountSoldByLeftMaker"); + expect(expectedTransferAmounts.amountSoldByLeftMaker).to.be.bignumber.equal(actualTransferAmounts.amountSoldByLeftMaker); + console.log("amountBoughtByRightMaker"); + expect(expectedTransferAmounts.amountBoughtByRightMaker).to.be.bignumber.equal(actualTransferAmounts.amountBoughtByRightMaker); + console.log("amountSoldByRightMaker"); + expect(expectedTransferAmounts.amountSoldByRightMaker).to.be.bignumber.equal(actualTransferAmounts.amountSoldByRightMaker); + console.log("amountReceivedByTaker"); + expect(expectedTransferAmounts.amountReceivedByTaker).to.be.bignumber.equal(actualTransferAmounts.amountReceivedByTaker); + console.log("feePaidByLeftMaker"); + expect(expectedTransferAmounts.feePaidByLeftMaker).to.be.bignumber.equal(actualTransferAmounts.feePaidByLeftMaker); + console.log("feePaidByRightMaker"); + expect(expectedTransferAmounts.feePaidByRightMaker).to.be.bignumber.equal(actualTransferAmounts.feePaidByRightMaker); + console.log("totalFeePaidByTaker"); + expect(expectedTransferAmounts.totalFeePaidByTaker).to.be.bignumber.equal(actualTransferAmounts.totalFeePaidByTaker); + + + +/* + const actualTransferAmounts = { + // Left Maker + amountBoughtByLeftMaker, + amountSoldByLeftMaker, + amountReceivedByLeftMaker, + feePaidByLeftMaker, + // Right Maker + amountBoughtByRightMaker, + amountSoldByRightMaker, + amountReceivedByRightMaker, + feePaidByRightMaker, + // Taker + amountReceivedByTaker, + feePaidByTakerLeft, + feePaidByTakerRight, + totalFeePaidByTaker, + // Fee Recipients + feeReceivedLeft, + feeReceivedRight, + };*/ + + // This is a catch-all to ensure that no other balances changed + console.log("Catch-all"); expect(didExpectedBalancesMatchRealBalances).to.be.true(); + + return [newERC20BalancesByOwner, newERC721TokenIdsByOwner]; } /// @dev Calculates expected transfer amounts between order makers, fee recipients, and @@ -180,7 +251,7 @@ export class MatchOrderTester { console.log("Amount bought by left maker = ", JSON.stringify(amountBoughtByLeftMaker)); console.log("Amount sold by left maker = ", JSON.stringify(amountSoldByLeftMaker)); console.log("Amount bought by right maker = ", JSON.stringify(amountBoughtByRightMaker)); - console.log("Amount sold by right maker = ", JSON.stringify(amountSoldByRightMaker)); + console.log("Amount sold by right maker = ", JSON.stringify(amountSoldByRightMaker)); console.log("Amount received by taker = ", JSON.stringify(amountReceivedByTaker)); //const amountReceivedByLeftMaker = amountSoldByRightMaker; const feePaidByLeftMaker = signedOrderLeft.makerFee @@ -189,12 +260,15 @@ export class MatchOrderTester { const feePaidByRightMaker = signedOrderRight.makerFee .times(amountSoldByRightMaker) .dividedToIntegerBy(signedOrderRight.makerAssetAmount); + const feePaidByTakerLeft = signedOrderLeft.takerFee - .times(amountSoldByLeftMaker) - .dividedToIntegerBy(signedOrderLeft.makerAssetAmount); + .times(amountBoughtByLeftMaker) + .dividedToIntegerBy(signedOrderLeft.takerAssetAmount); + + const feePaidByTakerRight = signedOrderRight.takerFee - .times(amountSoldByRightMaker) - .dividedToIntegerBy(signedOrderRight.makerAssetAmount); + .times(amountBoughtByRightMaker) + .dividedToIntegerBy(signedOrderRight.takerAssetAmount); const totalFeePaidByTaker = feePaidByTakerLeft.add(feePaidByTakerRight); const feeReceivedLeft = feePaidByLeftMaker.add(feePaidByTakerLeft); const feeReceivedRight = feePaidByRightMaker.add(feePaidByTakerRight); -- cgit v1.2.3 From ca5c9e77c0f068cf653afc9c8b61bdc25318f8e2 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 21 Aug 2018 14:45:39 -0700 Subject: Ironing out the new set of test cases for order matchubng --- packages/contracts/test/exchange/match_orders.ts | 2 +- .../contracts/test/utils/match_order_tester.ts | 111 +++++++++++++++++++-- 2 files changed, 102 insertions(+), 11 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 78f33ec7c..b2ffa31b2 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -212,7 +212,7 @@ describe.only('matchOrders', () => { }); */ - it.only('Jacobs Example', async () => { + it('Jacobs Example', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ makerAddress: makerAddressLeft, diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index 5160b47d3..f9e45319a 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -57,6 +57,27 @@ export class MatchOrderTester { ); return doesErc721TokenIdsMatch; } + + /* + private static compareTokenIdLists(list1: BigNumber[], list2: BigNumber[]) { + // ERC721 Token Ids + list1 = _.(list1); + _.sortBy(tokenIds); + }); + }, + ); + const sortedNewERC721TokenIdsByOwner = _.mapValues(realERC721TokenIdsByOwner, tokenIdsByOwner => { + _.mapValues(tokenIdsByOwner, tokenIds => { + _.sortBy(tokenIds); + }); + }); + const doesErc721TokenIdsMatch = _.isEqual( + sortedExpectedNewERC721TokenIdsByOwner, + sortedNewERC721TokenIdsByOwner, + ); + return doesErc721TokenIdsMatch; + }*/ + /// @dev Constructs new MatchOrderTester. /// @param exchangeWrapper Used to call to the Exchange. /// @param erc20Wrapper Used to fetch ERC20 balances. @@ -139,6 +160,68 @@ export class MatchOrderTester { newERC721TokenIdsByOwner, ); + // Individual balance comparisons + const makerAssetProxyIdLeft = assetDataUtils.decodeAssetProxyId(signedOrderLeft.makerAssetData); + const makerERC20AssetDataLeft = makerAssetProxyIdLeft == AssetProxyId.ERC20 ? assetDataUtils.decodeERC20AssetData(signedOrderLeft.makerAssetData) : assetDataUtils.decodeERC721AssetData(signedOrderLeft.makerAssetData); + const makerAssetAddressLeft = makerERC20AssetDataLeft.tokenAddress; + const makerAssetProxyIdRight = assetDataUtils.decodeAssetProxyId(signedOrderRight.makerAssetData); + const makerERC20AssetDataRight = makerAssetProxyIdRight == AssetProxyId.ERC20 ? assetDataUtils.decodeERC20AssetData(signedOrderRight.makerAssetData) : assetDataUtils.decodeERC721AssetData(signedOrderRight.makerAssetData); + const makerAssetAddressRight = makerERC20AssetDataRight.tokenAddress; + + console.log("Left Maker: Sell Amount"); + if(makerAssetProxyIdLeft == AssetProxyId.ERC20) { + expect(newERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft]); + } else if(makerAssetProxyIdLeft == AssetProxyId.ERC721) { + expect(newERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort()).to.be.deep.equal(newERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort()); + } else { + throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdLeft}`); + } + + console.log("Left Maker: Buy Amount"); + if(makerAssetProxyIdRight == AssetProxyId.ERC20) { + expect(newERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight]); + } else if(makerAssetProxyIdRight == AssetProxyId.ERC721) { + expect(newERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort()).to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort()); + } else { + throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdRight}`); + } + + console.log("Left Maker: Fees"); + expect(newERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress]); + + console.log("Right Maker: Sell Amount"); + if(makerAssetProxyIdRight == AssetProxyId.ERC20) { + expect(newERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]); + } else if(makerAssetProxyIdRight == AssetProxyId.ERC721) { + expect(newERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]).to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]); + } else { + throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdRight}`); + } + + console.log("Right Maker: Buy Amount"); + if(makerAssetProxyIdLeft == AssetProxyId.ERC20) { + expect(newERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft]); + } else if(makerAssetProxyIdLeft == AssetProxyId.ERC721) { + expect(newERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort()).to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort()); + } else { + throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdLeft}`); + } + console.log("Right Maker: Fees"); + expect(newERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress]); + console.log("Taker: Receive Amount"); + if(makerAssetProxyIdLeft == AssetProxyId.ERC20) { + expect(newERC20BalancesByOwner[takerAddress][makerAssetAddressLeft]).to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][makerAssetAddressLeft]); + } else if(makerAssetProxyIdLeft == AssetProxyId.ERC721) { + expect(newERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort()).to.be.deep.equal(expectedERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort()); + } else { + throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdLeft}`); + } + console.log("Taker: Fees"); + expect(newERC20BalancesByOwner[takerAddress][this._feeTokenAddress]).to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][this._feeTokenAddress]); + console.log("Balance catch-all"); + expect(didExpectedBalancesMatchRealBalances).to.be.true(); + + /* // Compute actual transfer amounts let actualTransferAmounts = {}; const makerAssetProxyIdLeft = assetDataUtils.decodeAssetProxyId(signedOrderLeft.makerAssetData); @@ -163,24 +246,24 @@ export class MatchOrderTester { // Fees actualTransferAmounts.feePaidByLeftMaker = erc20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress].sub(newERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress]); actualTransferAmounts.feePaidByRightMaker = erc20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress].sub(newERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress]); - actualTransferAmounts.totalFeePaidByTaker = erc20BalancesByOwner[takerAddress][this._feeTokenAddress].sub(newERC20BalancesByOwner[takerAddress][this._feeTokenAddress]); + actualTransferAmounts.totalFeePaidByTaker = erc20BalancesByOwner[takerAddress][this._feeTokenAddress].sub(newERC20BalancesByOwner[takerAddress][this._feeTokenAddress]); console.log("amountBoughtByLeftMaker"); - expect(expectedTransferAmounts.amountBoughtByLeftMaker).to.be.bignumber.equal(actualTransferAmounts.amountBoughtByLeftMaker); + // expect(expectedTransferAmounts.amountBoughtByLeftMaker).to.be.bignumber.equal(actualTransferAmounts.amountBoughtByLeftMaker); console.log("amountSoldByLeftMaker"); - expect(expectedTransferAmounts.amountSoldByLeftMaker).to.be.bignumber.equal(actualTransferAmounts.amountSoldByLeftMaker); + // expect(expectedTransferAmounts.amountSoldByLeftMaker).to.be.bignumber.equal(actualTransferAmounts.amountSoldByLeftMaker); console.log("amountBoughtByRightMaker"); - expect(expectedTransferAmounts.amountBoughtByRightMaker).to.be.bignumber.equal(actualTransferAmounts.amountBoughtByRightMaker); + // expect(expectedTransferAmounts.amountBoughtByRightMaker).to.be.bignumber.equal(actualTransferAmounts.amountBoughtByRightMaker); console.log("amountSoldByRightMaker"); - expect(expectedTransferAmounts.amountSoldByRightMaker).to.be.bignumber.equal(actualTransferAmounts.amountSoldByRightMaker); + //expect(expectedTransferAmounts.amountSoldByRightMaker).to.be.bignumber.equal(actualTransferAmounts.amountSoldByRightMaker); console.log("amountReceivedByTaker"); - expect(expectedTransferAmounts.amountReceivedByTaker).to.be.bignumber.equal(actualTransferAmounts.amountReceivedByTaker); + //expect(expectedTransferAmounts.amountReceivedByTaker).to.be.bignumber.equal(actualTransferAmounts.amountReceivedByTaker); console.log("feePaidByLeftMaker"); - expect(expectedTransferAmounts.feePaidByLeftMaker).to.be.bignumber.equal(actualTransferAmounts.feePaidByLeftMaker); + // expect(expectedTransferAmounts.feePaidByLeftMaker).to.be.bignumber.equal(actualTransferAmounts.feePaidByLeftMaker); console.log("feePaidByRightMaker"); - expect(expectedTransferAmounts.feePaidByRightMaker).to.be.bignumber.equal(actualTransferAmounts.feePaidByRightMaker); + //expect(expectedTransferAmounts.feePaidByRightMaker).to.be.bignumber.equal(actualTransferAmounts.feePaidByRightMaker); console.log("totalFeePaidByTaker"); - expect(expectedTransferAmounts.totalFeePaidByTaker).to.be.bignumber.equal(actualTransferAmounts.totalFeePaidByTaker); + // expect(expectedTransferAmounts.totalFeePaidByTaker).to.be.bignumber.equal(actualTransferAmounts.totalFeePaidByTaker); @@ -204,11 +287,12 @@ export class MatchOrderTester { // Fee Recipients feeReceivedLeft, feeReceivedRight, - };*/ + }; // This is a catch-all to ensure that no other balances changed console.log("Catch-all"); expect(didExpectedBalancesMatchRealBalances).to.be.true(); + */ return [newERC20BalancesByOwner, newERC721TokenIdsByOwner]; @@ -351,6 +435,13 @@ export class MatchOrderTester { _.remove(expectedNewERC721TokenIdsByOwner[makerAddressLeft][makerAssetAddressLeft], makerAssetIdLeft); // Right Maker expectedNewERC721TokenIdsByOwner[makerAddressRight][takerAssetAddressRight].push(takerAssetIdRight); + console.log("*** TRANSFERINF: " + JSON.stringify(makerAssetIdLeft)); + console.log("****"); + console.log(JSON.stringify(expectedNewERC721TokenIdsByOwner[makerAddressLeft][makerAssetAddressLeft])); + console.log("****"); + console.log(JSON.stringify(expectedNewERC721TokenIdsByOwner[makerAddressRight][takerAssetAddressRight])); + console.log("****"); + // Taker: Since there is only 1 asset transferred, the taker does not receive any of the left maker asset. } // Left Taker Asset (Right Maker Asset) -- cgit v1.2.3 From f697814849142a8a98229e17433f942eef5c25d3 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Wed, 22 Aug 2018 14:05:44 -0700 Subject: First balance test with intentional values --- packages/contracts/test/exchange/match_orders.ts | 35 ++++-- .../contracts/test/utils/match_order_tester.ts | 119 ++++++++++++--------- 2 files changed, 95 insertions(+), 59 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index b2ffa31b2..3cffef7ad 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -21,10 +21,11 @@ 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, OrderInfo, TransferAmountsByMatchOrders as TransferAmounts, OrderStatus } from '../utils/types'; import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; chaiSetup.configure(); + const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); @@ -212,7 +213,7 @@ describe.only('matchOrders', () => { }); */ - it('Jacobs Example', async () => { + it.only('Jacobs Example', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ makerAddress: makerAddressLeft, @@ -228,6 +229,23 @@ describe.only('matchOrders', () => { takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 0), feeRecipientAddress: feeRecipientAddressRight, }); + // TODO: These values will change after implementation of rounding up has been merged + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 0), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 18), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 16), // 75% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(9), 0), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 18), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 17), // 50% + }; + const expectedEndStateLeft = OrderStatus.FILLABLE; + const expectedEndStateRight = OrderStatus.FULLY_FILLED; // Match signedOrderLeft with signedOrderRight await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, @@ -235,15 +253,10 @@ describe.only('matchOrders', () => { takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, + expectedEndStateLeft, + expectedEndStateRight ); - // // Verify left order was fully filled - const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft); - //expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); - console.log("*** LEFT ORDER INFO ***\n", JSON.stringify(leftOrderInfo), "\n***************"); - // Verify right order was fully filled - const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight); - // expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED); - console.log("*** RIGHT ORDER INFO ***\n", JSON.stringify(rightOrderInfo), "\n***************"); }); @@ -825,6 +838,6 @@ describe.only('matchOrders', () => { // 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/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index f9e45319a..7b071015f 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -8,7 +8,7 @@ import { chaiSetup } from './chai_setup'; import { ERC20Wrapper } from './erc20_wrapper'; import { ERC721Wrapper } from './erc721_wrapper'; import { ExchangeWrapper } from './exchange_wrapper'; -import { ERC20BalancesByOwner, ERC721TokenIdsByOwner, TransferAmountsByMatchOrders as TransferAmounts } from './types'; +import { ERC20BalancesByOwner, ERC721TokenIdsByOwner, TransferAmountsByMatchOrders as TransferAmounts , OrderStatus} from './types'; import { TransactionReceiptWithDecodedLogs } from '../../../../node_modules/ethereum-types'; chaiSetup.configure(); @@ -58,25 +58,30 @@ export class MatchOrderTester { return doesErc721TokenIdsMatch; } - /* - private static compareTokenIdLists(list1: BigNumber[], list2: BigNumber[]) { - // ERC721 Token Ids - list1 = _.(list1); - _.sortBy(tokenIds); - }); - }, + private async _verifyInitialOrderStates( + signedOrderLeft: SignedOrder, + signedOrderRight: SignedOrder, + initialTakerAssetFilledAmountLeft?: BigNumber, + initialTakerAssetFilledAmountRight?: BigNumber, + ): Promise<[BigNumber, BigNumber]> { + // Verify Left order preconditions + const orderTakerAssetFilledAmountLeft = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( + orderHashUtils.getOrderHashHex(signedOrderLeft), ); - const sortedNewERC721TokenIdsByOwner = _.mapValues(realERC721TokenIdsByOwner, tokenIdsByOwner => { - _.mapValues(tokenIdsByOwner, tokenIds => { - _.sortBy(tokenIds); - }); - }); - const doesErc721TokenIdsMatch = _.isEqual( - sortedExpectedNewERC721TokenIdsByOwner, - sortedNewERC721TokenIdsByOwner, + const expectedOrderFilledAmountLeft = initialTakerAssetFilledAmountLeft + ? initialTakerAssetFilledAmountLeft + : new BigNumber(0); + expect(expectedOrderFilledAmountLeft).to.be.bignumber.equal(orderTakerAssetFilledAmountLeft); + // Verify Right order preconditions + const orderTakerAssetFilledAmountRight = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( + orderHashUtils.getOrderHashHex(signedOrderRight), ); - return doesErc721TokenIdsMatch; - }*/ + const expectedOrderFilledAmountRight = initialTakerAssetFilledAmountRight + ? initialTakerAssetFilledAmountRight + : new BigNumber(0); + expect(expectedOrderFilledAmountRight).to.be.bignumber.equal(orderTakerAssetFilledAmountRight); + return [orderTakerAssetFilledAmountLeft, orderTakerAssetFilledAmountRight]; + } /// @dev Constructs new MatchOrderTester. /// @param exchangeWrapper Used to call to the Exchange. @@ -110,30 +115,45 @@ export class MatchOrderTester { takerAddress: string, erc20BalancesByOwner: ERC20BalancesByOwner, erc721TokenIdsByOwner: ERC721TokenIdsByOwner, + expectedTransferAmounts: TransferAmounts, + leftOrderEndState: OrderStatus, + rightOrderEndState: OrderStatus, initialTakerAssetFilledAmountLeft?: BigNumber, initialTakerAssetFilledAmountRight?: BigNumber, ): Promise<[ERC20BalancesByOwner, ERC721TokenIdsByOwner]> { - // Verify Left order preconditions - const orderTakerAssetFilledAmountLeft = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( - orderHashUtils.getOrderHashHex(signedOrderLeft), - ); - const expectedOrderFilledAmountLeft = initialTakerAssetFilledAmountLeft - ? initialTakerAssetFilledAmountLeft - : new BigNumber(0); - expect(expectedOrderFilledAmountLeft).to.be.bignumber.equal(orderTakerAssetFilledAmountLeft); - // Verify Right order preconditions - const orderTakerAssetFilledAmountRight = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( - orderHashUtils.getOrderHashHex(signedOrderRight), + // Verify initial order states + let orderTakerAssetFilledAmountLeft: BigNumber; + let orderTakerAssetFilledAmountRight: BigNumber; + [orderTakerAssetFilledAmountLeft, orderTakerAssetFilledAmountRight] = await this._verifyInitialOrderStates( + signedOrderLeft, + signedOrderRight, + initialTakerAssetFilledAmountLeft, + initialTakerAssetFilledAmountRight ); - const expectedOrderFilledAmountRight = initialTakerAssetFilledAmountRight - ? initialTakerAssetFilledAmountRight - : new BigNumber(0); - expect(expectedOrderFilledAmountRight).to.be.bignumber.equal(orderTakerAssetFilledAmountRight); // Match left & right orders const transactionReceipt = await this._exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress); const newERC20BalancesByOwner = await this._erc20Wrapper.getBalancesAsync(); const newERC721TokenIdsByOwner = await this._erc721Wrapper.getBalancesAsync(); + // Calculate expected fees + + + // Verify logs + + // Verify balances + + + + console.log(JSON.stringify(transactionReceipt, null, 4)); + + // Verify logs (and return values) + + // Verify exchange state + + // Verify balances + + // Calculate expected balance changes + /* const expectedTransferAmounts = await this._calculateExpectedTransferAmountsAsync( signedOrderLeft, signedOrderRight, @@ -141,7 +161,7 @@ export class MatchOrderTester { orderTakerAssetFilledAmountRight, transactionReceipt, takerAddress - ); + );*/ let expectedERC20BalancesByOwner: ERC20BalancesByOwner; let expectedERC721TokenIdsByOwner: ERC721TokenIdsByOwner; [expectedERC20BalancesByOwner, expectedERC721TokenIdsByOwner] = this._calculateExpectedBalances( @@ -169,6 +189,7 @@ export class MatchOrderTester { const makerAssetAddressRight = makerERC20AssetDataRight.tokenAddress; console.log("Left Maker: Sell Amount"); + // describe('asdad', () => { if(makerAssetProxyIdLeft == AssetProxyId.ERC20) { expect(newERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft]); } else if(makerAssetProxyIdLeft == AssetProxyId.ERC721) { @@ -177,6 +198,8 @@ export class MatchOrderTester { throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdLeft}`); } + // }); + console.log("Left Maker: Buy Amount"); if(makerAssetProxyIdRight == AssetProxyId.ERC20) { expect(newERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight]); @@ -292,11 +315,12 @@ export class MatchOrderTester { // This is a catch-all to ensure that no other balances changed console.log("Catch-all"); expect(didExpectedBalancesMatchRealBalances).to.be.true(); - */ - + +*/ return [newERC20BalancesByOwner, newERC721TokenIdsByOwner]; } + /// @dev Calculates expected transfer amounts between order makers, fee recipients, and /// the taker when two orders are matched. /// @param signedOrderLeft First matched order. @@ -312,7 +336,7 @@ export class MatchOrderTester { transactionReceipt: TransactionReceiptWithDecodedLogs, takerAddress: string ): Promise { - // Parse logs + // Parse logs expect(transactionReceipt.logs.length).to.be.equal(2); // First log is for left fill const leftLog = ((transactionReceipt.logs[0] as any) as {args: { makerAddress: string, takerAddress: string, makerAssetFilledAmount: string, takerAssetFilledAmount: string, makerFeePaid: string, takerFeePaid: string}}); @@ -344,7 +368,6 @@ export class MatchOrderTester { const feePaidByRightMaker = signedOrderRight.makerFee .times(amountSoldByRightMaker) .dividedToIntegerBy(signedOrderRight.makerAssetAmount); - const feePaidByTakerLeft = signedOrderLeft.takerFee .times(amountBoughtByLeftMaker) .dividedToIntegerBy(signedOrderLeft.takerAssetAmount); @@ -361,21 +384,21 @@ export class MatchOrderTester { // Left Maker amountBoughtByLeftMaker, amountSoldByLeftMaker, - amountReceivedByLeftMaker, + // amountReceivedByLeftMaker, feePaidByLeftMaker, // Right Maker amountBoughtByRightMaker, amountSoldByRightMaker, - amountReceivedByRightMaker, + // amountReceivedByRightMaker, feePaidByRightMaker, // Taker amountReceivedByTaker, feePaidByTakerLeft, feePaidByTakerRight, - totalFeePaidByTaker, + // totalFeePaidByTaker, // Fee Recipients - feeReceivedLeft, - feeReceivedRight, + // feeReceivedLeft, + // feeReceivedRight, }; return expectedTransferAmounts; } @@ -418,7 +441,7 @@ export class MatchOrderTester { expectedNewERC20BalancesByOwner[makerAddressRight][ takerAssetAddressRight ] = expectedNewERC20BalancesByOwner[makerAddressRight][takerAssetAddressRight].add( - expectedTransferAmounts.amountReceivedByRightMaker, + expectedTransferAmounts.amountBoughtByRightMaker, ); // Taker expectedNewERC20BalancesByOwner[takerAddress][makerAssetAddressLeft] = expectedNewERC20BalancesByOwner[ @@ -455,7 +478,7 @@ export class MatchOrderTester { // Left Maker expectedNewERC20BalancesByOwner[makerAddressLeft][takerAssetAddressLeft] = expectedNewERC20BalancesByOwner[ makerAddressLeft - ][takerAssetAddressLeft].add(expectedTransferAmounts.amountReceivedByLeftMaker); + ][takerAssetAddressLeft].add(expectedTransferAmounts.amountBoughtByLeftMaker); // Right Maker expectedNewERC20BalancesByOwner[makerAddressRight][ makerAssetAddressRight @@ -485,18 +508,18 @@ export class MatchOrderTester { // Taker Fees expectedNewERC20BalancesByOwner[takerAddress][this._feeTokenAddress] = expectedNewERC20BalancesByOwner[ takerAddress - ][this._feeTokenAddress].minus(expectedTransferAmounts.totalFeePaidByTaker); + ][this._feeTokenAddress].minus(expectedTransferAmounts.feePaidByTakerLeft.add(expectedTransferAmounts.feePaidByTakerRight)); // Left Fee Recipient Fees expectedNewERC20BalancesByOwner[feeRecipientAddressLeft][ this._feeTokenAddress ] = expectedNewERC20BalancesByOwner[feeRecipientAddressLeft][this._feeTokenAddress].add( - expectedTransferAmounts.feeReceivedLeft, + expectedTransferAmounts.feePaidByLeftMaker.add(expectedTransferAmounts.feePaidByTakerLeft), ); // Right Fee Recipient Fees expectedNewERC20BalancesByOwner[feeRecipientAddressRight][ this._feeTokenAddress ] = expectedNewERC20BalancesByOwner[feeRecipientAddressRight][this._feeTokenAddress].add( - expectedTransferAmounts.feeReceivedRight, + expectedTransferAmounts.feePaidByRightMaker.add(expectedTransferAmounts.feePaidByTakerRight), ); return [expectedNewERC20BalancesByOwner, expectedNewERC721TokenIdsByOwner]; -- cgit v1.2.3 From 8c706ac6392c79ef46da37dd4b76f58f7d3f57e4 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Wed, 22 Aug 2018 15:22:27 -0700 Subject: Verify logs --- .../contracts/test/utils/match_order_tester.ts | 116 ++++++++------------- 1 file changed, 44 insertions(+), 72 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index 7b071015f..15915e511 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -134,10 +134,15 @@ export class MatchOrderTester { const transactionReceipt = await this._exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress); const newERC20BalancesByOwner = await this._erc20Wrapper.getBalancesAsync(); const newERC721TokenIdsByOwner = await this._erc721Wrapper.getBalancesAsync(); - // Calculate expected fees - - - // Verify logs + // Verify logs & return values + this._verifyLogsAsync( + signedOrderLeft, + signedOrderRight, + transactionReceipt, + takerAddress, + expectedTransferAmounts + ); + // Verify exchange state // Verify balances @@ -328,78 +333,45 @@ export class MatchOrderTester { /// @param orderTakerAssetFilledAmountLeft How much left order has been filled, prior to matching orders. /// @param orderTakerAssetFilledAmountRight How much the right order has been filled, prior to matching orders. /// @return TransferAmounts A struct containing the expected transfer amounts. - private async _calculateExpectedTransferAmountsAsync( + private async _verifyLogsAsync( signedOrderLeft: SignedOrder, signedOrderRight: SignedOrder, - orderTakerAssetFilledAmountLeft: BigNumber, - orderTakerAssetFilledAmountRight: BigNumber, transactionReceipt: TransactionReceiptWithDecodedLogs, - takerAddress: string - ): Promise { + takerAddress: string, + expectedTransferAmounts: TransferAmounts + ) { // Parse logs - expect(transactionReceipt.logs.length).to.be.equal(2); - // First log is for left fill - const leftLog = ((transactionReceipt.logs[0] as any) as {args: { makerAddress: string, takerAddress: string, makerAssetFilledAmount: string, takerAssetFilledAmount: string, makerFeePaid: string, takerFeePaid: string}}); - expect(leftLog.args.makerAddress).to.be.equal(signedOrderLeft.makerAddress); - expect(leftLog.args.takerAddress).to.be.equal(takerAddress); - const amountBoughtByLeftMaker = new BigNumber(leftLog.args.takerAssetFilledAmount); - const amountSoldByLeftMaker = new BigNumber(leftLog.args.makerAssetFilledAmount); - // Second log is for right fill - const rightLog = ((transactionReceipt.logs[1] as any) as {args: { makerAddress: string, takerAddress: string, makerAssetFilledAmount: string, takerAssetFilledAmount: string, makerFeePaid: string, takerFeePaid: string}}); - expect(rightLog.args.makerAddress).to.be.equal(signedOrderRight.makerAddress); - expect(rightLog.args.takerAddress).to.be.equal(takerAddress); - const amountBoughtByRightMaker = new BigNumber(rightLog.args.takerAssetFilledAmount); - const amountSoldByRightMaker = new BigNumber(rightLog.args.makerAssetFilledAmount); - // Determine amount received by taker - const amountReceivedByTaker = amountSoldByLeftMaker.sub(amountBoughtByRightMaker); - - const amountReceivedByLeftMaker = amountBoughtByLeftMaker; - const amountReceivedByRightMaker = amountBoughtByRightMaker; - - console.log("Amount bought by left maker = ", JSON.stringify(amountBoughtByLeftMaker)); - console.log("Amount sold by left maker = ", JSON.stringify(amountSoldByLeftMaker)); - console.log("Amount bought by right maker = ", JSON.stringify(amountBoughtByRightMaker)); - console.log("Amount sold by right maker = ", JSON.stringify(amountSoldByRightMaker)); - console.log("Amount received by taker = ", JSON.stringify(amountReceivedByTaker)); - //const amountReceivedByLeftMaker = amountSoldByRightMaker; - const feePaidByLeftMaker = signedOrderLeft.makerFee - .times(amountSoldByLeftMaker) - .dividedToIntegerBy(signedOrderLeft.makerAssetAmount); - const feePaidByRightMaker = signedOrderRight.makerFee - .times(amountSoldByRightMaker) - .dividedToIntegerBy(signedOrderRight.makerAssetAmount); - const feePaidByTakerLeft = signedOrderLeft.takerFee - .times(amountBoughtByLeftMaker) - .dividedToIntegerBy(signedOrderLeft.takerAssetAmount); - - - const feePaidByTakerRight = signedOrderRight.takerFee - .times(amountBoughtByRightMaker) - .dividedToIntegerBy(signedOrderRight.takerAssetAmount); - const totalFeePaidByTaker = feePaidByTakerLeft.add(feePaidByTakerRight); - const feeReceivedLeft = feePaidByLeftMaker.add(feePaidByTakerLeft); - const feeReceivedRight = feePaidByRightMaker.add(feePaidByTakerRight); - // Return values - const expectedTransferAmounts = { - // Left Maker - amountBoughtByLeftMaker, - amountSoldByLeftMaker, - // amountReceivedByLeftMaker, - feePaidByLeftMaker, - // Right Maker - amountBoughtByRightMaker, - amountSoldByRightMaker, - // amountReceivedByRightMaker, - feePaidByRightMaker, - // Taker - amountReceivedByTaker, - feePaidByTakerLeft, - feePaidByTakerRight, - // totalFeePaidByTaker, - // Fee Recipients - // feeReceivedLeft, - // feeReceivedRight, - }; + expect(transactionReceipt.logs.length, "Checking number of logs").to.be.equal(2); + // First log is for left fill + const leftLog = ((transactionReceipt.logs[0] as any) as {args: { makerAddress: string, takerAddress: string, makerAssetFilledAmount: string, takerAssetFilledAmount: string, makerFeePaid: string, takerFeePaid: string}}); + expect(leftLog.args.makerAddress, "Checking logged maker address of left order").to.be.equal(signedOrderLeft.makerAddress); + expect(leftLog.args.takerAddress, "Checking logged taker address of right order").to.be.equal(takerAddress); + const amountBoughtByLeftMaker = new BigNumber(leftLog.args.takerAssetFilledAmount); + const amountSoldByLeftMaker = new BigNumber(leftLog.args.makerAssetFilledAmount); + const feePaidByLeftMaker = new BigNumber(leftLog.args.makerFeePaid); + const feePaidByTakerLeft = new BigNumber(leftLog.args.takerFeePaid); + // Second log is for right fill + const rightLog = ((transactionReceipt.logs[1] as any) as {args: { makerAddress: string, takerAddress: string, makerAssetFilledAmount: string, takerAssetFilledAmount: string, makerFeePaid: string, takerFeePaid: string}}); + expect(rightLog.args.makerAddress, "Checking logged maker address of right order").to.be.equal(signedOrderRight.makerAddress); + expect(rightLog.args.takerAddress, "Checking loggerd taker address of right order").to.be.equal(takerAddress); + const amountBoughtByRightMaker = new BigNumber(rightLog.args.takerAssetFilledAmount); + const amountSoldByRightMaker = new BigNumber(rightLog.args.makerAssetFilledAmount); + const feePaidByRightMaker = new BigNumber(rightLog.args.makerFeePaid); + const feePaidByTakerRight = new BigNumber(rightLog.args.takerFeePaid); + // Derive amount received by taker + const amountReceivedByTaker = amountSoldByLeftMaker.sub(amountBoughtByRightMaker); + // Verify log values - left order + expect(expectedTransferAmounts.amountBoughtByLeftMaker, "Checking logged amount bought by left maker").to.be.bignumber.equal(amountBoughtByLeftMaker); + expect(expectedTransferAmounts.amountSoldByLeftMaker, "Checking logged amount sold by left maker").to.be.bignumber.equal(amountSoldByLeftMaker); + expect(expectedTransferAmounts.feePaidByLeftMaker, "Checking logged fee paid by left maker").to.be.bignumber.equal(feePaidByLeftMaker); + expect(expectedTransferAmounts.feePaidByTakerLeft, "Checking logged fee paid on left order by taker").to.be.bignumber.equal(feePaidByTakerLeft); + // Verify log values - right order + expect(expectedTransferAmounts.amountBoughtByRightMaker, "Checking logged amount bought by right maker").to.be.bignumber.equal(amountBoughtByRightMaker); + expect(expectedTransferAmounts.amountSoldByRightMaker, "Checking logged amount sold by right maker").to.be.bignumber.equal(amountSoldByRightMaker); + expect(expectedTransferAmounts.feePaidByRightMaker, "Checking logged fee paid by right maker").to.be.bignumber.equal(feePaidByRightMaker); + expect(expectedTransferAmounts.feePaidByTakerRight, "Checking logged fee paid on right order by taker").to.be.bignumber.equal(feePaidByTakerRight); + // Verify deriv ed amount received by taker + expect(expectedTransferAmounts.amountReceivedByTaker, "Checking logged amount received by taker").to.be.bignumber.equal(amountReceivedByTaker); return expectedTransferAmounts; } /// @dev Calculates the expected balances of order makers, fee recipients, and the taker, -- cgit v1.2.3 From d2912561585a044eec3cf82b65e76760e7cb47c3 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Wed, 22 Aug 2018 16:33:44 -0700 Subject: Passes comprehensive test --- .../contracts/test/utils/match_order_tester.ts | 332 +++++++++------------ 1 file changed, 141 insertions(+), 191 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index 15915e511..c412595f5 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -8,7 +8,7 @@ import { chaiSetup } from './chai_setup'; import { ERC20Wrapper } from './erc20_wrapper'; import { ERC721Wrapper } from './erc721_wrapper'; import { ExchangeWrapper } from './exchange_wrapper'; -import { ERC20BalancesByOwner, ERC721TokenIdsByOwner, TransferAmountsByMatchOrders as TransferAmounts , OrderStatus} from './types'; +import { ERC20BalancesByOwner, ERC721TokenIdsByOwner, TransferAmountsByMatchOrders as TransferAmounts, OrderInfo, OrderStatus} from './types'; import { TransactionReceiptWithDecodedLogs } from '../../../../node_modules/ethereum-types'; chaiSetup.configure(); @@ -20,23 +20,109 @@ export class MatchOrderTester { private readonly _erc721Wrapper: ERC721Wrapper; private readonly _feeTokenAddress: string; + private async _verifyBalancesAsync( + signedOrderLeft: SignedOrder, + signedOrderRight: SignedOrder, + initialERC20BalancesByOwner: ERC20BalancesByOwner, + initialERC721TokenIdsByOwner: ERC721TokenIdsByOwner, + finalERC20BalancesByOwner: ERC20BalancesByOwner, + finalERC721TokenIdsByOwner: ERC721TokenIdsByOwner, + expectedTransferAmounts: TransferAmounts, + takerAddress: string + ) + { + let expectedERC20BalancesByOwner: ERC20BalancesByOwner; + let expectedERC721TokenIdsByOwner: ERC721TokenIdsByOwner; + [expectedERC20BalancesByOwner, expectedERC721TokenIdsByOwner] = this._calculateExpectedBalances( + signedOrderLeft, + signedOrderRight, + takerAddress, + initialERC20BalancesByOwner, + initialERC721TokenIdsByOwner, + expectedTransferAmounts, + ); + // Verify balances of makers, taker, and fee recipients + await this._verifyMakerTakerAndFeeRecipientBalancesAsync( + signedOrderLeft, + signedOrderRight, + expectedERC20BalancesByOwner, + finalERC20BalancesByOwner, + expectedERC721TokenIdsByOwner, + finalERC721TokenIdsByOwner, + takerAddress + ); + // Verify balances for all known accounts + await this._verifyAllKnownBalancesAsync( + expectedERC20BalancesByOwner, + finalERC20BalancesByOwner, + expectedERC721TokenIdsByOwner, + finalERC721TokenIdsByOwner, + ); + } + + private async _verifyMakerTakerAndFeeRecipientBalancesAsync( + signedOrderLeft: SignedOrder, + signedOrderRight: SignedOrder, + expectedERC20BalancesByOwner: ERC20BalancesByOwner, + realERC20BalancesByOwner: ERC20BalancesByOwner, + expectedERC721TokenIdsByOwner: ERC721TokenIdsByOwner, + realERC721TokenIdsByOwner: ERC721TokenIdsByOwner, + takerAddress: string + ) { + // Individual balance comparisons + const makerAssetProxyIdLeft = assetDataUtils.decodeAssetProxyId(signedOrderLeft.makerAssetData); + const makerERC20AssetDataLeft = makerAssetProxyIdLeft == AssetProxyId.ERC20 ? assetDataUtils.decodeERC20AssetData(signedOrderLeft.makerAssetData) : assetDataUtils.decodeERC721AssetData(signedOrderLeft.makerAssetData); + const makerAssetAddressLeft = makerERC20AssetDataLeft.tokenAddress; + const makerAssetProxyIdRight = assetDataUtils.decodeAssetProxyId(signedOrderRight.makerAssetData); + const makerERC20AssetDataRight = makerAssetProxyIdRight == AssetProxyId.ERC20 ? assetDataUtils.decodeERC20AssetData(signedOrderRight.makerAssetData) : assetDataUtils.decodeERC721AssetData(signedOrderRight.makerAssetData); + const makerAssetAddressRight = makerERC20AssetDataRight.tokenAddress; + // describe('asdad', () => { + if(makerAssetProxyIdLeft == AssetProxyId.ERC20) { + expect(realERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft], "Checking left maker egress ERC20 account balance").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft]); + expect(realERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft], "Checking right maker ingress ERC20 account balance").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft]); + expect(realERC20BalancesByOwner[takerAddress][makerAssetAddressLeft], "Checking taker ingress ERC20 account balance").to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][makerAssetAddressLeft]); + } else if(makerAssetProxyIdLeft == AssetProxyId.ERC721) { + expect(realERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort(), "Checking left maker egress ERC721 account holdings").to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort()); + expect(realERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort(), "Checking right maker ERC721 account holdings").to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort()); + expect(realERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort(), "Checking taker ingress ERC721 account holdings").to.be.deep.equal(expectedERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort()); + } else { + throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdLeft}`); + } + if(makerAssetProxyIdRight == AssetProxyId.ERC20) { + expect(realERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight], "Checking left maker ingress ERC20 account balance").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight]); + expect(realERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight], "Checking right maker egress ERC20 account balance").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]); + } else if(makerAssetProxyIdRight == AssetProxyId.ERC721) { + expect(realERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort(), "Checking left maker ingress ERC721 account holdings").to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort()); + expect(realERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight], "Checking right maker agress ERC721 account holdings").to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]); + } else { + throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdRight}`); + } + // Paid fees + expect(realERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress], "Checking left maker egress ERC20 account fees").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress]); + expect(realERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress], "Checking right maker egress ERC20 account fees").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress]); + expect(realERC20BalancesByOwner[takerAddress][this._feeTokenAddress], "Checking taker egress ERC20 account fees").to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][this._feeTokenAddress]); + // Received fees + expect(realERC20BalancesByOwner[signedOrderLeft.feeRecipientAddress][this._feeTokenAddress], "Checking left fee recipient ingress ERC20 account fees").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.feeRecipientAddress][this._feeTokenAddress]); + expect(realERC20BalancesByOwner[signedOrderRight.feeRecipientAddress][this._feeTokenAddress], "Checking right fee receipient ingress ERC20 account fees").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.feeRecipientAddress][this._feeTokenAddress]); + } + /// @dev Compares a pair of ERC20 balances and a pair of ERC721 token owners. /// @param expectedNewERC20BalancesByOwner Expected ERC20 balances. /// @param realERC20BalancesByOwner Actual ERC20 balances. /// @param expectedNewERC721TokenIdsByOwner Expected ERC721 token owners. /// @param realERC721TokenIdsByOwner Actual ERC20 token owners. /// @return True only if ERC20 balances match and ERC721 token owners match. - private static _compareExpectedAndRealBalances( + private async _verifyAllKnownBalancesAsync( expectedNewERC20BalancesByOwner: ERC20BalancesByOwner, realERC20BalancesByOwner: ERC20BalancesByOwner, expectedNewERC721TokenIdsByOwner: ERC721TokenIdsByOwner, realERC721TokenIdsByOwner: ERC721TokenIdsByOwner, - ): boolean { + ) { + // ERC20 Balances - const doesErc20BalancesMatch = _.isEqual(expectedNewERC20BalancesByOwner, realERC20BalancesByOwner); - if (!doesErc20BalancesMatch) { - return false; - } + const erc20BalancesMatch = _.isEqual(expectedNewERC20BalancesByOwner, realERC20BalancesByOwner); + expect(erc20BalancesMatch, "Checking all known ERC20 account balances").to.be.true(); + // ERC721 Token Ids const sortedExpectedNewERC721TokenIdsByOwner = _.mapValues( expectedNewERC721TokenIdsByOwner, @@ -51,11 +137,11 @@ export class MatchOrderTester { _.sortBy(tokenIds); }); }); - const doesErc721TokenIdsMatch = _.isEqual( + const erc721TokenIdsMatch = _.isEqual( sortedExpectedNewERC721TokenIdsByOwner, sortedNewERC721TokenIdsByOwner, ); - return doesErc721TokenIdsMatch; + expect(erc721TokenIdsMatch, "Checking all known ERC721 account balances").to.be.true(); } private async _verifyInitialOrderStates( @@ -134,8 +220,8 @@ export class MatchOrderTester { const transactionReceipt = await this._exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress); const newERC20BalancesByOwner = await this._erc20Wrapper.getBalancesAsync(); const newERC721TokenIdsByOwner = await this._erc721Wrapper.getBalancesAsync(); - // Verify logs & return values - this._verifyLogsAsync( + // Verify logs + await this._verifyLogsAsync( signedOrderLeft, signedOrderRight, transactionReceipt, @@ -143,189 +229,26 @@ export class MatchOrderTester { expectedTransferAmounts ); // Verify exchange state - - // Verify balances - - - - console.log(JSON.stringify(transactionReceipt, null, 4)); - - // Verify logs (and return values) - - // Verify exchange state - - // Verify balances - - - // Calculate expected balance changes - /* - const expectedTransferAmounts = await this._calculateExpectedTransferAmountsAsync( + await this._verifyExchangeStateAsync( signedOrderLeft, signedOrderRight, orderTakerAssetFilledAmountLeft, orderTakerAssetFilledAmountRight, - transactionReceipt, - takerAddress - );*/ - let expectedERC20BalancesByOwner: ERC20BalancesByOwner; - let expectedERC721TokenIdsByOwner: ERC721TokenIdsByOwner; - [expectedERC20BalancesByOwner, expectedERC721TokenIdsByOwner] = this._calculateExpectedBalances( + expectedTransferAmounts + ); + // Verify balances of makers, taker, and fee recipients + await this._verifyBalancesAsync( signedOrderLeft, signedOrderRight, - takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, - expectedTransferAmounts, - ); - // Assert our expected balances are equal to the actual balances - const didExpectedBalancesMatchRealBalances = MatchOrderTester._compareExpectedAndRealBalances( - expectedERC20BalancesByOwner, newERC20BalancesByOwner, - expectedERC721TokenIdsByOwner, newERC721TokenIdsByOwner, + expectedTransferAmounts, + takerAddress ); - - // Individual balance comparisons - const makerAssetProxyIdLeft = assetDataUtils.decodeAssetProxyId(signedOrderLeft.makerAssetData); - const makerERC20AssetDataLeft = makerAssetProxyIdLeft == AssetProxyId.ERC20 ? assetDataUtils.decodeERC20AssetData(signedOrderLeft.makerAssetData) : assetDataUtils.decodeERC721AssetData(signedOrderLeft.makerAssetData); - const makerAssetAddressLeft = makerERC20AssetDataLeft.tokenAddress; - const makerAssetProxyIdRight = assetDataUtils.decodeAssetProxyId(signedOrderRight.makerAssetData); - const makerERC20AssetDataRight = makerAssetProxyIdRight == AssetProxyId.ERC20 ? assetDataUtils.decodeERC20AssetData(signedOrderRight.makerAssetData) : assetDataUtils.decodeERC721AssetData(signedOrderRight.makerAssetData); - const makerAssetAddressRight = makerERC20AssetDataRight.tokenAddress; - - console.log("Left Maker: Sell Amount"); - // describe('asdad', () => { - if(makerAssetProxyIdLeft == AssetProxyId.ERC20) { - expect(newERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft]); - } else if(makerAssetProxyIdLeft == AssetProxyId.ERC721) { - expect(newERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort()).to.be.deep.equal(newERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort()); - } else { - throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdLeft}`); - } - - // }); - - console.log("Left Maker: Buy Amount"); - if(makerAssetProxyIdRight == AssetProxyId.ERC20) { - expect(newERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight]); - } else if(makerAssetProxyIdRight == AssetProxyId.ERC721) { - expect(newERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort()).to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort()); - } else { - throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdRight}`); - } - - console.log("Left Maker: Fees"); - expect(newERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress]); - - console.log("Right Maker: Sell Amount"); - if(makerAssetProxyIdRight == AssetProxyId.ERC20) { - expect(newERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]); - } else if(makerAssetProxyIdRight == AssetProxyId.ERC721) { - expect(newERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]).to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]); - } else { - throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdRight}`); - } - - console.log("Right Maker: Buy Amount"); - if(makerAssetProxyIdLeft == AssetProxyId.ERC20) { - expect(newERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft]); - } else if(makerAssetProxyIdLeft == AssetProxyId.ERC721) { - expect(newERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort()).to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort()); - } else { - throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdLeft}`); - } - console.log("Right Maker: Fees"); - expect(newERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress]).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress]); - console.log("Taker: Receive Amount"); - if(makerAssetProxyIdLeft == AssetProxyId.ERC20) { - expect(newERC20BalancesByOwner[takerAddress][makerAssetAddressLeft]).to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][makerAssetAddressLeft]); - } else if(makerAssetProxyIdLeft == AssetProxyId.ERC721) { - expect(newERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort()).to.be.deep.equal(expectedERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort()); - } else { - throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdLeft}`); - } - console.log("Taker: Fees"); - expect(newERC20BalancesByOwner[takerAddress][this._feeTokenAddress]).to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][this._feeTokenAddress]); - console.log("Balance catch-all"); - expect(didExpectedBalancesMatchRealBalances).to.be.true(); - - /* - // Compute actual transfer amounts - let actualTransferAmounts = {}; - const makerAssetProxyIdLeft = assetDataUtils.decodeAssetProxyId(signedOrderLeft.makerAssetData); - if (makerAssetProxyIdLeft === AssetProxyId.ERC20) { - const erc20AssetData = assetDataUtils.decodeERC20AssetData(signedOrderLeft.makerAssetData); - const makerAssetAddressLeft = erc20AssetData.tokenAddress; - actualTransferAmounts.amountSoldByLeftMaker = erc20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sub(newERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft]); - actualTransferAmounts.amountBoughtByRightMaker = newERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sub(erc20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft]); - actualTransferAmounts.amountReceivedByRightMaker = actualTransferAmounts.amountBoughtByRightMaker; - actualTransferAmounts.amountReceivedByTaker = newERC20BalancesByOwner[takerAddress][makerAssetAddressLeft].sub(erc20BalancesByOwner[takerAddress][makerAssetAddressLeft]); - } else if(makerAssetProxyIdLeft === AssetProxyId.ERC721) { - } - const makerAssetProxyIdRight = assetDataUtils.decodeAssetProxyId(signedOrderRight.makerAssetData); - if (makerAssetProxyIdRight === AssetProxyId.ERC20) { - const erc20AssetData = assetDataUtils.decodeERC20AssetData(signedOrderRight.makerAssetData); - const makerAssetAddressRight = erc20AssetData.tokenAddress; - actualTransferAmounts.amountSoldByRightMaker = erc20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight].sub(newERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]); - actualTransferAmounts.amountBoughtByLeftMaker = newERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sub(erc20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight]); - actualTransferAmounts.amountReceivedByLeftMaker = actualTransferAmounts.amountBoughtByLeftMaker; - } else if(makerAssetProxyIdRight === AssetProxyId.ERC721) { - } - // Fees - actualTransferAmounts.feePaidByLeftMaker = erc20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress].sub(newERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress]); - actualTransferAmounts.feePaidByRightMaker = erc20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress].sub(newERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress]); - actualTransferAmounts.totalFeePaidByTaker = erc20BalancesByOwner[takerAddress][this._feeTokenAddress].sub(newERC20BalancesByOwner[takerAddress][this._feeTokenAddress]); - - console.log("amountBoughtByLeftMaker"); - // expect(expectedTransferAmounts.amountBoughtByLeftMaker).to.be.bignumber.equal(actualTransferAmounts.amountBoughtByLeftMaker); - console.log("amountSoldByLeftMaker"); - // expect(expectedTransferAmounts.amountSoldByLeftMaker).to.be.bignumber.equal(actualTransferAmounts.amountSoldByLeftMaker); - console.log("amountBoughtByRightMaker"); - // expect(expectedTransferAmounts.amountBoughtByRightMaker).to.be.bignumber.equal(actualTransferAmounts.amountBoughtByRightMaker); - console.log("amountSoldByRightMaker"); - //expect(expectedTransferAmounts.amountSoldByRightMaker).to.be.bignumber.equal(actualTransferAmounts.amountSoldByRightMaker); - console.log("amountReceivedByTaker"); - //expect(expectedTransferAmounts.amountReceivedByTaker).to.be.bignumber.equal(actualTransferAmounts.amountReceivedByTaker); - console.log("feePaidByLeftMaker"); - // expect(expectedTransferAmounts.feePaidByLeftMaker).to.be.bignumber.equal(actualTransferAmounts.feePaidByLeftMaker); - console.log("feePaidByRightMaker"); - //expect(expectedTransferAmounts.feePaidByRightMaker).to.be.bignumber.equal(actualTransferAmounts.feePaidByRightMaker); - console.log("totalFeePaidByTaker"); - // expect(expectedTransferAmounts.totalFeePaidByTaker).to.be.bignumber.equal(actualTransferAmounts.totalFeePaidByTaker); - - - -/* - const actualTransferAmounts = { - // Left Maker - amountBoughtByLeftMaker, - amountSoldByLeftMaker, - amountReceivedByLeftMaker, - feePaidByLeftMaker, - // Right Maker - amountBoughtByRightMaker, - amountSoldByRightMaker, - amountReceivedByRightMaker, - feePaidByRightMaker, - // Taker - amountReceivedByTaker, - feePaidByTakerLeft, - feePaidByTakerRight, - totalFeePaidByTaker, - // Fee Recipients - feeReceivedLeft, - feeReceivedRight, - }; - - // This is a catch-all to ensure that no other balances changed - console.log("Catch-all"); - expect(didExpectedBalancesMatchRealBalances).to.be.true(); - - -*/ return [newERC20BalancesByOwner, newERC721TokenIdsByOwner]; } - /// @dev Calculates expected transfer amounts between order makers, fee recipients, and /// the taker when two orders are matched. /// @param signedOrderLeft First matched order. @@ -370,13 +293,47 @@ export class MatchOrderTester { expect(expectedTransferAmounts.amountSoldByRightMaker, "Checking logged amount sold by right maker").to.be.bignumber.equal(amountSoldByRightMaker); expect(expectedTransferAmounts.feePaidByRightMaker, "Checking logged fee paid by right maker").to.be.bignumber.equal(feePaidByRightMaker); expect(expectedTransferAmounts.feePaidByTakerRight, "Checking logged fee paid on right order by taker").to.be.bignumber.equal(feePaidByTakerRight); - // Verify deriv ed amount received by taker + // Verify derived amount received by taker expect(expectedTransferAmounts.amountReceivedByTaker, "Checking logged amount received by taker").to.be.bignumber.equal(amountReceivedByTaker); - return expectedTransferAmounts; + } + /// @dev Calculates expected transfer amounts between order makers, fee recipients, and + /// the taker when two orders are matched. + /// @param signedOrderLeft First matched order. + /// @param signedOrderRight Second matched order. + /// @param orderTakerAssetFilledAmountLeft How much left order has been filled, prior to matching orders. + /// @param orderTakerAssetFilledAmountRight How much the right order has been filled, prior to matching orders. + /// @return TransferAmounts A struct containing the expected transfer amounts. + private async _verifyExchangeStateAsync( + signedOrderLeft: SignedOrder, + signedOrderRight: SignedOrder, + orderTakerAssetFilledAmountLeft: BigNumber, + orderTakerAssetFilledAmountRight: BigNumber, + expectedTransferAmounts: TransferAmounts + ) { + // Verify state for left order: amount bought by left maker + let amountBoughtByLeftMaker = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( + orderHashUtils.getOrderHashHex(signedOrderLeft), + ); + amountBoughtByLeftMaker = amountBoughtByLeftMaker.minus(orderTakerAssetFilledAmountLeft); + expect(expectedTransferAmounts.amountBoughtByLeftMaker, "Checking exchange state for left order").to.be.bignumber.equal(amountBoughtByLeftMaker); + // Verify state for right order: amount bought by right maker + let amountBoughtByRightMaker = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( + orderHashUtils.getOrderHashHex(signedOrderRight), + ); + amountBoughtByRightMaker = amountBoughtByRightMaker.minus(orderTakerAssetFilledAmountRight); + expect(expectedTransferAmounts.amountBoughtByRightMaker, "Checking exchange state for right order").to.be.bignumber.equal(amountBoughtByRightMaker); + // Verify left order status + const leftOrderInfo: OrderInfo = await this._exchangeWrapper.getOrderInfoAsync(signedOrderLeft); + const leftExpectedStatus = (expectedTransferAmounts.amountBoughtByLeftMaker.equals(signedOrderLeft.takerAssetAmount)) ? OrderStatus.FULLY_FILLED : OrderStatus.FILLABLE; + expect(leftOrderInfo.orderStatus as OrderStatus, "Checking exchange status for left order").to.be.equal(leftExpectedStatus); + // Verify right order status + const rightOrderInfo: OrderInfo = await this._exchangeWrapper.getOrderInfoAsync(signedOrderRight); + const rightExpectedStatus = (expectedTransferAmounts.amountBoughtByRightMaker.equals(signedOrderRight.takerAssetAmount)) ? OrderStatus.FULLY_FILLED : OrderStatus.FILLABLE; + expect(rightOrderInfo.orderStatus as OrderStatus, "Checking exchange status for right order").to.be.equal(rightExpectedStatus); } /// @dev Calculates the expected balances of order makers, fee recipients, and the taker, /// as a result of matching two orders. - /// @param signedOrderLeft First matched order. + /// @param signedOrderRight First matched order. /// @param signedOrderRight Second matched order. /// @param takerAddress Address of taker (the address who matched the two orders) /// @param erc20BalancesByOwner Current ERC20 balances. @@ -430,13 +387,6 @@ export class MatchOrderTester { _.remove(expectedNewERC721TokenIdsByOwner[makerAddressLeft][makerAssetAddressLeft], makerAssetIdLeft); // Right Maker expectedNewERC721TokenIdsByOwner[makerAddressRight][takerAssetAddressRight].push(takerAssetIdRight); - console.log("*** TRANSFERINF: " + JSON.stringify(makerAssetIdLeft)); - console.log("****"); - console.log(JSON.stringify(expectedNewERC721TokenIdsByOwner[makerAddressLeft][makerAssetAddressLeft])); - console.log("****"); - console.log(JSON.stringify(expectedNewERC721TokenIdsByOwner[makerAddressRight][takerAssetAddressRight])); - console.log("****"); - // Taker: Since there is only 1 asset transferred, the taker does not receive any of the left maker asset. } // Left Taker Asset (Right Maker Asset) -- cgit v1.2.3 From 5a1dce15be60fa88d6e317cd6dfa7b993c90c257 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Wed, 22 Aug 2018 20:17:23 -0700 Subject: Updated all existing match order tests to use new format --- packages/contracts/test/exchange/match_orders.ts | 409 +++++++++++++++------ .../contracts/test/utils/match_order_tester.ts | 20 +- 2 files changed, 301 insertions(+), 128 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 3cffef7ad..eb2644ba6 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -21,7 +21,13 @@ 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, TransferAmountsByMatchOrders as TransferAmounts, OrderStatus } from '../utils/types'; +import { + ERC20BalancesByOwner, + ERC721TokenIdsByOwner, + OrderInfo, + TransferAmountsByMatchOrders as TransferAmounts, + OrderStatus, +} from '../utils/types'; import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; chaiSetup.configure(); @@ -177,7 +183,6 @@ describe.only('matchOrders', () => { erc20BalancesByOwner = await erc20Wrapper.getBalancesAsync(); erc721TokenIdsByOwner = await erc721Wrapper.getBalancesAsync(); }); - /* it.only('should transfer the correct amounts when orders completely fill each other', async () => { @@ -213,7 +218,7 @@ describe.only('matchOrders', () => { }); */ - it.only('Jacobs Example', async () => { + it('Jacobs Example', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ makerAddress: makerAddressLeft, @@ -244,8 +249,6 @@ describe.only('matchOrders', () => { feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 18), // 100% feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 17), // 50% }; - const expectedEndStateLeft = OrderStatus.FILLABLE; - const expectedEndStateRight = OrderStatus.FULLY_FILLED; // Match signedOrderLeft with signedOrderRight await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, @@ -254,44 +257,8 @@ describe.only('matchOrders', () => { erc20BalancesByOwner, erc721TokenIdsByOwner, expectedTransferAmounts, - expectedEndStateLeft, - expectedEndStateRight ); }); - - - /* - it.only('Jacobs Example', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAddress: makerAddressLeft, - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 0), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), - feeRecipientAddress: feeRecipientAddressLeft, - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAddress: makerAddressRight, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 0), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), - feeRecipientAddress: feeRecipientAddressRight, - }); - // Match signedOrderLeft with signedOrderRight - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( - signedOrderLeft, - signedOrderRight, - takerAddress, - erc20BalancesByOwner, - erc721TokenIdsByOwner, - ); - // // 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); - });*/ const reentrancyTest = (functionNames: string[]) => { _.forEach(functionNames, async (functionName: string, functionId: number) => { @@ -333,19 +300,28 @@ describe.only('matchOrders', () => { 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), + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( 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 () => { @@ -358,32 +334,29 @@ describe.only('matchOrders', () => { makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), }); - // Store original taker balance - const takerInitialBalances = _.cloneDeep(erc20BalancesByOwner[takerAddress][defaultERC20MakerAssetAddress]); // Match signedOrderLeft with signedOrderRight - let newERC20BalancesByOwner: ERC20BalancesByOwner; - let newERC721TokenIdsByOwner: ERC721TokenIdsByOwner; - // prettier-ignore - [ - newERC20BalancesByOwner, - // tslint:disable-next-line:trailing-comma - newERC721TokenIdsByOwner - ] = 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(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), + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + }; + // Match signedOrderLeft with signedOrderRight + await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, - ); - // 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], + expectedTransferAmounts, ); }); @@ -397,20 +370,30 @@ describe.only('matchOrders', () => { makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), }); - // Match orders + // 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), // 40% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 16), // 100% + }; + // Match signedOrderLeft with signedOrderRight await matchOrderTester.matchOrdersAndVerifyBalancesAsync( 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 () => { @@ -423,20 +406,30 @@ describe.only('matchOrders', () => { makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), }); - // Match orders + // 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), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 50% + }; + // Match signedOrderLeft with signedOrderRight await matchOrderTester.matchOrdersAndVerifyBalancesAsync( 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 () => { @@ -452,6 +445,20 @@ describe.only('matchOrders', () => { // 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), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 50% + }; // prettier-ignore [ newERC20BalancesByOwner, @@ -463,13 +470,8 @@ describe.only('matchOrders', () => { 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" @@ -478,24 +480,34 @@ describe.only('matchOrders', () => { makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18), }); + // Match signedOrderLeft with signedOrderRight2 const leftTakerAssetFilledAmount = signedOrderRight.makerAssetAmount; const rightTakerAssetFilledAmount = new BigNumber(0); + 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.matchOrdersAndVerifyBalancesAsync( 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 () => { @@ -512,6 +524,20 @@ describe.only('matchOrders', () => { // 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, @@ -523,13 +549,9 @@ describe.only('matchOrders', () => { 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" @@ -544,23 +566,58 @@ describe.only('matchOrders', () => { erc20BalancesByOwner[takerAddress][defaultERC20MakerAssetAddress], ); const rightTakerAssetFilledAmount = signedOrderLeft.makerAssetAmount.minus(takerAmountReceived); + 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.matchOrdersAndVerifyBalancesAsync( 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); }); + /* + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 18), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 18), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 16), // 10% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 18), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 18), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 16), // 100% + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 18), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 16), // 50% + }; + await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + expectedTransferAmounts + ); + + */ 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({ @@ -574,12 +631,27 @@ describe.only('matchOrders', () => { feeRecipientAddress, }); // Match orders + 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), + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); }); @@ -595,12 +667,27 @@ describe.only('matchOrders', () => { }); // Match orders takerAddress = signedOrderLeft.makerAddress; + 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), + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); }); @@ -616,12 +703,27 @@ describe.only('matchOrders', () => { }); // Match orders takerAddress = signedOrderRight.makerAddress; + 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), + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); }); @@ -637,12 +739,27 @@ describe.only('matchOrders', () => { }); // Match orders takerAddress = feeRecipientAddressLeft; + 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), + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); }); @@ -658,12 +775,27 @@ describe.only('matchOrders', () => { }); // Match orders takerAddress = feeRecipientAddressRight; + 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), + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); }); @@ -678,12 +810,27 @@ describe.only('matchOrders', () => { takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), }); // Match orders + 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), + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, + expectedTransferAmounts, ); }); @@ -796,22 +943,31 @@ describe.only('matchOrders', () => { takerAssetAmount: new BigNumber(1), }); // Match orders + 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.matchOrdersAndVerifyBalancesAsync( 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 () => { + it.only('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({ @@ -822,22 +978,31 @@ describe.only('matchOrders', () => { const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ makerAssetData: assetDataUtils.encodeERC721AssetData(defaultERC721AssetAddress, erc721TokenToTransfer), makerAssetAmount: new BigNumber(1), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(8), 18), }); // Match orders + 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), // 50% + }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( 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/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index c412595f5..00816e7d4 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -202,8 +202,6 @@ export class MatchOrderTester { erc20BalancesByOwner: ERC20BalancesByOwner, erc721TokenIdsByOwner: ERC721TokenIdsByOwner, expectedTransferAmounts: TransferAmounts, - leftOrderEndState: OrderStatus, - rightOrderEndState: OrderStatus, initialTakerAssetFilledAmountLeft?: BigNumber, initialTakerAssetFilledAmountRight?: BigNumber, ): Promise<[ERC20BalancesByOwner, ERC721TokenIdsByOwner]> { @@ -234,7 +232,9 @@ export class MatchOrderTester { signedOrderRight, orderTakerAssetFilledAmountLeft, orderTakerAssetFilledAmountRight, - expectedTransferAmounts + expectedTransferAmounts, + initialTakerAssetFilledAmountLeft, + initialTakerAssetFilledAmountRight ); // Verify balances of makers, taker, and fee recipients await this._verifyBalancesAsync( @@ -308,7 +308,9 @@ export class MatchOrderTester { signedOrderRight: SignedOrder, orderTakerAssetFilledAmountLeft: BigNumber, orderTakerAssetFilledAmountRight: BigNumber, - expectedTransferAmounts: TransferAmounts + expectedTransferAmounts: TransferAmounts, + initialTakerAssetFilledAmountLeft?: BigNumber, + initialTakerAssetFilledAmountRight?: BigNumber ) { // Verify state for left order: amount bought by left maker let amountBoughtByLeftMaker = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( @@ -323,12 +325,18 @@ export class MatchOrderTester { amountBoughtByRightMaker = amountBoughtByRightMaker.minus(orderTakerAssetFilledAmountRight); expect(expectedTransferAmounts.amountBoughtByRightMaker, "Checking exchange state for right order").to.be.bignumber.equal(amountBoughtByRightMaker); // Verify left order status + const maxAmountBoughtByLeftMaker = initialTakerAssetFilledAmountLeft + ? signedOrderLeft.takerAssetAmount.sub(initialTakerAssetFilledAmountLeft) + : signedOrderLeft.takerAssetAmount; const leftOrderInfo: OrderInfo = await this._exchangeWrapper.getOrderInfoAsync(signedOrderLeft); - const leftExpectedStatus = (expectedTransferAmounts.amountBoughtByLeftMaker.equals(signedOrderLeft.takerAssetAmount)) ? OrderStatus.FULLY_FILLED : OrderStatus.FILLABLE; + const leftExpectedStatus = (expectedTransferAmounts.amountBoughtByLeftMaker.equals(maxAmountBoughtByLeftMaker)) ? OrderStatus.FULLY_FILLED : OrderStatus.FILLABLE; expect(leftOrderInfo.orderStatus as OrderStatus, "Checking exchange status for left order").to.be.equal(leftExpectedStatus); // Verify right order status + const maxAmountBoughtByRightMaker = initialTakerAssetFilledAmountRight + ? signedOrderRight.takerAssetAmount.sub(initialTakerAssetFilledAmountRight) + : signedOrderRight.takerAssetAmount; const rightOrderInfo: OrderInfo = await this._exchangeWrapper.getOrderInfoAsync(signedOrderRight); - const rightExpectedStatus = (expectedTransferAmounts.amountBoughtByRightMaker.equals(signedOrderRight.takerAssetAmount)) ? OrderStatus.FULLY_FILLED : OrderStatus.FILLABLE; + const rightExpectedStatus = (expectedTransferAmounts.amountBoughtByRightMaker.equals(maxAmountBoughtByRightMaker)) ? OrderStatus.FULLY_FILLED : OrderStatus.FILLABLE; expect(rightOrderInfo.orderStatus as OrderStatus, "Checking exchange status for right order").to.be.equal(rightExpectedStatus); } /// @dev Calculates the expected balances of order makers, fee recipients, and the taker, -- cgit v1.2.3 From 70863cca085c060d55932e5e173fc2023bda9df5 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 10:31:06 -0700 Subject: Ran prettier and linter --- packages/contracts/test/exchange/match_orders.ts | 44 +- .../contracts/test/utils/match_order_tester.ts | 487 +++++++++++++-------- packages/contracts/test/utils/types.ts | 10 +- 3 files changed, 317 insertions(+), 224 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index eb2644ba6..7e9776d52 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -3,7 +3,6 @@ import { assetDataUtils } from '@0xproject/order-utils'; import { RevertReason } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; -import * as chai from 'chai'; import * as _ from 'lodash'; import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token'; @@ -14,25 +13,15 @@ import { ExchangeContract } from '../../generated_contract_wrappers/exchange'; import { ReentrantERC20TokenContract } from '../../generated_contract_wrappers/reentrant_erc20_token'; import { artifacts } from '../utils/artifacts'; import { expectTransactionFailedAsync } from '../utils/assertions'; -import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; import { ERC20Wrapper } from '../utils/erc20_wrapper'; import { ERC721Wrapper } from '../utils/erc721_wrapper'; import { ExchangeWrapper } from '../utils/exchange_wrapper'; import { MatchOrderTester } from '../utils/match_order_tester'; import { OrderFactory } from '../utils/order_factory'; -import { - ERC20BalancesByOwner, - ERC721TokenIdsByOwner, - OrderInfo, - TransferAmountsByMatchOrders as TransferAmounts, - OrderStatus, -} from '../utils/types'; +import { ERC20BalancesByOwner, ERC721TokenIdsByOwner } from '../utils/types'; import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; -chaiSetup.configure(); - -const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); describe.only('matchOrders', () => { @@ -470,7 +459,7 @@ describe.only('matchOrders', () => { takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, - expectedTransferAmounts + expectedTransferAmounts, ); // Construct second right order // Note: This order needs makerAssetAmount=90/takerAssetAmount=[anything <= 45] to fully fill the right order. @@ -480,7 +469,6 @@ describe.only('matchOrders', () => { makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18), }); - // Match signedOrderLeft with signedOrderRight2 const leftTakerAssetFilledAmount = signedOrderRight.makerAssetAmount; const rightTakerAssetFilledAmount = new BigNumber(0); @@ -549,7 +537,7 @@ describe.only('matchOrders', () => { takerAddress, erc20BalancesByOwner, erc721TokenIdsByOwner, - expectedTransferAmounts + expectedTransferAmounts, ); // Create second left order @@ -592,32 +580,6 @@ describe.only('matchOrders', () => { ); }); - /* - // Match signedOrderLeft with signedOrderRight - const expectedTransferAmounts = { - // Left Maker - amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 18), - amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 18), - feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 16), // 10% - // Right Maker - amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 18), - amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 18), - feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 16), // 100% - // Taker - amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 18), - feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 16), // 100% - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(), 16), // 50% - }; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( - signedOrderLeft, - signedOrderRight, - takerAddress, - erc20BalancesByOwner, - erc721TokenIdsByOwner, - expectedTransferAmounts - ); - - */ 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({ diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index 00816e7d4..300073eb0 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -4,12 +4,19 @@ import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import * as _ from 'lodash'; +import { TransactionReceiptWithDecodedLogs } from '../../../../node_modules/ethereum-types'; + import { chaiSetup } from './chai_setup'; import { ERC20Wrapper } from './erc20_wrapper'; import { ERC721Wrapper } from './erc721_wrapper'; import { ExchangeWrapper } from './exchange_wrapper'; -import { ERC20BalancesByOwner, ERC721TokenIdsByOwner, TransferAmountsByMatchOrders as TransferAmounts, OrderInfo, OrderStatus} from './types'; -import { TransactionReceiptWithDecodedLogs } from '../../../../node_modules/ethereum-types'; +import { + ERC20BalancesByOwner, + ERC721TokenIdsByOwner, + OrderInfo, + OrderStatus, + TransferAmountsByMatchOrders as TransferAmounts, +} from './types'; chaiSetup.configure(); const expect = chai.expect; @@ -19,110 +26,117 @@ export class MatchOrderTester { private readonly _erc20Wrapper: ERC20Wrapper; private readonly _erc721Wrapper: ERC721Wrapper; private readonly _feeTokenAddress: string; - - private async _verifyBalancesAsync( + /// @dev Calculates expected transfer amounts between order makers, fee recipients, and + /// the taker when two orders are matched. + /// @param signedOrderLeft First matched order. + /// @param signedOrderRight Second matched order. + /// @param orderTakerAssetFilledAmountLeft How much left order has been filled, prior to matching orders. + /// @param orderTakerAssetFilledAmountRight How much the right order has been filled, prior to matching orders. + /// @return TransferAmounts A struct containing the expected transfer amounts. + private static async _verifyLogsAsync( signedOrderLeft: SignedOrder, signedOrderRight: SignedOrder, - initialERC20BalancesByOwner: ERC20BalancesByOwner, - initialERC721TokenIdsByOwner: ERC721TokenIdsByOwner, - finalERC20BalancesByOwner: ERC20BalancesByOwner, - finalERC721TokenIdsByOwner: ERC721TokenIdsByOwner, + transactionReceipt: TransactionReceiptWithDecodedLogs, + takerAddress: string, expectedTransferAmounts: TransferAmounts, - takerAddress: string - ) - { - let expectedERC20BalancesByOwner: ERC20BalancesByOwner; - let expectedERC721TokenIdsByOwner: ERC721TokenIdsByOwner; - [expectedERC20BalancesByOwner, expectedERC721TokenIdsByOwner] = this._calculateExpectedBalances( - signedOrderLeft, - signedOrderRight, - takerAddress, - initialERC20BalancesByOwner, - initialERC721TokenIdsByOwner, - expectedTransferAmounts, - ); - // Verify balances of makers, taker, and fee recipients - await this._verifyMakerTakerAndFeeRecipientBalancesAsync( - signedOrderLeft, - signedOrderRight, - expectedERC20BalancesByOwner, - finalERC20BalancesByOwner, - expectedERC721TokenIdsByOwner, - finalERC721TokenIdsByOwner, - takerAddress + ): Promise { + // Parse logs + expect(transactionReceipt.logs.length, 'Checking number of logs').to.be.equal(2); + // First log is for left fill + const leftLog = (transactionReceipt.logs[0] as any) as { + args: { + makerAddress: string; + takerAddress: string; + makerAssetFilledAmount: string; + takerAssetFilledAmount: string; + makerFeePaid: string; + takerFeePaid: string; + }; + }; + expect(leftLog.args.makerAddress, 'Checking logged maker address of left order').to.be.equal( + signedOrderLeft.makerAddress, ); - // Verify balances for all known accounts - await this._verifyAllKnownBalancesAsync( - expectedERC20BalancesByOwner, - finalERC20BalancesByOwner, - expectedERC721TokenIdsByOwner, - finalERC721TokenIdsByOwner, + expect(leftLog.args.takerAddress, 'Checking logged taker address of right order').to.be.equal(takerAddress); + const amountBoughtByLeftMaker = new BigNumber(leftLog.args.takerAssetFilledAmount); + const amountSoldByLeftMaker = new BigNumber(leftLog.args.makerAssetFilledAmount); + const feePaidByLeftMaker = new BigNumber(leftLog.args.makerFeePaid); + const feePaidByTakerLeft = new BigNumber(leftLog.args.takerFeePaid); + // Second log is for right fill + const rightLog = (transactionReceipt.logs[1] as any) as { + args: { + makerAddress: string; + takerAddress: string; + makerAssetFilledAmount: string; + takerAssetFilledAmount: string; + makerFeePaid: string; + takerFeePaid: string; + }; + }; + expect(rightLog.args.makerAddress, 'Checking logged maker address of right order').to.be.equal( + signedOrderRight.makerAddress, ); + expect(rightLog.args.takerAddress, 'Checking loggerd taker address of right order').to.be.equal(takerAddress); + const amountBoughtByRightMaker = new BigNumber(rightLog.args.takerAssetFilledAmount); + const amountSoldByRightMaker = new BigNumber(rightLog.args.makerAssetFilledAmount); + const feePaidByRightMaker = new BigNumber(rightLog.args.makerFeePaid); + const feePaidByTakerRight = new BigNumber(rightLog.args.takerFeePaid); + // Derive amount received by taker + const amountReceivedByTaker = amountSoldByLeftMaker.sub(amountBoughtByRightMaker); + // Verify log values - left order + expect( + expectedTransferAmounts.amountBoughtByLeftMaker, + 'Checking logged amount bought by left maker', + ).to.be.bignumber.equal(amountBoughtByLeftMaker); + expect( + expectedTransferAmounts.amountSoldByLeftMaker, + 'Checking logged amount sold by left maker', + ).to.be.bignumber.equal(amountSoldByLeftMaker); + expect( + expectedTransferAmounts.feePaidByLeftMaker, + 'Checking logged fee paid by left maker', + ).to.be.bignumber.equal(feePaidByLeftMaker); + expect( + expectedTransferAmounts.feePaidByTakerLeft, + 'Checking logged fee paid on left order by taker', + ).to.be.bignumber.equal(feePaidByTakerLeft); + // Verify log values - right order + expect( + expectedTransferAmounts.amountBoughtByRightMaker, + 'Checking logged amount bought by right maker', + ).to.be.bignumber.equal(amountBoughtByRightMaker); + expect( + expectedTransferAmounts.amountSoldByRightMaker, + 'Checking logged amount sold by right maker', + ).to.be.bignumber.equal(amountSoldByRightMaker); + expect( + expectedTransferAmounts.feePaidByRightMaker, + 'Checking logged fee paid by right maker', + ).to.be.bignumber.equal(feePaidByRightMaker); + expect( + expectedTransferAmounts.feePaidByTakerRight, + 'Checking logged fee paid on right order by taker', + ).to.be.bignumber.equal(feePaidByTakerRight); + // Verify derived amount received by taker + expect( + expectedTransferAmounts.amountReceivedByTaker, + 'Checking logged amount received by taker', + ).to.be.bignumber.equal(amountReceivedByTaker); } - - private async _verifyMakerTakerAndFeeRecipientBalancesAsync( - signedOrderLeft: SignedOrder, - signedOrderRight: SignedOrder, - expectedERC20BalancesByOwner: ERC20BalancesByOwner, - realERC20BalancesByOwner: ERC20BalancesByOwner, - expectedERC721TokenIdsByOwner: ERC721TokenIdsByOwner, - realERC721TokenIdsByOwner: ERC721TokenIdsByOwner, - takerAddress: string - ) { - // Individual balance comparisons - const makerAssetProxyIdLeft = assetDataUtils.decodeAssetProxyId(signedOrderLeft.makerAssetData); - const makerERC20AssetDataLeft = makerAssetProxyIdLeft == AssetProxyId.ERC20 ? assetDataUtils.decodeERC20AssetData(signedOrderLeft.makerAssetData) : assetDataUtils.decodeERC721AssetData(signedOrderLeft.makerAssetData); - const makerAssetAddressLeft = makerERC20AssetDataLeft.tokenAddress; - const makerAssetProxyIdRight = assetDataUtils.decodeAssetProxyId(signedOrderRight.makerAssetData); - const makerERC20AssetDataRight = makerAssetProxyIdRight == AssetProxyId.ERC20 ? assetDataUtils.decodeERC20AssetData(signedOrderRight.makerAssetData) : assetDataUtils.decodeERC721AssetData(signedOrderRight.makerAssetData); - const makerAssetAddressRight = makerERC20AssetDataRight.tokenAddress; - // describe('asdad', () => { - if(makerAssetProxyIdLeft == AssetProxyId.ERC20) { - expect(realERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft], "Checking left maker egress ERC20 account balance").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft]); - expect(realERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft], "Checking right maker ingress ERC20 account balance").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft]); - expect(realERC20BalancesByOwner[takerAddress][makerAssetAddressLeft], "Checking taker ingress ERC20 account balance").to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][makerAssetAddressLeft]); - } else if(makerAssetProxyIdLeft == AssetProxyId.ERC721) { - expect(realERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort(), "Checking left maker egress ERC721 account holdings").to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort()); - expect(realERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort(), "Checking right maker ERC721 account holdings").to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort()); - expect(realERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort(), "Checking taker ingress ERC721 account holdings").to.be.deep.equal(expectedERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort()); - } else { - throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdLeft}`); - } - if(makerAssetProxyIdRight == AssetProxyId.ERC20) { - expect(realERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight], "Checking left maker ingress ERC20 account balance").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight]); - expect(realERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight], "Checking right maker egress ERC20 account balance").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]); - } else if(makerAssetProxyIdRight == AssetProxyId.ERC721) { - expect(realERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort(), "Checking left maker ingress ERC721 account holdings").to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort()); - expect(realERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight], "Checking right maker agress ERC721 account holdings").to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]); - } else { - throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdRight}`); - } - // Paid fees - expect(realERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress], "Checking left maker egress ERC20 account fees").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress]); - expect(realERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress], "Checking right maker egress ERC20 account fees").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress]); - expect(realERC20BalancesByOwner[takerAddress][this._feeTokenAddress], "Checking taker egress ERC20 account fees").to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][this._feeTokenAddress]); - // Received fees - expect(realERC20BalancesByOwner[signedOrderLeft.feeRecipientAddress][this._feeTokenAddress], "Checking left fee recipient ingress ERC20 account fees").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.feeRecipientAddress][this._feeTokenAddress]); - expect(realERC20BalancesByOwner[signedOrderRight.feeRecipientAddress][this._feeTokenAddress], "Checking right fee receipient ingress ERC20 account fees").to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.feeRecipientAddress][this._feeTokenAddress]); - } - /// @dev Compares a pair of ERC20 balances and a pair of ERC721 token owners. /// @param expectedNewERC20BalancesByOwner Expected ERC20 balances. /// @param realERC20BalancesByOwner Actual ERC20 balances. /// @param expectedNewERC721TokenIdsByOwner Expected ERC721 token owners. /// @param realERC721TokenIdsByOwner Actual ERC20 token owners. /// @return True only if ERC20 balances match and ERC721 token owners match. - private async _verifyAllKnownBalancesAsync( + private static async _verifyAllKnownBalancesAsync( expectedNewERC20BalancesByOwner: ERC20BalancesByOwner, realERC20BalancesByOwner: ERC20BalancesByOwner, expectedNewERC721TokenIdsByOwner: ERC721TokenIdsByOwner, realERC721TokenIdsByOwner: ERC721TokenIdsByOwner, - ) { - + ): Promise { // ERC20 Balances - const erc20BalancesMatch = _.isEqual(expectedNewERC20BalancesByOwner, realERC20BalancesByOwner); - expect(erc20BalancesMatch, "Checking all known ERC20 account balances").to.be.true(); - + const areERC20BalancesEqual = _.isEqual(expectedNewERC20BalancesByOwner, realERC20BalancesByOwner); + expect(areERC20BalancesEqual, 'Checking all known ERC20 account balances').to.be.true(); // ERC721 Token Ids const sortedExpectedNewERC721TokenIdsByOwner = _.mapValues( expectedNewERC721TokenIdsByOwner, @@ -137,38 +151,12 @@ export class MatchOrderTester { _.sortBy(tokenIds); }); }); - const erc721TokenIdsMatch = _.isEqual( + const areERC721TokenIdsEqual = _.isEqual( sortedExpectedNewERC721TokenIdsByOwner, sortedNewERC721TokenIdsByOwner, ); - expect(erc721TokenIdsMatch, "Checking all known ERC721 account balances").to.be.true(); + expect(areERC721TokenIdsEqual, 'Checking all known ERC721 account balances').to.be.true(); } - - private async _verifyInitialOrderStates( - signedOrderLeft: SignedOrder, - signedOrderRight: SignedOrder, - initialTakerAssetFilledAmountLeft?: BigNumber, - initialTakerAssetFilledAmountRight?: BigNumber, - ): Promise<[BigNumber, BigNumber]> { - // Verify Left order preconditions - const orderTakerAssetFilledAmountLeft = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( - orderHashUtils.getOrderHashHex(signedOrderLeft), - ); - const expectedOrderFilledAmountLeft = initialTakerAssetFilledAmountLeft - ? initialTakerAssetFilledAmountLeft - : new BigNumber(0); - expect(expectedOrderFilledAmountLeft).to.be.bignumber.equal(orderTakerAssetFilledAmountLeft); - // Verify Right order preconditions - const orderTakerAssetFilledAmountRight = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( - orderHashUtils.getOrderHashHex(signedOrderRight), - ); - const expectedOrderFilledAmountRight = initialTakerAssetFilledAmountRight - ? initialTakerAssetFilledAmountRight - : new BigNumber(0); - expect(expectedOrderFilledAmountRight).to.be.bignumber.equal(orderTakerAssetFilledAmountRight); - return [orderTakerAssetFilledAmountLeft, orderTakerAssetFilledAmountRight]; - } - /// @dev Constructs new MatchOrderTester. /// @param exchangeWrapper Used to call to the Exchange. /// @param erc20Wrapper Used to fetch ERC20 balances. @@ -208,23 +196,27 @@ export class MatchOrderTester { // Verify initial order states let orderTakerAssetFilledAmountLeft: BigNumber; let orderTakerAssetFilledAmountRight: BigNumber; - [orderTakerAssetFilledAmountLeft, orderTakerAssetFilledAmountRight] = await this._verifyInitialOrderStates( + [orderTakerAssetFilledAmountLeft, orderTakerAssetFilledAmountRight] = await this._verifyInitialOrderStatesAsync( signedOrderLeft, signedOrderRight, initialTakerAssetFilledAmountLeft, - initialTakerAssetFilledAmountRight + initialTakerAssetFilledAmountRight, ); // Match left & right orders - const transactionReceipt = await this._exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress); + const transactionReceipt = await this._exchangeWrapper.matchOrdersAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + ); const newERC20BalancesByOwner = await this._erc20Wrapper.getBalancesAsync(); const newERC721TokenIdsByOwner = await this._erc721Wrapper.getBalancesAsync(); // Verify logs - await this._verifyLogsAsync( + await MatchOrderTester._verifyLogsAsync( signedOrderLeft, signedOrderRight, transactionReceipt, takerAddress, - expectedTransferAmounts + expectedTransferAmounts, ); // Verify exchange state await this._verifyExchangeStateAsync( @@ -234,7 +226,7 @@ export class MatchOrderTester { orderTakerAssetFilledAmountRight, expectedTransferAmounts, initialTakerAssetFilledAmountLeft, - initialTakerAssetFilledAmountRight + initialTakerAssetFilledAmountRight, ); // Verify balances of makers, taker, and fee recipients await this._verifyBalancesAsync( @@ -245,57 +237,35 @@ export class MatchOrderTester { newERC20BalancesByOwner, newERC721TokenIdsByOwner, expectedTransferAmounts, - takerAddress + takerAddress, ); return [newERC20BalancesByOwner, newERC721TokenIdsByOwner]; } - /// @dev Calculates expected transfer amounts between order makers, fee recipients, and - /// the taker when two orders are matched. - /// @param signedOrderLeft First matched order. - /// @param signedOrderRight Second matched order. - /// @param orderTakerAssetFilledAmountLeft How much left order has been filled, prior to matching orders. - /// @param orderTakerAssetFilledAmountRight How much the right order has been filled, prior to matching orders. - /// @return TransferAmounts A struct containing the expected transfer amounts. - private async _verifyLogsAsync( + private async _verifyInitialOrderStatesAsync( signedOrderLeft: SignedOrder, signedOrderRight: SignedOrder, - transactionReceipt: TransactionReceiptWithDecodedLogs, - takerAddress: string, - expectedTransferAmounts: TransferAmounts - ) { - // Parse logs - expect(transactionReceipt.logs.length, "Checking number of logs").to.be.equal(2); - // First log is for left fill - const leftLog = ((transactionReceipt.logs[0] as any) as {args: { makerAddress: string, takerAddress: string, makerAssetFilledAmount: string, takerAssetFilledAmount: string, makerFeePaid: string, takerFeePaid: string}}); - expect(leftLog.args.makerAddress, "Checking logged maker address of left order").to.be.equal(signedOrderLeft.makerAddress); - expect(leftLog.args.takerAddress, "Checking logged taker address of right order").to.be.equal(takerAddress); - const amountBoughtByLeftMaker = new BigNumber(leftLog.args.takerAssetFilledAmount); - const amountSoldByLeftMaker = new BigNumber(leftLog.args.makerAssetFilledAmount); - const feePaidByLeftMaker = new BigNumber(leftLog.args.makerFeePaid); - const feePaidByTakerLeft = new BigNumber(leftLog.args.takerFeePaid); - // Second log is for right fill - const rightLog = ((transactionReceipt.logs[1] as any) as {args: { makerAddress: string, takerAddress: string, makerAssetFilledAmount: string, takerAssetFilledAmount: string, makerFeePaid: string, takerFeePaid: string}}); - expect(rightLog.args.makerAddress, "Checking logged maker address of right order").to.be.equal(signedOrderRight.makerAddress); - expect(rightLog.args.takerAddress, "Checking loggerd taker address of right order").to.be.equal(takerAddress); - const amountBoughtByRightMaker = new BigNumber(rightLog.args.takerAssetFilledAmount); - const amountSoldByRightMaker = new BigNumber(rightLog.args.makerAssetFilledAmount); - const feePaidByRightMaker = new BigNumber(rightLog.args.makerFeePaid); - const feePaidByTakerRight = new BigNumber(rightLog.args.takerFeePaid); - // Derive amount received by taker - const amountReceivedByTaker = amountSoldByLeftMaker.sub(amountBoughtByRightMaker); - // Verify log values - left order - expect(expectedTransferAmounts.amountBoughtByLeftMaker, "Checking logged amount bought by left maker").to.be.bignumber.equal(amountBoughtByLeftMaker); - expect(expectedTransferAmounts.amountSoldByLeftMaker, "Checking logged amount sold by left maker").to.be.bignumber.equal(amountSoldByLeftMaker); - expect(expectedTransferAmounts.feePaidByLeftMaker, "Checking logged fee paid by left maker").to.be.bignumber.equal(feePaidByLeftMaker); - expect(expectedTransferAmounts.feePaidByTakerLeft, "Checking logged fee paid on left order by taker").to.be.bignumber.equal(feePaidByTakerLeft); - // Verify log values - right order - expect(expectedTransferAmounts.amountBoughtByRightMaker, "Checking logged amount bought by right maker").to.be.bignumber.equal(amountBoughtByRightMaker); - expect(expectedTransferAmounts.amountSoldByRightMaker, "Checking logged amount sold by right maker").to.be.bignumber.equal(amountSoldByRightMaker); - expect(expectedTransferAmounts.feePaidByRightMaker, "Checking logged fee paid by right maker").to.be.bignumber.equal(feePaidByRightMaker); - expect(expectedTransferAmounts.feePaidByTakerRight, "Checking logged fee paid on right order by taker").to.be.bignumber.equal(feePaidByTakerRight); - // Verify derived amount received by taker - expect(expectedTransferAmounts.amountReceivedByTaker, "Checking logged amount received by taker").to.be.bignumber.equal(amountReceivedByTaker); + initialTakerAssetFilledAmountLeft?: BigNumber, + initialTakerAssetFilledAmountRight?: BigNumber, + ): Promise<[BigNumber, BigNumber]> { + // Verify Left order preconditions + const orderTakerAssetFilledAmountLeft = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( + orderHashUtils.getOrderHashHex(signedOrderLeft), + ); + const expectedOrderFilledAmountLeft = initialTakerAssetFilledAmountLeft + ? initialTakerAssetFilledAmountLeft + : new BigNumber(0); + expect(expectedOrderFilledAmountLeft).to.be.bignumber.equal(orderTakerAssetFilledAmountLeft); + // Verify Right order preconditions + const orderTakerAssetFilledAmountRight = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( + orderHashUtils.getOrderHashHex(signedOrderRight), + ); + const expectedOrderFilledAmountRight = initialTakerAssetFilledAmountRight + ? initialTakerAssetFilledAmountRight + : new BigNumber(0); + expect(expectedOrderFilledAmountRight).to.be.bignumber.equal(orderTakerAssetFilledAmountRight); + return [orderTakerAssetFilledAmountLeft, orderTakerAssetFilledAmountRight]; } + /// @dev Calculates expected transfer amounts between order makers, fee recipients, and /// the taker when two orders are matched. /// @param signedOrderLeft First matched order. @@ -310,34 +280,193 @@ export class MatchOrderTester { orderTakerAssetFilledAmountRight: BigNumber, expectedTransferAmounts: TransferAmounts, initialTakerAssetFilledAmountLeft?: BigNumber, - initialTakerAssetFilledAmountRight?: BigNumber - ) { + initialTakerAssetFilledAmountRight?: BigNumber, + ): Promise { // Verify state for left order: amount bought by left maker let amountBoughtByLeftMaker = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderLeft), ); amountBoughtByLeftMaker = amountBoughtByLeftMaker.minus(orderTakerAssetFilledAmountLeft); - expect(expectedTransferAmounts.amountBoughtByLeftMaker, "Checking exchange state for left order").to.be.bignumber.equal(amountBoughtByLeftMaker); + expect( + expectedTransferAmounts.amountBoughtByLeftMaker, + 'Checking exchange state for left order', + ).to.be.bignumber.equal(amountBoughtByLeftMaker); // Verify state for right order: amount bought by right maker let amountBoughtByRightMaker = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderRight), ); amountBoughtByRightMaker = amountBoughtByRightMaker.minus(orderTakerAssetFilledAmountRight); - expect(expectedTransferAmounts.amountBoughtByRightMaker, "Checking exchange state for right order").to.be.bignumber.equal(amountBoughtByRightMaker); + expect( + expectedTransferAmounts.amountBoughtByRightMaker, + 'Checking exchange state for right order', + ).to.be.bignumber.equal(amountBoughtByRightMaker); // Verify left order status const maxAmountBoughtByLeftMaker = initialTakerAssetFilledAmountLeft ? signedOrderLeft.takerAssetAmount.sub(initialTakerAssetFilledAmountLeft) : signedOrderLeft.takerAssetAmount; const leftOrderInfo: OrderInfo = await this._exchangeWrapper.getOrderInfoAsync(signedOrderLeft); - const leftExpectedStatus = (expectedTransferAmounts.amountBoughtByLeftMaker.equals(maxAmountBoughtByLeftMaker)) ? OrderStatus.FULLY_FILLED : OrderStatus.FILLABLE; - expect(leftOrderInfo.orderStatus as OrderStatus, "Checking exchange status for left order").to.be.equal(leftExpectedStatus); + const leftExpectedStatus = expectedTransferAmounts.amountBoughtByLeftMaker.equals(maxAmountBoughtByLeftMaker) + ? OrderStatus.FULLY_FILLED + : OrderStatus.FILLABLE; + expect(leftOrderInfo.orderStatus as OrderStatus, 'Checking exchange status for left order').to.be.equal( + leftExpectedStatus, + ); // Verify right order status const maxAmountBoughtByRightMaker = initialTakerAssetFilledAmountRight ? signedOrderRight.takerAssetAmount.sub(initialTakerAssetFilledAmountRight) : signedOrderRight.takerAssetAmount; const rightOrderInfo: OrderInfo = await this._exchangeWrapper.getOrderInfoAsync(signedOrderRight); - const rightExpectedStatus = (expectedTransferAmounts.amountBoughtByRightMaker.equals(maxAmountBoughtByRightMaker)) ? OrderStatus.FULLY_FILLED : OrderStatus.FILLABLE; - expect(rightOrderInfo.orderStatus as OrderStatus, "Checking exchange status for right order").to.be.equal(rightExpectedStatus); + const rightExpectedStatus = expectedTransferAmounts.amountBoughtByRightMaker.equals(maxAmountBoughtByRightMaker) + ? OrderStatus.FULLY_FILLED + : OrderStatus.FILLABLE; + expect(rightOrderInfo.orderStatus as OrderStatus, 'Checking exchange status for right order').to.be.equal( + rightExpectedStatus, + ); + } + private async _verifyBalancesAsync( + signedOrderLeft: SignedOrder, + signedOrderRight: SignedOrder, + initialERC20BalancesByOwner: ERC20BalancesByOwner, + initialERC721TokenIdsByOwner: ERC721TokenIdsByOwner, + finalERC20BalancesByOwner: ERC20BalancesByOwner, + finalERC721TokenIdsByOwner: ERC721TokenIdsByOwner, + expectedTransferAmounts: TransferAmounts, + takerAddress: string, + ): Promise { + let expectedERC20BalancesByOwner: ERC20BalancesByOwner; + let expectedERC721TokenIdsByOwner: ERC721TokenIdsByOwner; + [expectedERC20BalancesByOwner, expectedERC721TokenIdsByOwner] = this._calculateExpectedBalances( + signedOrderLeft, + signedOrderRight, + takerAddress, + initialERC20BalancesByOwner, + initialERC721TokenIdsByOwner, + expectedTransferAmounts, + ); + // Verify balances of makers, taker, and fee recipients + await this._verifyMakerTakerAndFeeRecipientBalancesAsync( + signedOrderLeft, + signedOrderRight, + expectedERC20BalancesByOwner, + finalERC20BalancesByOwner, + expectedERC721TokenIdsByOwner, + finalERC721TokenIdsByOwner, + takerAddress, + ); + // Verify balances for all known accounts + await MatchOrderTester._verifyAllKnownBalancesAsync( + expectedERC20BalancesByOwner, + finalERC20BalancesByOwner, + expectedERC721TokenIdsByOwner, + finalERC721TokenIdsByOwner, + ); + } + private async _verifyMakerTakerAndFeeRecipientBalancesAsync( + signedOrderLeft: SignedOrder, + signedOrderRight: SignedOrder, + expectedERC20BalancesByOwner: ERC20BalancesByOwner, + realERC20BalancesByOwner: ERC20BalancesByOwner, + expectedERC721TokenIdsByOwner: ERC721TokenIdsByOwner, + realERC721TokenIdsByOwner: ERC721TokenIdsByOwner, + takerAddress: string, + ): Promise { + // Individual balance comparisons + const makerAssetProxyIdLeft = assetDataUtils.decodeAssetProxyId(signedOrderLeft.makerAssetData); + const makerERC20AssetDataLeft = + makerAssetProxyIdLeft === AssetProxyId.ERC20 + ? assetDataUtils.decodeERC20AssetData(signedOrderLeft.makerAssetData) + : assetDataUtils.decodeERC721AssetData(signedOrderLeft.makerAssetData); + const makerAssetAddressLeft = makerERC20AssetDataLeft.tokenAddress; + const makerAssetProxyIdRight = assetDataUtils.decodeAssetProxyId(signedOrderRight.makerAssetData); + const makerERC20AssetDataRight = + makerAssetProxyIdRight === AssetProxyId.ERC20 + ? assetDataUtils.decodeERC20AssetData(signedOrderRight.makerAssetData) + : assetDataUtils.decodeERC721AssetData(signedOrderRight.makerAssetData); + const makerAssetAddressRight = makerERC20AssetDataRight.tokenAddress; + if (makerAssetProxyIdLeft === AssetProxyId.ERC20) { + expect( + realERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft], + 'Checking left maker egress ERC20 account balance', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft]); + expect( + realERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft], + 'Checking right maker ingress ERC20 account balance', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft]); + expect( + realERC20BalancesByOwner[takerAddress][makerAssetAddressLeft], + 'Checking taker ingress ERC20 account balance', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][makerAssetAddressLeft]); + } else if (makerAssetProxyIdLeft === AssetProxyId.ERC721) { + expect( + realERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort(), + 'Checking left maker egress ERC721 account holdings', + ).to.be.deep.equal( + expectedERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort(), + ); + expect( + realERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort(), + 'Checking right maker ERC721 account holdings', + ).to.be.deep.equal( + expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort(), + ); + expect( + realERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort(), + 'Checking taker ingress ERC721 account holdings', + ).to.be.deep.equal(expectedERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort()); + } else { + throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdLeft}`); + } + if (makerAssetProxyIdRight === AssetProxyId.ERC20) { + expect( + realERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight], + 'Checking left maker ingress ERC20 account balance', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight]); + expect( + realERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight], + 'Checking right maker egress ERC20 account balance', + ).to.be.bignumber.equal( + expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight], + ); + } else if (makerAssetProxyIdRight === AssetProxyId.ERC721) { + expect( + realERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort(), + 'Checking left maker ingress ERC721 account holdings', + ).to.be.deep.equal( + expectedERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort(), + ); + expect( + realERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight], + 'Checking right maker agress ERC721 account holdings', + ).to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]); + } else { + throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdRight}`); + } + // Paid fees + expect( + realERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress], + 'Checking left maker egress ERC20 account fees', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress]); + expect( + realERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress], + 'Checking right maker egress ERC20 account fees', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress]); + expect( + realERC20BalancesByOwner[takerAddress][this._feeTokenAddress], + 'Checking taker egress ERC20 account fees', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][this._feeTokenAddress]); + // Received fees + expect( + realERC20BalancesByOwner[signedOrderLeft.feeRecipientAddress][this._feeTokenAddress], + 'Checking left fee recipient ingress ERC20 account fees', + ).to.be.bignumber.equal( + expectedERC20BalancesByOwner[signedOrderLeft.feeRecipientAddress][this._feeTokenAddress], + ); + expect( + realERC20BalancesByOwner[signedOrderRight.feeRecipientAddress][this._feeTokenAddress], + 'Checking right fee receipient ingress ERC20 account fees', + ).to.be.bignumber.equal( + expectedERC20BalancesByOwner[signedOrderRight.feeRecipientAddress][this._feeTokenAddress], + ); } /// @dev Calculates the expected balances of order makers, fee recipients, and the taker, /// as a result of matching two orders. @@ -438,7 +567,9 @@ export class MatchOrderTester { // Taker Fees expectedNewERC20BalancesByOwner[takerAddress][this._feeTokenAddress] = expectedNewERC20BalancesByOwner[ takerAddress - ][this._feeTokenAddress].minus(expectedTransferAmounts.feePaidByTakerLeft.add(expectedTransferAmounts.feePaidByTakerRight)); + ][this._feeTokenAddress].minus( + expectedTransferAmounts.feePaidByTakerLeft.add(expectedTransferAmounts.feePaidByTakerRight), + ); // Left Fee Recipient Fees expectedNewERC20BalancesByOwner[feeRecipientAddressLeft][ this._feeTokenAddress @@ -454,4 +585,4 @@ export class MatchOrderTester { return [expectedNewERC20BalancesByOwner, expectedNewERC721TokenIdsByOwner]; } -} +} // tslint:disable-line:max-file-line-count diff --git a/packages/contracts/test/utils/types.ts b/packages/contracts/test/utils/types.ts index 481ee87d6..172622777 100644 --- a/packages/contracts/test/utils/types.ts +++ b/packages/contracts/test/utils/types.ts @@ -117,21 +117,21 @@ export interface TransferAmountsByMatchOrders { // Left Maker amountBoughtByLeftMaker: BigNumber; amountSoldByLeftMaker: BigNumber; - amountReceivedByLeftMaker: BigNumber; + // amountReceivedByLeftMaker: BigNumber; feePaidByLeftMaker: BigNumber; // Right Maker amountBoughtByRightMaker: BigNumber; amountSoldByRightMaker: BigNumber; - amountReceivedByRightMaker: BigNumber; + // amountReceivedByRightMaker: BigNumber; feePaidByRightMaker: BigNumber; // Taker amountReceivedByTaker: BigNumber; feePaidByTakerLeft: BigNumber; feePaidByTakerRight: BigNumber; - totalFeePaidByTaker: BigNumber; + // totalFeePaidByTaker: BigNumber; // Fee Recipients - feeReceivedLeft: BigNumber; - feeReceivedRight: BigNumber; + // feeReceivedLeft: BigNumber; + // feeReceivedRight: BigNumber; } export interface OrderInfo { -- cgit v1.2.3 From ac872e518197573fddaded73745e58b4235f8af3 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 10:40:02 -0700 Subject: Added `expect` messages for checking left/right order states --- packages/contracts/test/utils/match_order_tester.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index 300073eb0..98bf80011 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -254,7 +254,9 @@ export class MatchOrderTester { const expectedOrderFilledAmountLeft = initialTakerAssetFilledAmountLeft ? initialTakerAssetFilledAmountLeft : new BigNumber(0); - expect(expectedOrderFilledAmountLeft).to.be.bignumber.equal(orderTakerAssetFilledAmountLeft); + expect(expectedOrderFilledAmountLeft, 'Checking inital state of left order').to.be.bignumber.equal( + orderTakerAssetFilledAmountLeft, + ); // Verify Right order preconditions const orderTakerAssetFilledAmountRight = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderRight), @@ -262,7 +264,9 @@ export class MatchOrderTester { const expectedOrderFilledAmountRight = initialTakerAssetFilledAmountRight ? initialTakerAssetFilledAmountRight : new BigNumber(0); - expect(expectedOrderFilledAmountRight).to.be.bignumber.equal(orderTakerAssetFilledAmountRight); + expect(expectedOrderFilledAmountRight, 'Checking inital state of right order').to.be.bignumber.equal( + orderTakerAssetFilledAmountRight, + ); return [orderTakerAssetFilledAmountLeft, orderTakerAssetFilledAmountRight]; } -- cgit v1.2.3 From 9a5ec8d030c9e9b08018abac54a8d16933affabc Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 11:38:27 -0700 Subject: Added function signature comments --- packages/contracts/test/exchange/match_orders.ts | 54 +-- .../contracts/test/utils/match_order_tester.ts | 403 ++++++++++----------- packages/contracts/test/utils/types.ts | 15 +- 3 files changed, 233 insertions(+), 239 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 7e9776d52..58b8250ab 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -228,15 +228,15 @@ describe.only('matchOrders', () => { // Left Maker amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 0), amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), - feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 18), // 100% + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Right Maker amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 16), // 75% // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(9), 0), - feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 18), // 100% - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 17), // 50% + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 16), // 50% }; // Match signedOrderLeft with signedOrderRight await matchOrderTester.matchOrdersAndVerifyBalancesAsync( @@ -300,8 +300,8 @@ describe.only('matchOrders', () => { feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), - feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, @@ -335,8 +335,8 @@ describe.only('matchOrders', () => { feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(0), 18), - feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; // Match signedOrderLeft with signedOrderRight await matchOrderTester.matchOrdersAndVerifyBalancesAsync( @@ -371,8 +371,8 @@ describe.only('matchOrders', () => { feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 16), // 50% // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), - feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 40% - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 16), // 100% + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 16), // 50% }; // Match signedOrderLeft with signedOrderRight await matchOrderTester.matchOrdersAndVerifyBalancesAsync( @@ -407,8 +407,8 @@ describe.only('matchOrders', () => { feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), - feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 16), // 100% - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 50% + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 16), // 10% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; // Match signedOrderLeft with signedOrderRight await matchOrderTester.matchOrdersAndVerifyBalancesAsync( @@ -445,8 +445,8 @@ describe.only('matchOrders', () => { feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), - feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 16), // 100% - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 50% + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 16), // 10% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; // prettier-ignore [ @@ -604,8 +604,8 @@ describe.only('matchOrders', () => { feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), - feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, @@ -640,8 +640,8 @@ describe.only('matchOrders', () => { feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), - feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, @@ -676,8 +676,8 @@ describe.only('matchOrders', () => { feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), - feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, @@ -712,8 +712,8 @@ describe.only('matchOrders', () => { feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), - feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, @@ -748,8 +748,8 @@ describe.only('matchOrders', () => { feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), - feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, @@ -783,8 +783,8 @@ describe.only('matchOrders', () => { feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 18), - feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, @@ -929,7 +929,7 @@ describe.only('matchOrders', () => { ); }); - it.only('should transfer correct amounts when right order maker asset is an ERC721 token', async () => { + 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({ @@ -955,7 +955,7 @@ describe.only('matchOrders', () => { // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 50% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index 98bf80011..7d763c6df 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -16,6 +16,7 @@ import { OrderInfo, OrderStatus, TransferAmountsByMatchOrders as TransferAmounts, + TransferAmountsLoggedByMatchOrders as LoggedTransferAmounts, } from './types'; chaiSetup.configure(); @@ -26,13 +27,14 @@ export class MatchOrderTester { private readonly _erc20Wrapper: ERC20Wrapper; private readonly _erc721Wrapper: ERC721Wrapper; private readonly _feeTokenAddress: string; - /// @dev Calculates expected transfer amounts between order makers, fee recipients, and - /// the taker when two orders are matched. + /// @dev Checks values from the logs produced by Exchange.matchOrders against the expected transfer amounts. + /// Values include the amounts transferred from the left/right makers and taker, along with + /// the fees paid on each matched order. These are also the return values of MatchOrders. /// @param signedOrderLeft First matched order. /// @param signedOrderRight Second matched order. - /// @param orderTakerAssetFilledAmountLeft How much left order has been filled, prior to matching orders. - /// @param orderTakerAssetFilledAmountRight How much the right order has been filled, prior to matching orders. - /// @return TransferAmounts A struct containing the expected transfer amounts. + /// @param transactionReceipt Transaction receipt and logs produced by Exchange.matchOrders. + /// @param takerAddress Address of taker (account that called Exchange.matchOrders) + /// @param expectedTransferAmounts Expected amounts transferred as a result of order matching. private static async _verifyLogsAsync( signedOrderLeft: SignedOrder, signedOrderRight: SignedOrder, @@ -40,46 +42,28 @@ export class MatchOrderTester { takerAddress: string, expectedTransferAmounts: TransferAmounts, ): Promise { - // Parse logs + // Should have two logs -- one for each order. expect(transactionReceipt.logs.length, 'Checking number of logs').to.be.equal(2); // First log is for left fill - const leftLog = (transactionReceipt.logs[0] as any) as { - args: { - makerAddress: string; - takerAddress: string; - makerAssetFilledAmount: string; - takerAssetFilledAmount: string; - makerFeePaid: string; - takerFeePaid: string; - }; - }; - expect(leftLog.args.makerAddress, 'Checking logged maker address of left order').to.be.equal( + const leftLog = (transactionReceipt.logs[0] as any).args as LoggedTransferAmounts; + expect(leftLog.makerAddress, 'Checking logged maker address of left order').to.be.equal( signedOrderLeft.makerAddress, ); - expect(leftLog.args.takerAddress, 'Checking logged taker address of right order').to.be.equal(takerAddress); - const amountBoughtByLeftMaker = new BigNumber(leftLog.args.takerAssetFilledAmount); - const amountSoldByLeftMaker = new BigNumber(leftLog.args.makerAssetFilledAmount); - const feePaidByLeftMaker = new BigNumber(leftLog.args.makerFeePaid); - const feePaidByTakerLeft = new BigNumber(leftLog.args.takerFeePaid); + expect(leftLog.takerAddress, 'Checking logged taker address of right order').to.be.equal(takerAddress); + const amountBoughtByLeftMaker = new BigNumber(leftLog.takerAssetFilledAmount); + const amountSoldByLeftMaker = new BigNumber(leftLog.makerAssetFilledAmount); + const feePaidByLeftMaker = new BigNumber(leftLog.makerFeePaid); + const feePaidByTakerLeft = new BigNumber(leftLog.takerFeePaid); // Second log is for right fill - const rightLog = (transactionReceipt.logs[1] as any) as { - args: { - makerAddress: string; - takerAddress: string; - makerAssetFilledAmount: string; - takerAssetFilledAmount: string; - makerFeePaid: string; - takerFeePaid: string; - }; - }; - expect(rightLog.args.makerAddress, 'Checking logged maker address of right order').to.be.equal( + const rightLog = (transactionReceipt.logs[1] as any).args as LoggedTransferAmounts; + expect(rightLog.makerAddress, 'Checking logged maker address of right order').to.be.equal( signedOrderRight.makerAddress, ); - expect(rightLog.args.takerAddress, 'Checking loggerd taker address of right order').to.be.equal(takerAddress); - const amountBoughtByRightMaker = new BigNumber(rightLog.args.takerAssetFilledAmount); - const amountSoldByRightMaker = new BigNumber(rightLog.args.makerAssetFilledAmount); - const feePaidByRightMaker = new BigNumber(rightLog.args.makerFeePaid); - const feePaidByTakerRight = new BigNumber(rightLog.args.takerFeePaid); + expect(rightLog.takerAddress, 'Checking loggerd taker address of right order').to.be.equal(takerAddress); + const amountBoughtByRightMaker = new BigNumber(rightLog.takerAssetFilledAmount); + const amountSoldByRightMaker = new BigNumber(rightLog.makerAssetFilledAmount); + const feePaidByRightMaker = new BigNumber(rightLog.makerFeePaid); + const feePaidByTakerRight = new BigNumber(rightLog.takerFeePaid); // Derive amount received by taker const amountReceivedByTaker = amountSoldByLeftMaker.sub(amountBoughtByRightMaker); // Verify log values - left order @@ -122,30 +106,26 @@ export class MatchOrderTester { 'Checking logged amount received by taker', ).to.be.bignumber.equal(amountReceivedByTaker); } - /// @dev Compares a pair of ERC20 balances and a pair of ERC721 token owners. - /// @param expectedNewERC20BalancesByOwner Expected ERC20 balances. - /// @param realERC20BalancesByOwner Actual ERC20 balances. - /// @param expectedNewERC721TokenIdsByOwner Expected ERC721 token owners. - /// @param realERC721TokenIdsByOwner Actual ERC20 token owners. - /// @return True only if ERC20 balances match and ERC721 token owners match. + /// @dev Verifies all expected ERC20 and ERC721 account holdings match the real holdings. + /// @param expectedERC20BalancesByOwner Expected ERC20 balances. + /// @param realERC20BalancesByOwner Real ERC20 balances. + /// @param expectedERC721TokenIdsByOwner Expected ERC721 token owners. + /// @param realERC721TokenIdsByOwner Real ERC20 token owners. private static async _verifyAllKnownBalancesAsync( - expectedNewERC20BalancesByOwner: ERC20BalancesByOwner, + expectedERC20BalancesByOwner: ERC20BalancesByOwner, realERC20BalancesByOwner: ERC20BalancesByOwner, - expectedNewERC721TokenIdsByOwner: ERC721TokenIdsByOwner, + expectedERC721TokenIdsByOwner: ERC721TokenIdsByOwner, realERC721TokenIdsByOwner: ERC721TokenIdsByOwner, ): Promise { // ERC20 Balances - const areERC20BalancesEqual = _.isEqual(expectedNewERC20BalancesByOwner, realERC20BalancesByOwner); + const areERC20BalancesEqual = _.isEqual(expectedERC20BalancesByOwner, realERC20BalancesByOwner); expect(areERC20BalancesEqual, 'Checking all known ERC20 account balances').to.be.true(); // ERC721 Token Ids - const sortedExpectedNewERC721TokenIdsByOwner = _.mapValues( - expectedNewERC721TokenIdsByOwner, - tokenIdsByOwner => { - _.mapValues(tokenIdsByOwner, tokenIds => { - _.sortBy(tokenIds); - }); - }, - ); + const sortedExpectedNewERC721TokenIdsByOwner = _.mapValues(expectedERC721TokenIdsByOwner, tokenIdsByOwner => { + _.mapValues(tokenIdsByOwner, tokenIds => { + _.sortBy(tokenIds); + }); + }); const sortedNewERC721TokenIdsByOwner = _.mapValues(realERC721TokenIdsByOwner, tokenIdsByOwner => { _.mapValues(tokenIdsByOwner, tokenIds => { _.sortBy(tokenIds); @@ -173,15 +153,16 @@ export class MatchOrderTester { this._erc721Wrapper = erc721Wrapper; this._feeTokenAddress = feeTokenAddress; } - /// @dev Matches two complementary orders and validates results. + /// @dev Matches two complementary orders and verifies results. /// Validation either succeeds or throws. /// @param signedOrderLeft First matched order. /// @param signedOrderRight Second matched order. /// @param takerAddress Address of taker (the address who matched the two orders) /// @param erc20BalancesByOwner Current ERC20 balances. /// @param erc721TokenIdsByOwner Current ERC721 token owners. - /// @param initialTakerAssetFilledAmountLeft Current amount the left order has been filled. - /// @param initialTakerAssetFilledAmountRight Current amount the right order has been filled. + /// @param expectedTransferAmounts Expected amounts transferred as a result of order matching. + /// @param initialLeftOrderFilledAmount How much left order has been filled, prior to matching orders. + /// @param initialRightOrderFilledAmount How much the right order has been filled, prior to matching orders. /// @return New ERC20 balances & ERC721 token owners. public async matchOrdersAndVerifyBalancesAsync( signedOrderLeft: SignedOrder, @@ -190,17 +171,22 @@ export class MatchOrderTester { erc20BalancesByOwner: ERC20BalancesByOwner, erc721TokenIdsByOwner: ERC721TokenIdsByOwner, expectedTransferAmounts: TransferAmounts, - initialTakerAssetFilledAmountLeft?: BigNumber, - initialTakerAssetFilledAmountRight?: BigNumber, + initialLeftOrderFilledAmount?: BigNumber, + initialRightOrderFilledAmount?: BigNumber, ): Promise<[ERC20BalancesByOwner, ERC721TokenIdsByOwner]> { + // Assign default values to optional parameters if undefined + if (initialLeftOrderFilledAmount === undefined) { + initialLeftOrderFilledAmount = new BigNumber(0); + } + if (initialRightOrderFilledAmount === undefined) { + initialRightOrderFilledAmount = new BigNumber(0); + } // Verify initial order states - let orderTakerAssetFilledAmountLeft: BigNumber; - let orderTakerAssetFilledAmountRight: BigNumber; - [orderTakerAssetFilledAmountLeft, orderTakerAssetFilledAmountRight] = await this._verifyInitialOrderStatesAsync( + await this._verifyInitialOrderStatesAsync( signedOrderLeft, signedOrderRight, - initialTakerAssetFilledAmountLeft, - initialTakerAssetFilledAmountRight, + initialLeftOrderFilledAmount, + initialRightOrderFilledAmount, ); // Match left & right orders const transactionReceipt = await this._exchangeWrapper.matchOrdersAsync( @@ -222,11 +208,9 @@ export class MatchOrderTester { await this._verifyExchangeStateAsync( signedOrderLeft, signedOrderRight, - orderTakerAssetFilledAmountLeft, - orderTakerAssetFilledAmountRight, + initialLeftOrderFilledAmount, + initialRightOrderFilledAmount, expectedTransferAmounts, - initialTakerAssetFilledAmountLeft, - initialTakerAssetFilledAmountRight, ); // Verify balances of makers, taker, and fee recipients await this._verifyBalancesAsync( @@ -241,56 +225,50 @@ export class MatchOrderTester { ); return [newERC20BalancesByOwner, newERC721TokenIdsByOwner]; } + /// @dev Verifies initial exchange state for the left and right orders. + /// @param signedOrderLeft First matched order. + /// @param signedOrderRight Second matched order. + /// @param expectedOrderFilledAmountLeft How much left order has been filled, prior to matching orders. + /// @param expectedOrderFilledAmountRight How much the right order has been filled, prior to matching orders. private async _verifyInitialOrderStatesAsync( signedOrderLeft: SignedOrder, signedOrderRight: SignedOrder, - initialTakerAssetFilledAmountLeft?: BigNumber, - initialTakerAssetFilledAmountRight?: BigNumber, - ): Promise<[BigNumber, BigNumber]> { - // Verify Left order preconditions + expectedOrderFilledAmountLeft: BigNumber, + expectedOrderFilledAmountRight: BigNumber, + ): Promise { + // Verify left order initial state const orderTakerAssetFilledAmountLeft = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderLeft), ); - const expectedOrderFilledAmountLeft = initialTakerAssetFilledAmountLeft - ? initialTakerAssetFilledAmountLeft - : new BigNumber(0); expect(expectedOrderFilledAmountLeft, 'Checking inital state of left order').to.be.bignumber.equal( orderTakerAssetFilledAmountLeft, ); - // Verify Right order preconditions + // Verify right order initial state const orderTakerAssetFilledAmountRight = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderRight), ); - const expectedOrderFilledAmountRight = initialTakerAssetFilledAmountRight - ? initialTakerAssetFilledAmountRight - : new BigNumber(0); expect(expectedOrderFilledAmountRight, 'Checking inital state of right order').to.be.bignumber.equal( orderTakerAssetFilledAmountRight, ); - return [orderTakerAssetFilledAmountLeft, orderTakerAssetFilledAmountRight]; } - - /// @dev Calculates expected transfer amounts between order makers, fee recipients, and - /// the taker when two orders are matched. + /// @dev Verifies the exchange state against the expected amounts transferred by from matching orders. /// @param signedOrderLeft First matched order. /// @param signedOrderRight Second matched order. - /// @param orderTakerAssetFilledAmountLeft How much left order has been filled, prior to matching orders. - /// @param orderTakerAssetFilledAmountRight How much the right order has been filled, prior to matching orders. + /// @param initialLeftOrderFilledAmount How much left order has been filled, prior to matching orders. + /// @param initialRightOrderFilledAmount How much the right order has been filled, prior to matching orders. /// @return TransferAmounts A struct containing the expected transfer amounts. private async _verifyExchangeStateAsync( signedOrderLeft: SignedOrder, signedOrderRight: SignedOrder, - orderTakerAssetFilledAmountLeft: BigNumber, - orderTakerAssetFilledAmountRight: BigNumber, + initialLeftOrderFilledAmount: BigNumber, + initialRightOrderFilledAmount: BigNumber, expectedTransferAmounts: TransferAmounts, - initialTakerAssetFilledAmountLeft?: BigNumber, - initialTakerAssetFilledAmountRight?: BigNumber, ): Promise { // Verify state for left order: amount bought by left maker let amountBoughtByLeftMaker = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderLeft), ); - amountBoughtByLeftMaker = amountBoughtByLeftMaker.minus(orderTakerAssetFilledAmountLeft); + amountBoughtByLeftMaker = amountBoughtByLeftMaker.minus(initialLeftOrderFilledAmount); expect( expectedTransferAmounts.amountBoughtByLeftMaker, 'Checking exchange state for left order', @@ -299,15 +277,13 @@ export class MatchOrderTester { let amountBoughtByRightMaker = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderRight), ); - amountBoughtByRightMaker = amountBoughtByRightMaker.minus(orderTakerAssetFilledAmountRight); + amountBoughtByRightMaker = amountBoughtByRightMaker.minus(initialRightOrderFilledAmount); expect( expectedTransferAmounts.amountBoughtByRightMaker, 'Checking exchange state for right order', ).to.be.bignumber.equal(amountBoughtByRightMaker); // Verify left order status - const maxAmountBoughtByLeftMaker = initialTakerAssetFilledAmountLeft - ? signedOrderLeft.takerAssetAmount.sub(initialTakerAssetFilledAmountLeft) - : signedOrderLeft.takerAssetAmount; + const maxAmountBoughtByLeftMaker = signedOrderLeft.takerAssetAmount.minus(initialLeftOrderFilledAmount); const leftOrderInfo: OrderInfo = await this._exchangeWrapper.getOrderInfoAsync(signedOrderLeft); const leftExpectedStatus = expectedTransferAmounts.amountBoughtByLeftMaker.equals(maxAmountBoughtByLeftMaker) ? OrderStatus.FULLY_FILLED @@ -316,9 +292,7 @@ export class MatchOrderTester { leftExpectedStatus, ); // Verify right order status - const maxAmountBoughtByRightMaker = initialTakerAssetFilledAmountRight - ? signedOrderRight.takerAssetAmount.sub(initialTakerAssetFilledAmountRight) - : signedOrderRight.takerAssetAmount; + const maxAmountBoughtByRightMaker = signedOrderRight.takerAssetAmount.minus(initialRightOrderFilledAmount); const rightOrderInfo: OrderInfo = await this._exchangeWrapper.getOrderInfoAsync(signedOrderRight); const rightExpectedStatus = expectedTransferAmounts.amountBoughtByRightMaker.equals(maxAmountBoughtByRightMaker) ? OrderStatus.FULLY_FILLED @@ -327,6 +301,15 @@ export class MatchOrderTester { rightExpectedStatus, ); } + /// @dev Verifies account balances after matching orders. + /// @param signedOrderLeft First matched order. + /// @param signedOrderRight Second matched order. + /// @param initialERC20BalancesByOwner ERC20 balances prior to order matching. + /// @param initialERC721TokenIdsByOwner ERC721 token owners prior to order matching. + /// @param finalERC20BalancesByOwner ERC20 balances after order matching. + /// @param finalERC721TokenIdsByOwner ERC721 token owners after order matching. + /// @param expectedTransferAmounts Expected amounts transferred as a result of order matching. + /// @param takerAddress Address of taker (account that called Exchange.matchOrders). private async _verifyBalancesAsync( signedOrderLeft: SignedOrder, signedOrderRight: SignedOrder, @@ -365,113 +348,6 @@ export class MatchOrderTester { finalERC721TokenIdsByOwner, ); } - private async _verifyMakerTakerAndFeeRecipientBalancesAsync( - signedOrderLeft: SignedOrder, - signedOrderRight: SignedOrder, - expectedERC20BalancesByOwner: ERC20BalancesByOwner, - realERC20BalancesByOwner: ERC20BalancesByOwner, - expectedERC721TokenIdsByOwner: ERC721TokenIdsByOwner, - realERC721TokenIdsByOwner: ERC721TokenIdsByOwner, - takerAddress: string, - ): Promise { - // Individual balance comparisons - const makerAssetProxyIdLeft = assetDataUtils.decodeAssetProxyId(signedOrderLeft.makerAssetData); - const makerERC20AssetDataLeft = - makerAssetProxyIdLeft === AssetProxyId.ERC20 - ? assetDataUtils.decodeERC20AssetData(signedOrderLeft.makerAssetData) - : assetDataUtils.decodeERC721AssetData(signedOrderLeft.makerAssetData); - const makerAssetAddressLeft = makerERC20AssetDataLeft.tokenAddress; - const makerAssetProxyIdRight = assetDataUtils.decodeAssetProxyId(signedOrderRight.makerAssetData); - const makerERC20AssetDataRight = - makerAssetProxyIdRight === AssetProxyId.ERC20 - ? assetDataUtils.decodeERC20AssetData(signedOrderRight.makerAssetData) - : assetDataUtils.decodeERC721AssetData(signedOrderRight.makerAssetData); - const makerAssetAddressRight = makerERC20AssetDataRight.tokenAddress; - if (makerAssetProxyIdLeft === AssetProxyId.ERC20) { - expect( - realERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft], - 'Checking left maker egress ERC20 account balance', - ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft]); - expect( - realERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft], - 'Checking right maker ingress ERC20 account balance', - ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft]); - expect( - realERC20BalancesByOwner[takerAddress][makerAssetAddressLeft], - 'Checking taker ingress ERC20 account balance', - ).to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][makerAssetAddressLeft]); - } else if (makerAssetProxyIdLeft === AssetProxyId.ERC721) { - expect( - realERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort(), - 'Checking left maker egress ERC721 account holdings', - ).to.be.deep.equal( - expectedERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort(), - ); - expect( - realERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort(), - 'Checking right maker ERC721 account holdings', - ).to.be.deep.equal( - expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort(), - ); - expect( - realERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort(), - 'Checking taker ingress ERC721 account holdings', - ).to.be.deep.equal(expectedERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort()); - } else { - throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdLeft}`); - } - if (makerAssetProxyIdRight === AssetProxyId.ERC20) { - expect( - realERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight], - 'Checking left maker ingress ERC20 account balance', - ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight]); - expect( - realERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight], - 'Checking right maker egress ERC20 account balance', - ).to.be.bignumber.equal( - expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight], - ); - } else if (makerAssetProxyIdRight === AssetProxyId.ERC721) { - expect( - realERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort(), - 'Checking left maker ingress ERC721 account holdings', - ).to.be.deep.equal( - expectedERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort(), - ); - expect( - realERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight], - 'Checking right maker agress ERC721 account holdings', - ).to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]); - } else { - throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdRight}`); - } - // Paid fees - expect( - realERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress], - 'Checking left maker egress ERC20 account fees', - ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress]); - expect( - realERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress], - 'Checking right maker egress ERC20 account fees', - ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress]); - expect( - realERC20BalancesByOwner[takerAddress][this._feeTokenAddress], - 'Checking taker egress ERC20 account fees', - ).to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][this._feeTokenAddress]); - // Received fees - expect( - realERC20BalancesByOwner[signedOrderLeft.feeRecipientAddress][this._feeTokenAddress], - 'Checking left fee recipient ingress ERC20 account fees', - ).to.be.bignumber.equal( - expectedERC20BalancesByOwner[signedOrderLeft.feeRecipientAddress][this._feeTokenAddress], - ); - expect( - realERC20BalancesByOwner[signedOrderRight.feeRecipientAddress][this._feeTokenAddress], - 'Checking right fee receipient ingress ERC20 account fees', - ).to.be.bignumber.equal( - expectedERC20BalancesByOwner[signedOrderRight.feeRecipientAddress][this._feeTokenAddress], - ); - } /// @dev Calculates the expected balances of order makers, fee recipients, and the taker, /// as a result of matching two orders. /// @param signedOrderRight First matched order. @@ -479,7 +355,7 @@ export class MatchOrderTester { /// @param takerAddress Address of taker (the address who matched the two orders) /// @param erc20BalancesByOwner Current ERC20 balances. /// @param erc721TokenIdsByOwner Current ERC721 token owners. - /// @param expectedTransferAmounts A struct containing the expected transfer amounts. + /// @param expectedTransferAmounts Expected amounts transferred as a result of order matching. /// @return Expected ERC20 balances & ERC721 token owners after orders have been matched. private _calculateExpectedBalances( signedOrderLeft: SignedOrder, @@ -589,4 +465,119 @@ export class MatchOrderTester { return [expectedNewERC20BalancesByOwner, expectedNewERC721TokenIdsByOwner]; } + /// @dev + /// @param signedOrderLeft First matched order. + /// @param signedOrderRight Second matched order. + /// @param expectedERC20BalancesByOwner Expected ERC20 balances. + /// @param realERC20BalancesByOwner Real ERC20 balances. + /// @param expectedERC721TokenIdsByOwner Expected ERC721 token owners. + /// @param realERC721TokenIdsByOwner Real ERC20 token owners. + /// @param takerAddress Address of taker (account that called Exchange.matchOrders). + private async _verifyMakerTakerAndFeeRecipientBalancesAsync( + signedOrderLeft: SignedOrder, + signedOrderRight: SignedOrder, + expectedERC20BalancesByOwner: ERC20BalancesByOwner, + realERC20BalancesByOwner: ERC20BalancesByOwner, + expectedERC721TokenIdsByOwner: ERC721TokenIdsByOwner, + realERC721TokenIdsByOwner: ERC721TokenIdsByOwner, + takerAddress: string, + ): Promise { + // Individual balance comparisons + const makerAssetProxyIdLeft = assetDataUtils.decodeAssetProxyId(signedOrderLeft.makerAssetData); + const makerERC20AssetDataLeft = + makerAssetProxyIdLeft === AssetProxyId.ERC20 + ? assetDataUtils.decodeERC20AssetData(signedOrderLeft.makerAssetData) + : assetDataUtils.decodeERC721AssetData(signedOrderLeft.makerAssetData); + const makerAssetAddressLeft = makerERC20AssetDataLeft.tokenAddress; + const makerAssetProxyIdRight = assetDataUtils.decodeAssetProxyId(signedOrderRight.makerAssetData); + const makerERC20AssetDataRight = + makerAssetProxyIdRight === AssetProxyId.ERC20 + ? assetDataUtils.decodeERC20AssetData(signedOrderRight.makerAssetData) + : assetDataUtils.decodeERC721AssetData(signedOrderRight.makerAssetData); + const makerAssetAddressRight = makerERC20AssetDataRight.tokenAddress; + if (makerAssetProxyIdLeft === AssetProxyId.ERC20) { + expect( + realERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft], + 'Checking left maker egress ERC20 account balance', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft]); + expect( + realERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft], + 'Checking right maker ingress ERC20 account balance', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft]); + expect( + realERC20BalancesByOwner[takerAddress][makerAssetAddressLeft], + 'Checking taker ingress ERC20 account balance', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][makerAssetAddressLeft]); + } else if (makerAssetProxyIdLeft === AssetProxyId.ERC721) { + expect( + realERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort(), + 'Checking left maker egress ERC721 account holdings', + ).to.be.deep.equal( + expectedERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressLeft].sort(), + ); + expect( + realERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort(), + 'Checking right maker ERC721 account holdings', + ).to.be.deep.equal( + expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressLeft].sort(), + ); + expect( + realERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort(), + 'Checking taker ingress ERC721 account holdings', + ).to.be.deep.equal(expectedERC721TokenIdsByOwner[takerAddress][makerAssetAddressLeft].sort()); + } else { + throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdLeft}`); + } + if (makerAssetProxyIdRight === AssetProxyId.ERC20) { + expect( + realERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight], + 'Checking left maker ingress ERC20 account balance', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight]); + expect( + realERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight], + 'Checking right maker egress ERC20 account balance', + ).to.be.bignumber.equal( + expectedERC20BalancesByOwner[signedOrderRight.makerAddress][makerAssetAddressRight], + ); + } else if (makerAssetProxyIdRight === AssetProxyId.ERC721) { + expect( + realERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort(), + 'Checking left maker ingress ERC721 account holdings', + ).to.be.deep.equal( + expectedERC721TokenIdsByOwner[signedOrderLeft.makerAddress][makerAssetAddressRight].sort(), + ); + expect( + realERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight], + 'Checking right maker agress ERC721 account holdings', + ).to.be.deep.equal(expectedERC721TokenIdsByOwner[signedOrderRight.makerAddress][makerAssetAddressRight]); + } else { + throw new Error(`Unhandled Asset Proxy ID: ${makerAssetProxyIdRight}`); + } + // Paid fees + expect( + realERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress], + 'Checking left maker egress ERC20 account fees', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderLeft.makerAddress][this._feeTokenAddress]); + expect( + realERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress], + 'Checking right maker egress ERC20 account fees', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[signedOrderRight.makerAddress][this._feeTokenAddress]); + expect( + realERC20BalancesByOwner[takerAddress][this._feeTokenAddress], + 'Checking taker egress ERC20 account fees', + ).to.be.bignumber.equal(expectedERC20BalancesByOwner[takerAddress][this._feeTokenAddress]); + // Received fees + expect( + realERC20BalancesByOwner[signedOrderLeft.feeRecipientAddress][this._feeTokenAddress], + 'Checking left fee recipient ingress ERC20 account fees', + ).to.be.bignumber.equal( + expectedERC20BalancesByOwner[signedOrderLeft.feeRecipientAddress][this._feeTokenAddress], + ); + expect( + realERC20BalancesByOwner[signedOrderRight.feeRecipientAddress][this._feeTokenAddress], + 'Checking right fee receipient ingress ERC20 account fees', + ).to.be.bignumber.equal( + expectedERC20BalancesByOwner[signedOrderRight.feeRecipientAddress][this._feeTokenAddress], + ); + } } // tslint:disable-line:max-file-line-count diff --git a/packages/contracts/test/utils/types.ts b/packages/contracts/test/utils/types.ts index 172622777..598fb5393 100644 --- a/packages/contracts/test/utils/types.ts +++ b/packages/contracts/test/utils/types.ts @@ -117,21 +117,24 @@ export interface TransferAmountsByMatchOrders { // Left Maker amountBoughtByLeftMaker: BigNumber; amountSoldByLeftMaker: BigNumber; - // amountReceivedByLeftMaker: BigNumber; feePaidByLeftMaker: BigNumber; // Right Maker amountBoughtByRightMaker: BigNumber; amountSoldByRightMaker: BigNumber; - // amountReceivedByRightMaker: BigNumber; feePaidByRightMaker: BigNumber; // Taker amountReceivedByTaker: BigNumber; feePaidByTakerLeft: BigNumber; feePaidByTakerRight: BigNumber; - // totalFeePaidByTaker: BigNumber; - // Fee Recipients - // feeReceivedLeft: BigNumber; - // feeReceivedRight: BigNumber; +} + +export interface TransferAmountsLoggedByMatchOrders { + makerAddress: string; + takerAddress: string; + makerAssetFilledAmount: string; + takerAssetFilledAmount: string; + makerFeePaid: string; + takerFeePaid: string; } export interface OrderInfo { -- cgit v1.2.3 From 9fcb2dda73ed7fbcf772364029b987fb147d7387 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 11:39:59 -0700 Subject: Fixed a function comment --- packages/contracts/test/utils/match_order_tester.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index 7d763c6df..ee298097d 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -465,7 +465,8 @@ export class MatchOrderTester { return [expectedNewERC20BalancesByOwner, expectedNewERC721TokenIdsByOwner]; } - /// @dev + /// @dev Verifies ERC20 account balances and ERC721 token holdings that result from order matching. + /// Specifically checks balances of makers, taker and fee recipients. /// @param signedOrderLeft First matched order. /// @param signedOrderRight Second matched order. /// @param expectedERC20BalancesByOwner Expected ERC20 balances. -- cgit v1.2.3 From 1e6b83719a6de17f4501b13afe3e8bef82087def Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 11:45:20 -0700 Subject: Renaming verify -> assert in order matching --- packages/contracts/test/exchange/match_orders.ts | 36 +++++------ .../contracts/test/utils/match_order_tester.ts | 69 +++++++++++----------- 2 files changed, 52 insertions(+), 53 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 58b8250ab..95d4b7502 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -191,7 +191,7 @@ describe.only('matchOrders', () => { feeRecipientAddress: feeRecipientAddressRight, }); // Match signedOrderLeft with signedOrderRight - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -239,7 +239,7 @@ describe.only('matchOrders', () => { feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 16), // 50% }; // Match signedOrderLeft with signedOrderRight - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -303,7 +303,7 @@ describe.only('matchOrders', () => { feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -339,7 +339,7 @@ describe.only('matchOrders', () => { feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; // Match signedOrderLeft with signedOrderRight - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -375,7 +375,7 @@ describe.only('matchOrders', () => { feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 16), // 50% }; // Match signedOrderLeft with signedOrderRight - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -411,7 +411,7 @@ describe.only('matchOrders', () => { feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; // Match signedOrderLeft with signedOrderRight - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -453,7 +453,7 @@ describe.only('matchOrders', () => { newERC20BalancesByOwner, // tslint:disable-next-line:trailing-comma newERC721TokenIdsByOwner - ] = await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + ] = await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -486,7 +486,7 @@ describe.only('matchOrders', () => { feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 16), // 90% (10% paid earlier) feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 16), // 90% }; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight2, takerAddress, @@ -531,7 +531,7 @@ describe.only('matchOrders', () => { newERC20BalancesByOwner, // tslint:disable-next-line:trailing-comma newERC721TokenIdsByOwner - ] = await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + ] = await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -568,7 +568,7 @@ describe.only('matchOrders', () => { feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(96), 16), // 96% feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(96), 16), // 96% }; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft2, signedOrderRight, takerAddress, @@ -607,7 +607,7 @@ describe.only('matchOrders', () => { feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -643,7 +643,7 @@ describe.only('matchOrders', () => { feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -679,7 +679,7 @@ describe.only('matchOrders', () => { feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -715,7 +715,7 @@ describe.only('matchOrders', () => { feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -751,7 +751,7 @@ describe.only('matchOrders', () => { feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -786,7 +786,7 @@ describe.only('matchOrders', () => { feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -919,7 +919,7 @@ describe.only('matchOrders', () => { feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 50% }; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, @@ -957,7 +957,7 @@ describe.only('matchOrders', () => { feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; - await matchOrderTester.matchOrdersAndVerifyBalancesAsync( + await matchOrderTester.matchOrdersAndAssertEffectsAsync( signedOrderLeft, signedOrderRight, takerAddress, diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index ee298097d..7f2110e0f 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -35,7 +35,7 @@ export class MatchOrderTester { /// @param transactionReceipt Transaction receipt and logs produced by Exchange.matchOrders. /// @param takerAddress Address of taker (account that called Exchange.matchOrders) /// @param expectedTransferAmounts Expected amounts transferred as a result of order matching. - private static async _verifyLogsAsync( + private static async _assertLogsAsync( signedOrderLeft: SignedOrder, signedOrderRight: SignedOrder, transactionReceipt: TransactionReceiptWithDecodedLogs, @@ -66,7 +66,7 @@ export class MatchOrderTester { const feePaidByTakerRight = new BigNumber(rightLog.takerFeePaid); // Derive amount received by taker const amountReceivedByTaker = amountSoldByLeftMaker.sub(amountBoughtByRightMaker); - // Verify log values - left order + // Assert log values - left order expect( expectedTransferAmounts.amountBoughtByLeftMaker, 'Checking logged amount bought by left maker', @@ -83,7 +83,7 @@ export class MatchOrderTester { expectedTransferAmounts.feePaidByTakerLeft, 'Checking logged fee paid on left order by taker', ).to.be.bignumber.equal(feePaidByTakerLeft); - // Verify log values - right order + // Assert log values - right order expect( expectedTransferAmounts.amountBoughtByRightMaker, 'Checking logged amount bought by right maker', @@ -100,18 +100,18 @@ export class MatchOrderTester { expectedTransferAmounts.feePaidByTakerRight, 'Checking logged fee paid on right order by taker', ).to.be.bignumber.equal(feePaidByTakerRight); - // Verify derived amount received by taker + // Assert derived amount received by taker expect( expectedTransferAmounts.amountReceivedByTaker, 'Checking logged amount received by taker', ).to.be.bignumber.equal(amountReceivedByTaker); } - /// @dev Verifies all expected ERC20 and ERC721 account holdings match the real holdings. + /// @dev Asserts all expected ERC20 and ERC721 account holdings match the real holdings. /// @param expectedERC20BalancesByOwner Expected ERC20 balances. /// @param realERC20BalancesByOwner Real ERC20 balances. /// @param expectedERC721TokenIdsByOwner Expected ERC721 token owners. /// @param realERC721TokenIdsByOwner Real ERC20 token owners. - private static async _verifyAllKnownBalancesAsync( + private static async _assertAllKnownBalancesAsync( expectedERC20BalancesByOwner: ERC20BalancesByOwner, realERC20BalancesByOwner: ERC20BalancesByOwner, expectedERC721TokenIdsByOwner: ERC721TokenIdsByOwner, @@ -153,8 +153,7 @@ export class MatchOrderTester { this._erc721Wrapper = erc721Wrapper; this._feeTokenAddress = feeTokenAddress; } - /// @dev Matches two complementary orders and verifies results. - /// Validation either succeeds or throws. + /// @dev Matches two complementary orders and asserts results. /// @param signedOrderLeft First matched order. /// @param signedOrderRight Second matched order. /// @param takerAddress Address of taker (the address who matched the two orders) @@ -164,7 +163,7 @@ export class MatchOrderTester { /// @param initialLeftOrderFilledAmount How much left order has been filled, prior to matching orders. /// @param initialRightOrderFilledAmount How much the right order has been filled, prior to matching orders. /// @return New ERC20 balances & ERC721 token owners. - public async matchOrdersAndVerifyBalancesAsync( + public async matchOrdersAndAssertEffectsAsync( signedOrderLeft: SignedOrder, signedOrderRight: SignedOrder, takerAddress: string, @@ -181,8 +180,8 @@ export class MatchOrderTester { if (initialRightOrderFilledAmount === undefined) { initialRightOrderFilledAmount = new BigNumber(0); } - // Verify initial order states - await this._verifyInitialOrderStatesAsync( + // Assert initial order states + await this._assertInitialOrderStatesAsync( signedOrderLeft, signedOrderRight, initialLeftOrderFilledAmount, @@ -196,24 +195,24 @@ export class MatchOrderTester { ); const newERC20BalancesByOwner = await this._erc20Wrapper.getBalancesAsync(); const newERC721TokenIdsByOwner = await this._erc721Wrapper.getBalancesAsync(); - // Verify logs - await MatchOrderTester._verifyLogsAsync( + // Assert logs + await MatchOrderTester._assertLogsAsync( signedOrderLeft, signedOrderRight, transactionReceipt, takerAddress, expectedTransferAmounts, ); - // Verify exchange state - await this._verifyExchangeStateAsync( + // Assert exchange state + await this._assertExchangeStateAsync( signedOrderLeft, signedOrderRight, initialLeftOrderFilledAmount, initialRightOrderFilledAmount, expectedTransferAmounts, ); - // Verify balances of makers, taker, and fee recipients - await this._verifyBalancesAsync( + // Assert balances of makers, taker, and fee recipients + await this._assertBalancesAsync( signedOrderLeft, signedOrderRight, erc20BalancesByOwner, @@ -225,25 +224,25 @@ export class MatchOrderTester { ); return [newERC20BalancesByOwner, newERC721TokenIdsByOwner]; } - /// @dev Verifies initial exchange state for the left and right orders. + /// @dev Asserts initial exchange state for the left and right orders. /// @param signedOrderLeft First matched order. /// @param signedOrderRight Second matched order. /// @param expectedOrderFilledAmountLeft How much left order has been filled, prior to matching orders. /// @param expectedOrderFilledAmountRight How much the right order has been filled, prior to matching orders. - private async _verifyInitialOrderStatesAsync( + private async _assertInitialOrderStatesAsync( signedOrderLeft: SignedOrder, signedOrderRight: SignedOrder, expectedOrderFilledAmountLeft: BigNumber, expectedOrderFilledAmountRight: BigNumber, ): Promise { - // Verify left order initial state + // Assert left order initial state const orderTakerAssetFilledAmountLeft = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderLeft), ); expect(expectedOrderFilledAmountLeft, 'Checking inital state of left order').to.be.bignumber.equal( orderTakerAssetFilledAmountLeft, ); - // Verify right order initial state + // Assert right order initial state const orderTakerAssetFilledAmountRight = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderRight), ); @@ -251,20 +250,20 @@ export class MatchOrderTester { orderTakerAssetFilledAmountRight, ); } - /// @dev Verifies the exchange state against the expected amounts transferred by from matching orders. + /// @dev Asserts the exchange state against the expected amounts transferred by from matching orders. /// @param signedOrderLeft First matched order. /// @param signedOrderRight Second matched order. /// @param initialLeftOrderFilledAmount How much left order has been filled, prior to matching orders. /// @param initialRightOrderFilledAmount How much the right order has been filled, prior to matching orders. /// @return TransferAmounts A struct containing the expected transfer amounts. - private async _verifyExchangeStateAsync( + private async _assertExchangeStateAsync( signedOrderLeft: SignedOrder, signedOrderRight: SignedOrder, initialLeftOrderFilledAmount: BigNumber, initialRightOrderFilledAmount: BigNumber, expectedTransferAmounts: TransferAmounts, ): Promise { - // Verify state for left order: amount bought by left maker + // Assert state for left order: amount bought by left maker let amountBoughtByLeftMaker = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderLeft), ); @@ -273,7 +272,7 @@ export class MatchOrderTester { expectedTransferAmounts.amountBoughtByLeftMaker, 'Checking exchange state for left order', ).to.be.bignumber.equal(amountBoughtByLeftMaker); - // Verify state for right order: amount bought by right maker + // Assert state for right order: amount bought by right maker let amountBoughtByRightMaker = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderRight), ); @@ -282,7 +281,7 @@ export class MatchOrderTester { expectedTransferAmounts.amountBoughtByRightMaker, 'Checking exchange state for right order', ).to.be.bignumber.equal(amountBoughtByRightMaker); - // Verify left order status + // Assert left order status const maxAmountBoughtByLeftMaker = signedOrderLeft.takerAssetAmount.minus(initialLeftOrderFilledAmount); const leftOrderInfo: OrderInfo = await this._exchangeWrapper.getOrderInfoAsync(signedOrderLeft); const leftExpectedStatus = expectedTransferAmounts.amountBoughtByLeftMaker.equals(maxAmountBoughtByLeftMaker) @@ -291,7 +290,7 @@ export class MatchOrderTester { expect(leftOrderInfo.orderStatus as OrderStatus, 'Checking exchange status for left order').to.be.equal( leftExpectedStatus, ); - // Verify right order status + // Assert right order status const maxAmountBoughtByRightMaker = signedOrderRight.takerAssetAmount.minus(initialRightOrderFilledAmount); const rightOrderInfo: OrderInfo = await this._exchangeWrapper.getOrderInfoAsync(signedOrderRight); const rightExpectedStatus = expectedTransferAmounts.amountBoughtByRightMaker.equals(maxAmountBoughtByRightMaker) @@ -301,7 +300,7 @@ export class MatchOrderTester { rightExpectedStatus, ); } - /// @dev Verifies account balances after matching orders. + /// @dev Asserts account balances after matching orders. /// @param signedOrderLeft First matched order. /// @param signedOrderRight Second matched order. /// @param initialERC20BalancesByOwner ERC20 balances prior to order matching. @@ -310,7 +309,7 @@ export class MatchOrderTester { /// @param finalERC721TokenIdsByOwner ERC721 token owners after order matching. /// @param expectedTransferAmounts Expected amounts transferred as a result of order matching. /// @param takerAddress Address of taker (account that called Exchange.matchOrders). - private async _verifyBalancesAsync( + private async _assertBalancesAsync( signedOrderLeft: SignedOrder, signedOrderRight: SignedOrder, initialERC20BalancesByOwner: ERC20BalancesByOwner, @@ -330,8 +329,8 @@ export class MatchOrderTester { initialERC721TokenIdsByOwner, expectedTransferAmounts, ); - // Verify balances of makers, taker, and fee recipients - await this._verifyMakerTakerAndFeeRecipientBalancesAsync( + // Assert balances of makers, taker, and fee recipients + await this._assertMakerTakerAndFeeRecipientBalancesAsync( signedOrderLeft, signedOrderRight, expectedERC20BalancesByOwner, @@ -340,8 +339,8 @@ export class MatchOrderTester { finalERC721TokenIdsByOwner, takerAddress, ); - // Verify balances for all known accounts - await MatchOrderTester._verifyAllKnownBalancesAsync( + // Assert balances for all known accounts + await MatchOrderTester._assertAllKnownBalancesAsync( expectedERC20BalancesByOwner, finalERC20BalancesByOwner, expectedERC721TokenIdsByOwner, @@ -465,7 +464,7 @@ export class MatchOrderTester { return [expectedNewERC20BalancesByOwner, expectedNewERC721TokenIdsByOwner]; } - /// @dev Verifies ERC20 account balances and ERC721 token holdings that result from order matching. + /// @dev Asserts ERC20 account balances and ERC721 token holdings that result from order matching. /// Specifically checks balances of makers, taker and fee recipients. /// @param signedOrderLeft First matched order. /// @param signedOrderRight Second matched order. @@ -474,7 +473,7 @@ export class MatchOrderTester { /// @param expectedERC721TokenIdsByOwner Expected ERC721 token owners. /// @param realERC721TokenIdsByOwner Real ERC20 token owners. /// @param takerAddress Address of taker (account that called Exchange.matchOrders). - private async _verifyMakerTakerAndFeeRecipientBalancesAsync( + private async _assertMakerTakerAndFeeRecipientBalancesAsync( signedOrderLeft: SignedOrder, signedOrderRight: SignedOrder, expectedERC20BalancesByOwner: ERC20BalancesByOwner, -- cgit v1.2.3 From ba15fb6a06b3c924ec3662a706c99f143fcef0f9 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 13:27:59 -0700 Subject: Swapped direction of `expect` values to match output in failure cases --- .../contracts/test/utils/match_order_tester.ts | 63 +++++++++------------- 1 file changed, 26 insertions(+), 37 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index 7f2110e0f..e6347a962 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -67,44 +67,35 @@ export class MatchOrderTester { // Derive amount received by taker const amountReceivedByTaker = amountSoldByLeftMaker.sub(amountBoughtByRightMaker); // Assert log values - left order - expect( + expect(amountBoughtByLeftMaker, 'Checking logged amount bought by left maker').to.be.bignumber.equal( expectedTransferAmounts.amountBoughtByLeftMaker, - 'Checking logged amount bought by left maker', - ).to.be.bignumber.equal(amountBoughtByLeftMaker); - expect( + ); + expect(amountSoldByLeftMaker, 'Checking logged amount sold by left maker').to.be.bignumber.equal( expectedTransferAmounts.amountSoldByLeftMaker, - 'Checking logged amount sold by left maker', - ).to.be.bignumber.equal(amountSoldByLeftMaker); - expect( + ); + expect(feePaidByLeftMaker, 'Checking logged fee paid by left maker').to.be.bignumber.equal( expectedTransferAmounts.feePaidByLeftMaker, - 'Checking logged fee paid by left maker', - ).to.be.bignumber.equal(feePaidByLeftMaker); - expect( + ); + expect(feePaidByTakerLeft, 'Checking logged fee paid on left order by taker').to.be.bignumber.equal( expectedTransferAmounts.feePaidByTakerLeft, - 'Checking logged fee paid on left order by taker', - ).to.be.bignumber.equal(feePaidByTakerLeft); + ); // Assert log values - right order - expect( + expect(amountBoughtByRightMaker, 'Checking logged amount bought by right maker').to.be.bignumber.equal( expectedTransferAmounts.amountBoughtByRightMaker, - 'Checking logged amount bought by right maker', - ).to.be.bignumber.equal(amountBoughtByRightMaker); - expect( + ); + expect(amountSoldByRightMaker, 'Checking logged amount sold by right maker').to.be.bignumber.equal( expectedTransferAmounts.amountSoldByRightMaker, - 'Checking logged amount sold by right maker', - ).to.be.bignumber.equal(amountSoldByRightMaker); - expect( + ); + expect(feePaidByRightMaker, 'Checking logged fee paid by right maker').to.be.bignumber.equal( expectedTransferAmounts.feePaidByRightMaker, - 'Checking logged fee paid by right maker', - ).to.be.bignumber.equal(feePaidByRightMaker); - expect( + ); + expect(feePaidByTakerRight, 'Checking logged fee paid on right order by taker').to.be.bignumber.equal( expectedTransferAmounts.feePaidByTakerRight, - 'Checking logged fee paid on right order by taker', - ).to.be.bignumber.equal(feePaidByTakerRight); + ); // Assert derived amount received by taker - expect( + expect(amountReceivedByTaker, 'Checking logged amount received by taker').to.be.bignumber.equal( expectedTransferAmounts.amountReceivedByTaker, - 'Checking logged amount received by taker', - ).to.be.bignumber.equal(amountReceivedByTaker); + ); } /// @dev Asserts all expected ERC20 and ERC721 account holdings match the real holdings. /// @param expectedERC20BalancesByOwner Expected ERC20 balances. @@ -239,15 +230,15 @@ export class MatchOrderTester { const orderTakerAssetFilledAmountLeft = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderLeft), ); - expect(expectedOrderFilledAmountLeft, 'Checking inital state of left order').to.be.bignumber.equal( - orderTakerAssetFilledAmountLeft, + expect(orderTakerAssetFilledAmountLeft, 'Checking inital state of left order').to.be.bignumber.equal( + expectedOrderFilledAmountLeft, ); // Assert right order initial state const orderTakerAssetFilledAmountRight = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderRight), ); - expect(expectedOrderFilledAmountRight, 'Checking inital state of right order').to.be.bignumber.equal( - orderTakerAssetFilledAmountRight, + expect(orderTakerAssetFilledAmountRight, 'Checking inital state of right order').to.be.bignumber.equal( + expectedOrderFilledAmountRight, ); } /// @dev Asserts the exchange state against the expected amounts transferred by from matching orders. @@ -268,19 +259,17 @@ export class MatchOrderTester { orderHashUtils.getOrderHashHex(signedOrderLeft), ); amountBoughtByLeftMaker = amountBoughtByLeftMaker.minus(initialLeftOrderFilledAmount); - expect( + expect(amountBoughtByLeftMaker, 'Checking exchange state for left order').to.be.bignumber.equal( expectedTransferAmounts.amountBoughtByLeftMaker, - 'Checking exchange state for left order', - ).to.be.bignumber.equal(amountBoughtByLeftMaker); + ); // Assert state for right order: amount bought by right maker let amountBoughtByRightMaker = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderRight), ); amountBoughtByRightMaker = amountBoughtByRightMaker.minus(initialRightOrderFilledAmount); - expect( + expect(amountBoughtByRightMaker, 'Checking exchange state for right order').to.be.bignumber.equal( expectedTransferAmounts.amountBoughtByRightMaker, - 'Checking exchange state for right order', - ).to.be.bignumber.equal(amountBoughtByRightMaker); + ); // Assert left order status const maxAmountBoughtByLeftMaker = signedOrderLeft.takerAssetAmount.minus(initialLeftOrderFilledAmount); const leftOrderInfo: OrderInfo = await this._exchangeWrapper.getOrderInfoAsync(signedOrderLeft); -- cgit v1.2.3 From f8e565bc06b48f4e59b9a246fdf1e4b16245bd83 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 16:07:13 -0700 Subject: Tests for matchOrders edge cases --- packages/contracts/test/exchange/match_orders.ts | 84 ++++++++++++++++++++---- 1 file changed, 71 insertions(+), 13 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 95d4b7502..95a6fff33 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -173,8 +173,7 @@ describe.only('matchOrders', () => { erc721TokenIdsByOwner = await erc721Wrapper.getBalancesAsync(); }); - /* - it.only('should transfer the correct amounts when orders completely fill each other', async () => { + it('Should give right maker a better price when correct price is not integral', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ makerAddress: makerAddressLeft, @@ -190,6 +189,67 @@ describe.only('matchOrders', () => { takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(3000), 0), feeRecipientAddress: feeRecipientAddressRight, }); + // Note: + // The maker/taker fee percentage paid on the right order differs because + // they received different sale prices. + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2000), 0), + amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1001), 0), + feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + // Right Maker + amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1001), 0), + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(300), 0), // @TODO: Update once rounding up is implemented + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10.01), 16), // @TODO: Update once rounding up is implemented + // Taker + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1700), 0), + feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 16), // @TODO: Update once rounding up is implemented + }; + // Match signedOrderLeft with signedOrderRight + await matchOrderTester.matchOrdersAndAssertEffectsAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, + erc20BalancesByOwner, + erc721TokenIdsByOwner, + expectedTransferAmounts, + ); + }); + + it('Should give left maker a better price when correct price is non-integral', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAddress: makerAddressLeft, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(12), 0), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(97), 0), + feeRecipientAddress: feeRecipientAddressLeft, + }); + 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. + 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, @@ -197,17 +257,11 @@ describe.only('matchOrders', () => { 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('Jacobs Example', async () => { + 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, @@ -230,13 +284,17 @@ describe.only('matchOrders', () => { amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Right Maker + // Note: + // For order [4,2] valid fill amounts through `Exchange.fillOrder` would be [2, 1] or [4, 2] + // In this case we have fill amounts of [3, 1] which is attainable through + // `Exchange.matchOrders` but not `Exchange.fillOrder` amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), - amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), - feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 16), // 75% + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), // @TODO: Update once rounding up is implemented + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 16), // 75% (@TODO: Update once rounding up is implemented) // Taker amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(9), 0), feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 16), // 50% + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 16), // 50% (@TODO: Update once rounding up is implemented) }; // Match signedOrderLeft with signedOrderRight await matchOrderTester.matchOrdersAndAssertEffectsAsync( -- cgit v1.2.3 From 81dc893d1d07e5a8485fb40aa247428cda79d788 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 16:07:39 -0700 Subject: Removed a redundant comment from matchOrders --- packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index b3f376eb8..d932c743f 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -165,11 +165,6 @@ contract MixinMatchOrders is pure returns (LibFillResults.MatchedFillResults memory matchedFillResults) { - // We settle orders at the exchange rate of the right order. - // The amount saved by the left maker goes to the taker. - // Either the left or right order will be fully filled; possibly both. - // The left order is fully filled iff the right order can sell more than left can buy. - // Derive maker asset amounts for left & right orders, given store taker assert amounts uint256 leftTakerAssetAmountRemaining = safeSub(leftOrder.takerAssetAmount, leftOrderTakerAssetFilledAmount); uint256 leftMakerAssetAmountRemaining = getPartialAmountFloor( @@ -184,6 +179,7 @@ contract MixinMatchOrders is rightTakerAssetAmountRemaining ); + // Calculate fill results for maker and taker assets if (leftTakerAssetAmountRemaining >= rightMakerAssetAmountRemaining) { // Case 1: Right order is fully filled: maximally fill right matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining; -- cgit v1.2.3 From 6833e243b7b3aa6991f286e07666c6229313a046 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 16:14:51 -0700 Subject: Addressed linter errors in match order tesster --- packages/contracts/test/utils/match_order_tester.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index e6347a962..4d27fc630 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -161,16 +161,9 @@ export class MatchOrderTester { erc20BalancesByOwner: ERC20BalancesByOwner, erc721TokenIdsByOwner: ERC721TokenIdsByOwner, expectedTransferAmounts: TransferAmounts, - initialLeftOrderFilledAmount?: BigNumber, - initialRightOrderFilledAmount?: BigNumber, + initialLeftOrderFilledAmount: BigNumber = new BigNumber(0), + initialRightOrderFilledAmount: BigNumber = new BigNumber(0), ): Promise<[ERC20BalancesByOwner, ERC721TokenIdsByOwner]> { - // Assign default values to optional parameters if undefined - if (initialLeftOrderFilledAmount === undefined) { - initialLeftOrderFilledAmount = new BigNumber(0); - } - if (initialRightOrderFilledAmount === undefined) { - initialRightOrderFilledAmount = new BigNumber(0); - } // Assert initial order states await this._assertInitialOrderStatesAsync( signedOrderLeft, -- cgit v1.2.3 From a93f95c55ea43bc89d0633190590865d9723b2f9 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 16:16:01 -0700 Subject: Wording in MixinMatchOrders --- packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index d932c743f..a0696c616 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -181,7 +181,7 @@ contract MixinMatchOrders is // Calculate fill results for maker and taker assets if (leftTakerAssetAmountRemaining >= rightMakerAssetAmountRemaining) { - // Case 1: Right order is fully filled: maximally fill right + // Case 1: Right order is fully filled matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining; matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining; matchedFillResults.left.makerAssetFilledAmount = getPartialAmountFloor( @@ -191,7 +191,7 @@ contract MixinMatchOrders is ); matchedFillResults.left.takerAssetFilledAmount = matchedFillResults.right.makerAssetFilledAmount; } else { - // Case 2: Left order is fully filled: maximall fill left + // Case 2: Left order is fully filled matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining; matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining; matchedFillResults.right.makerAssetFilledAmount = matchedFillResults.left.takerAssetFilledAmount; -- cgit v1.2.3 From 0a6f107243eb7df309766cc83942e667ce28e858 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 23 Aug 2018 16:28:03 -0700 Subject: Added temporary @todo to MixinMatchOrders --- .../contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index a0696c616..8f289ee85 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -209,24 +209,24 @@ contract MixinMatchOrders is ); // Compute fees for left order - matchedFillResults.left.makerFeePaid = getPartialAmount( + matchedFillResults.left.makerFeePaid = getPartialAmountFloor( matchedFillResults.left.makerAssetFilledAmount, leftOrder.makerAssetAmount, leftOrder.makerFee ); - matchedFillResults.left.takerFeePaid = getPartialAmount( + matchedFillResults.left.takerFeePaid = getPartialAmountFloor( matchedFillResults.left.takerAssetFilledAmount, leftOrder.takerAssetAmount, leftOrder.takerFee ); // Compute fees for right order - matchedFillResults.right.makerFeePaid = getPartialAmount( + matchedFillResults.right.makerFeePaid = getPartialAmountFloor( matchedFillResults.right.makerAssetFilledAmount, rightOrder.makerAssetAmount, rightOrder.makerFee ); - matchedFillResults.right.takerFeePaid = getPartialAmount( + matchedFillResults.right.takerFeePaid = getPartialAmountFloor( matchedFillResults.right.takerAssetFilledAmount, rightOrder.takerAssetAmount, rightOrder.takerFee -- cgit v1.2.3 From 1c7ba6a31533b3202be7a464452b14aa58a0337b Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 24 Aug 2018 17:17:52 -0700 Subject: Extract only `fill` event logs --- packages/contracts/test/utils/match_order_tester.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index 4d27fc630..e0c55b834 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -42,10 +42,11 @@ export class MatchOrderTester { takerAddress: string, expectedTransferAmounts: TransferAmounts, ): Promise { - // Should have two logs -- one for each order. - expect(transactionReceipt.logs.length, 'Checking number of logs').to.be.equal(2); + // Should have two fill event logs -- one for each order. + const transactionFillLogs = _.filter(transactionReceipt.logs, ['event', 'Fill']); + expect(transactionFillLogs.length, 'Checking number of logs').to.be.equal(2); // First log is for left fill - const leftLog = (transactionReceipt.logs[0] as any).args as LoggedTransferAmounts; + const leftLog = (transactionFillLogs[0] as any).args as LoggedTransferAmounts; expect(leftLog.makerAddress, 'Checking logged maker address of left order').to.be.equal( signedOrderLeft.makerAddress, ); @@ -55,7 +56,7 @@ export class MatchOrderTester { const feePaidByLeftMaker = new BigNumber(leftLog.makerFeePaid); const feePaidByTakerLeft = new BigNumber(leftLog.takerFeePaid); // Second log is for right fill - const rightLog = (transactionReceipt.logs[1] as any).args as LoggedTransferAmounts; + const rightLog = (transactionFillLogs[1] as any).args as LoggedTransferAmounts; expect(rightLog.makerAddress, 'Checking logged maker address of right order').to.be.equal( signedOrderRight.makerAddress, ); -- cgit v1.2.3 From c7a7ae7e18bb6d0f5f148a37b63dccf2e485cc6f Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 24 Aug 2018 17:19:28 -0700 Subject: Give right maker better price when correct value is not integral --- packages/contracts/test/exchange/match_orders.ts | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 95a6fff33..2e6e9410a 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -191,7 +191,8 @@ describe.only('matchOrders', () => { }); // Note: // The maker/taker fee percentage paid on the right order differs because - // they received different sale prices. + // they received different sale prices. Similarly, the right maker pays a + // slightly higher lower than the right taker. const expectedTransferAmounts = { // Left Maker amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2000), 0), @@ -199,12 +200,12 @@ describe.only('matchOrders', () => { feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% // Right Maker amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1001), 0), - amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(300), 0), // @TODO: Update once rounding up is implemented - feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10.01), 16), // @TODO: Update once rounding up is implemented + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(301), 0), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10.01), 16), // 10.01% // Taker - amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1700), 0), + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1699), 0), feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 16), // @TODO: Update once rounding up is implemented + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber('10.0333333333333333'), 16), // 10.03% }; // Match signedOrderLeft with signedOrderRight await matchOrderTester.matchOrdersAndAssertEffectsAsync( @@ -217,7 +218,7 @@ describe.only('matchOrders', () => { ); }); - it('Should give left maker a better price when correct price is non-integral', async () => { + it('Should give left maker a better price when correct price is not integral', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ makerAddress: makerAddressLeft, @@ -288,13 +289,16 @@ describe.only('matchOrders', () => { // For order [4,2] valid fill amounts through `Exchange.fillOrder` would be [2, 1] or [4, 2] // In this case we have fill amounts of [3, 1] which is attainable through // `Exchange.matchOrders` but not `Exchange.fillOrder` + // Note: + // The right maker fee differs from the right taker fee because their exchange rate differs. + // The right maker always receives the better exchange and fee price. amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0), - amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0), // @TODO: Update once rounding up is implemented - feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 16), // 75% (@TODO: Update once rounding up is implemented) + amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 0), + feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 16), // 75% // Taker - amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(9), 0), + amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(8), 0), feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% - feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 16), // 50% (@TODO: Update once rounding up is implemented) + feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100% }; // Match signedOrderLeft with signedOrderRight await matchOrderTester.matchOrdersAndAssertEffectsAsync( -- cgit v1.2.3 From 287830d6e01a919bcb4ac4ccec4173cdbdcf9470 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 24 Aug 2018 17:29:31 -0700 Subject: Run all tests --- packages/contracts/test/exchange/match_orders.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 2e6e9410a..8732e20ba 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -24,7 +24,7 @@ import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -describe.only('matchOrders', () => { +describe('matchOrders', () => { let makerAddressLeft: string; let makerAddressRight: string; let owner: string; -- cgit v1.2.3 From ec2e726be0a653465d3c83e640aa4c1556a0f10f Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 24 Aug 2018 18:09:32 -0700 Subject: Rephrased some of the math in MixinMatchOrders to improve readability --- packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index 8f289ee85..226a1bfb6 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -184,12 +184,12 @@ contract MixinMatchOrders is // Case 1: Right order is fully filled matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining; matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining; + matchedFillResults.left.takerAssetFilledAmount = matchedFillResults.right.makerAssetFilledAmount; matchedFillResults.left.makerAssetFilledAmount = getPartialAmountFloor( leftOrder.makerAssetAmount, leftOrder.takerAssetAmount, - matchedFillResults.right.makerAssetFilledAmount + matchedFillResults.left.takerAssetFilledAmount ); - matchedFillResults.left.takerAssetFilledAmount = matchedFillResults.right.makerAssetFilledAmount; } else { // Case 2: Left order is fully filled matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining; @@ -198,7 +198,7 @@ contract MixinMatchOrders is matchedFillResults.right.takerAssetFilledAmount = getPartialAmountCeil( rightOrder.takerAssetAmount, rightOrder.makerAssetAmount, - matchedFillResults.left.takerAssetFilledAmount + matchedFillResults.right.makerAssetFilledAmount ); } -- cgit v1.2.3 From 878db3b84993d7c9724c0ed8591493a4e2b6cc94 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 24 Aug 2018 18:48:29 -0700 Subject: Added comments to order matching --- .../src/2.0.0/protocol/Exchange/MixinMatchOrders.sol | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index 226a1bfb6..4b8360c23 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -179,12 +179,22 @@ contract MixinMatchOrders is rightTakerAssetAmountRemaining ); - // Calculate fill results for maker and taker assets + // Calculate fill results for maker and taker assets: at least one order will be fully filled. + // The maximum amount the left maker can buy is `leftTakerAssetAmountRemaining` + // The maximum amount the right maker can sell is `rightMakerAssetAmountRemaining` + // We have two distinct cases for calculating the fill results: + // Case 1. + // If the left maker can buy more than the right maker can sell, then only the right order is fully filled. + // If the left maker can buy exactly what the right maker can sell, then both orders are fully filled. + // Case 2. + // If the left maker cannot buy more than the right maker can sell, then only the left order is fully filled. if (leftTakerAssetAmountRemaining >= rightMakerAssetAmountRemaining) { // Case 1: Right order is fully filled matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining; matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining; matchedFillResults.left.takerAssetFilledAmount = matchedFillResults.right.makerAssetFilledAmount; + // Round down to ensure the maker's exchange rate does not exceed the price specified by the order. + // We favor the maker when the exchange rate must be rounded. matchedFillResults.left.makerAssetFilledAmount = getPartialAmountFloor( leftOrder.makerAssetAmount, leftOrder.takerAssetAmount, @@ -195,6 +205,8 @@ contract MixinMatchOrders is matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining; matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining; matchedFillResults.right.makerAssetFilledAmount = matchedFillResults.left.takerAssetFilledAmount; + // Round up to ensure the maker's exchange rate does not exceed the price specified by the order. + // We favor the maker when the exchange rate must be rounded. matchedFillResults.right.takerAssetFilledAmount = getPartialAmountCeil( rightOrder.takerAssetAmount, rightOrder.makerAssetAmount, -- cgit v1.2.3