From 589b791cd7667338c5a0f189f06fccc89809a195 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 26 May 2019 11:35:18 -0700 Subject: [PATCH] Add names to return values and fix breaking transactions tests --- .../contracts/src/MixinWrapperFunctions.sol | 52 +++++++++---------- .../src/interfaces/IWrapperFunctions.sol | 14 ++--- contracts/exchange/test/transactions.ts | 8 +-- 3 files changed, 38 insertions(+), 36 deletions(-) diff --git a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol index dc2d217cc7..dd1ae3bbdd 100644 --- a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol +++ b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol @@ -114,10 +114,10 @@ contract MixinWrapperFunctions is ) public nonReentrant - returns (FillResults[] memory) + returns (FillResults[] memory fillResults) { uint256 ordersLength = orders.length; - FillResults[] memory fillResults = new FillResults[](ordersLength); + fillResults = new FillResults[](ordersLength); for (uint256 i = 0; i != ordersLength; i++) { fillResults[i] = _fillOrder( orders[i], @@ -140,10 +140,10 @@ contract MixinWrapperFunctions is ) public nonReentrant - returns (FillResults[] memory) + returns (FillResults[] memory fillResults) { uint256 ordersLength = orders.length; - FillResults[] memory fillResults = new FillResults[](ordersLength); + fillResults = new FillResults[](ordersLength); for (uint256 i = 0; i != ordersLength; i++) { fillResults[i] = _fillOrKillOrder( orders[i], @@ -166,10 +166,10 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - returns (FillResults[] memory) + returns (FillResults[] memory fillResults) { uint256 ordersLength = orders.length; - FillResults[] memory fillResults = new FillResults[](ordersLength); + fillResults = new FillResults[](ordersLength); for (uint256 i = 0; i != ordersLength; i++) { fillResults[i] = fillOrderNoThrow( orders[i], @@ -192,7 +192,7 @@ contract MixinWrapperFunctions is ) public nonReentrant - returns (FillResults memory totalFillResults) + returns (FillResults memory fillResults) { bytes memory takerAssetData = orders[0].takerAssetData; @@ -205,7 +205,7 @@ contract MixinWrapperFunctions is orders[i].takerAssetData = takerAssetData; // Calculate the remaining amount of takerAsset to sell - uint256 remainingTakerAssetFillAmount = _safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); + uint256 remainingTakerAssetFillAmount = _safeSub(takerAssetFillAmount, fillResults.takerAssetFilledAmount); // Attempt to sell the remaining amount of takerAsset FillResults memory singleFillResults = _fillOrder( @@ -215,14 +215,14 @@ contract MixinWrapperFunctions is ); // Update amounts filled and fees paid by maker and taker - _addFillResults(totalFillResults, singleFillResults); + _addFillResults(fillResults, singleFillResults); // Stop execution if the entire amount of takerAsset has been sold - if (totalFillResults.takerAssetFilledAmount >= takerAssetFillAmount) { + if (fillResults.takerAssetFilledAmount >= takerAssetFillAmount) { break; } } - return totalFillResults; + return fillResults; } /// @dev Synchronously executes multiple calls of fillOrder until total amount of takerAsset is sold by taker. @@ -237,7 +237,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - returns (FillResults memory totalFillResults) + returns (FillResults memory fillResults) { bytes memory takerAssetData = orders[0].takerAssetData; @@ -250,7 +250,7 @@ contract MixinWrapperFunctions is orders[i].takerAssetData = takerAssetData; // Calculate the remaining amount of takerAsset to sell - uint256 remainingTakerAssetFillAmount = _safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); + uint256 remainingTakerAssetFillAmount = _safeSub(takerAssetFillAmount, fillResults.takerAssetFilledAmount); // Attempt to sell the remaining amount of takerAsset FillResults memory singleFillResults = fillOrderNoThrow( @@ -260,14 +260,14 @@ contract MixinWrapperFunctions is ); // Update amounts filled and fees paid by maker and taker - _addFillResults(totalFillResults, singleFillResults); + _addFillResults(fillResults, singleFillResults); // Stop execution if the entire amount of takerAsset has been sold - if (totalFillResults.takerAssetFilledAmount >= takerAssetFillAmount) { + if (fillResults.takerAssetFilledAmount >= takerAssetFillAmount) { break; } } - return totalFillResults; + return fillResults; } /// @dev Synchronously executes multiple calls of fillOrder until total amount of makerAsset is bought by taker. @@ -282,7 +282,7 @@ contract MixinWrapperFunctions is ) public nonReentrant - returns (FillResults memory totalFillResults) + returns (FillResults memory fillResults) { bytes memory makerAssetData = orders[0].makerAssetData; @@ -295,7 +295,7 @@ contract MixinWrapperFunctions is orders[i].makerAssetData = makerAssetData; // Calculate the remaining amount of makerAsset to buy - uint256 remainingMakerAssetFillAmount = _safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); + uint256 remainingMakerAssetFillAmount = _safeSub(makerAssetFillAmount, fillResults.makerAssetFilledAmount); // Convert the remaining amount of makerAsset to buy into remaining amount // of takerAsset to sell, assuming entire amount can be sold in the current order @@ -313,14 +313,14 @@ contract MixinWrapperFunctions is ); // Update amounts filled and fees paid by maker and taker - _addFillResults(totalFillResults, singleFillResults); + _addFillResults(fillResults, singleFillResults); // Stop execution if the entire amount of makerAsset has been bought - if (totalFillResults.makerAssetFilledAmount >= makerAssetFillAmount) { + if (fillResults.makerAssetFilledAmount >= makerAssetFillAmount) { break; } } - return totalFillResults; + return fillResults; } /// @dev Synchronously executes multiple fill orders in a single transaction until total amount is bought by taker. @@ -335,7 +335,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - returns (FillResults memory totalFillResults) + returns (FillResults memory fillResults) { bytes memory makerAssetData = orders[0].makerAssetData; @@ -348,7 +348,7 @@ contract MixinWrapperFunctions is orders[i].makerAssetData = makerAssetData; // Calculate the remaining amount of makerAsset to buy - uint256 remainingMakerAssetFillAmount = _safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); + uint256 remainingMakerAssetFillAmount = _safeSub(makerAssetFillAmount, fillResults.makerAssetFilledAmount); // Convert the remaining amount of makerAsset to buy into remaining amount // of takerAsset to sell, assuming entire amount can be sold in the current order @@ -366,14 +366,14 @@ contract MixinWrapperFunctions is ); // Update amounts filled and fees paid by maker and taker - _addFillResults(totalFillResults, singleFillResults); + _addFillResults(fillResults, singleFillResults); // Stop execution if the entire amount of makerAsset has been bought - if (totalFillResults.makerAssetFilledAmount >= makerAssetFillAmount) { + if (fillResults.makerAssetFilledAmount >= makerAssetFillAmount) { break; } } - return totalFillResults; + return fillResults; } /// @dev After calling, the order can not be filled anymore. diff --git a/contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol b/contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol index a3c244f246..e28c45be62 100644 --- a/contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol +++ b/contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol @@ -62,7 +62,7 @@ contract IWrapperFunctions { bytes[] memory signatures ) public - returns (LibFillResults.FillResults[] memory); + returns (LibFillResults.FillResults[] memory fillResults); /// @dev Synchronously executes multiple calls of fillOrKill. /// @param orders Array of order specifications. @@ -75,7 +75,7 @@ contract IWrapperFunctions { bytes[] memory signatures ) public - returns (LibFillResults.FillResults[] memory); + returns (LibFillResults.FillResults[] memory fillResults); /// @dev Fills an order with specified parameters and ECDSA signature. /// Returns false if the transaction would otherwise revert. @@ -89,7 +89,7 @@ contract IWrapperFunctions { bytes[] memory signatures ) public - returns (LibFillResults.FillResults[] memory); + returns (LibFillResults.FillResults[] memory fillResults); /// @dev Synchronously executes multiple calls of fillOrder until total amount of takerAsset is sold by taker. /// @param orders Array of order specifications. @@ -102,7 +102,7 @@ contract IWrapperFunctions { bytes[] memory signatures ) public - returns (LibFillResults.FillResults memory totalFillResults); + returns (LibFillResults.FillResults memory fillResults); /// @dev Synchronously executes multiple calls of fillOrder until total amount of takerAsset is sold by taker. /// Returns false if the transaction would otherwise revert. @@ -116,7 +116,7 @@ contract IWrapperFunctions { bytes[] memory signatures ) public - returns (LibFillResults.FillResults memory totalFillResults); + returns (LibFillResults.FillResults memory fillResults); /// @dev Synchronously executes multiple calls of fillOrder until total amount of makerAsset is bought by taker. /// @param orders Array of order specifications. @@ -129,7 +129,7 @@ contract IWrapperFunctions { bytes[] memory signatures ) public - returns (LibFillResults.FillResults memory totalFillResults); + returns (LibFillResults.FillResults memory fillResults); /// @dev Synchronously executes multiple fill orders in a single transaction until total amount is bought by taker. /// Returns false if the transaction would otherwise revert. @@ -143,7 +143,7 @@ contract IWrapperFunctions { bytes[] memory signatures ) public - returns (LibFillResults.FillResults memory totalFillResults); + returns (LibFillResults.FillResults memory fillResults); /// @dev Synchronously cancels multiple orders in a single transaction. /// @param orders Array of order specifications. diff --git a/contracts/exchange/test/transactions.ts b/contracts/exchange/test/transactions.ts index 63cdca60e7..c250fd8ae7 100644 --- a/contracts/exchange/test/transactions.ts +++ b/contracts/exchange/test/transactions.ts @@ -286,11 +286,13 @@ describe('Exchange transactions', () => { const abi = artifacts.Exchange.compilerOutput.abi; const methodAbi = abi.filter(abiItem => (abiItem as MethodAbi).name === fnName)[0] as MethodAbi; const abiEncoder = new AbiEncoder.Method(methodAbi); + const decodedReturnData = abiEncoder.decodeReturnValues(returnData); - const fillResults: FillResults = - decodedReturnData.fillResults === undefined - ? decodedReturnData.totalFillResults + const fillResults = + exchangeConstants.BATCH_FILL_FN_NAMES.indexOf(fnName) !== -1 + ? decodedReturnData.fillResults[0] : decodedReturnData.fillResults; + expect(fillResults.makerAssetFilledAmount).to.be.bignumber.eq(order.makerAssetAmount); expect(fillResults.takerAssetFilledAmount).to.be.bignumber.eq(order.takerAssetAmount); expect(fillResults.makerFeePaid).to.be.bignumber.eq(order.makerFee);