aboutsummaryrefslogtreecommitdiffstats
path: root/packages/contracts
diff options
context:
space:
mode:
Diffstat (limited to 'packages/contracts')
-rw-r--r--packages/contracts/package.json4
-rw-r--r--packages/contracts/src/2.0.0/forwarder/MixinAssets.sol17
-rw-r--r--packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol16
-rw-r--r--packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol57
-rw-r--r--packages/contracts/src/2.0.0/forwarder/MixinWeth.sol12
-rw-r--r--packages/contracts/src/2.0.0/forwarder/interfaces/IAssets.sol10
-rw-r--r--packages/contracts/src/2.0.0/forwarder/mixins/MAssets.sol2
-rw-r--r--packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol4
-rw-r--r--packages/contracts/src/2.0.0/forwarder/mixins/MForwarderCore.sol42
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol1
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/libs/LibAbiEncoder.sol17
-rw-r--r--packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol1
-rw-r--r--packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol22
-rw-r--r--packages/contracts/test/forwarder/forwarder.ts81
-rw-r--r--packages/contracts/test/utils/exchange_wrapper.ts25
-rw-r--r--packages/contracts/test/utils/fill_order_combinatorial_utils.ts55
-rw-r--r--packages/contracts/test/utils/forwarder_wrapper.ts6
17 files changed, 184 insertions, 188 deletions
diff --git a/packages/contracts/package.json b/packages/contracts/package.json
index 1f5c15674..067a20775 100644
--- a/packages/contracts/package.json
+++ b/packages/contracts/package.json
@@ -62,7 +62,7 @@
"copyfiles": "^1.2.0",
"dirty-chai": "^2.0.1",
"make-promises-safe": "^1.1.0",
- "mocha": "^4.0.1",
+ "mocha": "^4.1.0",
"npm-run-all": "^4.1.2",
"shx": "^0.2.2",
"solc": "^0.4.24",
@@ -86,6 +86,6 @@
"ethereumjs-util": "^5.1.1",
"ethers": "3.0.22",
"js-combinatorics": "^0.5.3",
- "lodash": "^4.17.4"
+ "lodash": "^4.17.5"
}
}
diff --git a/packages/contracts/src/2.0.0/forwarder/MixinAssets.sol b/packages/contracts/src/2.0.0/forwarder/MixinAssets.sol
index 5cf5f831b..e06f9a8e3 100644
--- a/packages/contracts/src/2.0.0/forwarder/MixinAssets.sol
+++ b/packages/contracts/src/2.0.0/forwarder/MixinAssets.sol
@@ -36,28 +36,25 @@ contract MixinAssets is
bytes4 constant internal ERC20_TRANSFER_SELECTOR = bytes4(keccak256("transfer(address,uint256)"));
- /// @dev Withdraws ERC20 tokens from this contract. The contract requires a ZRX balance in order to
+ /// @dev Withdraws assets from this contract. The contract requires a ZRX balance in order to
/// function optimally, and this function allows the ZRX to be withdrawn by owner. It may also be
- /// used to withdraw tokens that were accidentally sent to this contract.
- /// @param token Address of ERC20 token to withdraw.
+ /// used to withdraw assets that were accidentally sent to this contract.
+ /// @param assetData Byte array encoded for the respective asset proxy.
/// @param amount Amount of ERC20 token to withdraw.
- function withdrawERC20(
- address token,
+ function withdrawAsset(
+ bytes assetData,
uint256 amount
)
external
onlyOwner
{
- require(
- IERC20Token(token).transfer(msg.sender, amount),
- "WITHDRAWAL_FAILED"
- );
+ transferAssetToSender(assetData, amount);
}
/// @dev Transfers given amount of asset to sender.
/// @param assetData Byte array encoded for the respective asset proxy.
/// @param amount Amount of asset to transfer to sender.
- function transferPurchasedAssetToSender(
+ function transferAssetToSender(
bytes memory assetData,
uint256 amount
)
diff --git a/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol b/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol
index f3aa483c5..4584bb840 100644
--- a/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol
+++ b/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol
@@ -139,7 +139,7 @@ contract MixinExchangeWrapper is
/// @param makerAssetFillAmount Desired amount of makerAsset to buy.
/// @param signatures Proofs that orders have been signed by makers.
/// @return Amounts filled and fees paid by makers and taker.
- function marketBuyWithWeth(
+ function marketBuyExactAmountWithWeth(
LibOrder.Order[] memory orders,
uint256 makerAssetFillAmount,
bytes[] memory signatures
@@ -180,10 +180,16 @@ contract MixinExchangeWrapper is
addFillResults(totalFillResults, singleFillResults);
// Stop execution if the entire amount of makerAsset has been bought
- if (totalFillResults.makerAssetFilledAmount >= makerAssetFillAmount) {
+ uint256 makerAssetFilledAmount = totalFillResults.makerAssetFilledAmount;
+ if (makerAssetFilledAmount >= makerAssetFillAmount) {
break;
}
}
+
+ require(
+ makerAssetFilledAmount >= makerAssetFillAmount,
+ "COMPLETE_FILL_FAILED"
+ );
return totalFillResults;
}
@@ -196,7 +202,7 @@ contract MixinExchangeWrapper is
/// @param zrxBuyAmount Desired amount of ZRX to buy.
/// @param signatures Proofs that orders have been created by makers.
/// @return totalFillResults Amounts filled and fees paid by maker and taker.
- function marketBuyZrxWithWeth(
+ function marketBuyExactZrxWithWeth(
LibOrder.Order[] memory orders,
uint256 zrxBuyAmount,
bytes[] memory signatures
@@ -248,6 +254,10 @@ contract MixinExchangeWrapper is
}
}
+ require(
+ zrxPurchased >= zrxBuyAmount,
+ "COMPLETE_FILL_FAILED"
+ );
return totalFillResults;
}
}
diff --git a/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol b/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol
index 1164ae919..93cbf79be 100644
--- a/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol
+++ b/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol
@@ -23,7 +23,7 @@ import "./libs/LibConstants.sol";
import "./mixins/MWeth.sol";
import "./mixins/MAssets.sol";
import "./mixins/MExchangeWrapper.sol";
-import "./mixins/MForwarderCore.sol";
+import "./interfaces/IForwarderCore.sol";
import "../utils/LibBytes/LibBytes.sol";
import "../protocol/Exchange/libs/LibOrder.sol";
import "../protocol/Exchange/libs/LibFillResults.sol";
@@ -37,7 +37,7 @@ contract MixinForwarderCore is
MWeth,
MAssets,
MExchangeWrapper,
- MForwarderCore
+ IForwarderCore
{
using LibBytes for bytes;
@@ -117,7 +117,7 @@ contract MixinForwarderCore is
);
// Buy back all ZRX spent on fees.
zrxBuyAmount = orderFillResults.takerFeePaid;
- feeOrderFillResults = marketBuyZrxWithWeth(
+ feeOrderFillResults = marketBuyExactZrxWithWeth(
feeOrders,
zrxBuyAmount,
feeSignatures
@@ -125,13 +125,6 @@ contract MixinForwarderCore is
makerAssetAmountPurchased = orderFillResults.makerAssetFilledAmount;
}
- // Ensure that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold.
- assertValidFillResults(
- orderFillResults,
- feeOrderFillResults,
- zrxBuyAmount
- );
-
// Transfer feePercentage of total ETH spent on primary orders to feeRecipient.
// Refund remaining ETH to msg.sender.
transferEthFeeAndRefund(
@@ -142,7 +135,7 @@ contract MixinForwarderCore is
);
// Transfer purchased assets to msg.sender.
- transferPurchasedAssetToSender(orders[0].makerAssetData, makerAssetAmountPurchased);
+ transferAssetToSender(orders[0].makerAssetData, makerAssetAmountPurchased);
}
/// @dev Attempt to purchase makerAssetFillAmount of makerAsset by selling ETH provided with transaction.
@@ -180,7 +173,7 @@ contract MixinForwarderCore is
if (orders[0].makerAssetData.equals(ZRX_ASSET_DATA)) {
// If the makerAsset is ZRX, it is not necessary to pay fees out of this
// contracts's ZRX balance because fees are factored into the price of the order.
- orderFillResults = marketBuyZrxWithWeth(
+ orderFillResults = marketBuyExactZrxWithWeth(
orders,
makerAssetFillAmount,
signatures
@@ -190,14 +183,14 @@ contract MixinForwarderCore is
} else {
// Attemp to purchase desired amount of makerAsset.
// ZRX fees are payed with this contract's balance.
- orderFillResults = marketBuyWithWeth(
+ orderFillResults = marketBuyExactAmountWithWeth(
orders,
makerAssetFillAmount,
signatures
);
// Buy back all ZRX spent on fees.
zrxBuyAmount = orderFillResults.takerFeePaid;
- feeOrderFillResults = marketBuyZrxWithWeth(
+ feeOrderFillResults = marketBuyExactZrxWithWeth(
feeOrders,
zrxBuyAmount,
feeSignatures
@@ -205,13 +198,6 @@ contract MixinForwarderCore is
makerAssetAmountPurchased = orderFillResults.makerAssetFilledAmount;
}
- // Ensure that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold.
- assertValidFillResults(
- orderFillResults,
- feeOrderFillResults,
- zrxBuyAmount
- );
-
// Transfer feePercentage of total ETH spent on primary orders to feeRecipient.
// Refund remaining ETH to msg.sender.
transferEthFeeAndRefund(
@@ -222,33 +208,6 @@ contract MixinForwarderCore is
);
// Transfer purchased assets to msg.sender.
- transferPurchasedAssetToSender(orders[0].makerAssetData, makerAssetAmountPurchased);
- }
-
- /// @dev Ensures that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold.
- /// @param orderFillResults Amounts filled and fees paid for primary orders.
- /// @param feeOrderFillResults Amounts filled and fees paid for fee orders.
- /// @param zrxBuyAmount The amount of ZRX that needed to be repurchased after filling primary orders.
- function assertValidFillResults(
- FillResults memory orderFillResults,
- FillResults memory feeOrderFillResults,
- uint256 zrxBuyAmount
- )
- internal
- view
- {
- // Ensure that all ZRX spent while filling primary orders has been repurchased.
- uint256 zrxPurchased = safeSub(feeOrderFillResults.makerAssetFilledAmount, feeOrderFillResults.takerFeePaid);
- require(
- zrxPurchased >= zrxBuyAmount,
- "COMPLETE_FILL_FAILED"
- );
-
- // Ensure that no extra WETH owned by this contract has been sold.
- uint256 wethSold = safeAdd(orderFillResults.takerAssetFilledAmount, feeOrderFillResults.takerAssetFilledAmount);
- require(
- wethSold <= msg.value,
- "OVERSOLD_WETH"
- );
+ transferAssetToSender(orders[0].makerAssetData, makerAssetAmountPurchased);
}
}
diff --git a/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol b/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol
index 8ba236e7f..e07940776 100644
--- a/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol
+++ b/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol
@@ -71,12 +71,16 @@ contract MixinWeth is
"FEE_PERCENTAGE_TOO_LARGE"
);
- // Calculate amount of WETH that hasn't been sold.
- uint256 wethRemaining = safeSub(
- msg.value,
- safeAdd(wethSoldExcludingFeeOrders, wethSoldForZrx)
+ // Ensure that no extra WETH owned by this contract has been sold.
+ uint256 wethSold = safeAdd(wethSoldExcludingFeeOrders, wethSoldForZrx);
+ require(
+ wethSold <= msg.value,
+ "OVERSOLD_WETH"
);
+ // Calculate amount of WETH that hasn't been sold.
+ uint256 wethRemaining = safeSub(msg.value, wethSold);
+
// Calculate ETH fee to pay to feeRecipient.
uint256 ethFee = getPartialAmount(
feePercentage,
diff --git a/packages/contracts/src/2.0.0/forwarder/interfaces/IAssets.sol b/packages/contracts/src/2.0.0/forwarder/interfaces/IAssets.sol
index 9b0d995eb..1e034c003 100644
--- a/packages/contracts/src/2.0.0/forwarder/interfaces/IAssets.sol
+++ b/packages/contracts/src/2.0.0/forwarder/interfaces/IAssets.sol
@@ -21,13 +21,13 @@ pragma solidity 0.4.24;
contract IAssets {
- /// @dev Withdraws ERC20 tokens from this contract. The contract requires a ZRX balance in order to
+ /// @dev Withdraws assets from this contract. The contract requires a ZRX balance in order to
/// function optimally, and this function allows the ZRX to be withdrawn by owner. It may also be
- /// used to withdraw tokens that were accidentally sent to this contract.
- /// @param token Address of ERC20 token to withdraw.
+ /// used to withdraw assets that were accidentally sent to this contract.
+ /// @param assetData Byte array encoded for the respective asset proxy.
/// @param amount Amount of ERC20 token to withdraw.
- function withdrawERC20(
- address token,
+ function withdrawAsset(
+ bytes assetData,
uint256 amount
)
external;
diff --git a/packages/contracts/src/2.0.0/forwarder/mixins/MAssets.sol b/packages/contracts/src/2.0.0/forwarder/mixins/MAssets.sol
index 340ee0bcb..83636432a 100644
--- a/packages/contracts/src/2.0.0/forwarder/mixins/MAssets.sol
+++ b/packages/contracts/src/2.0.0/forwarder/mixins/MAssets.sol
@@ -28,7 +28,7 @@ contract MAssets is
/// @dev Transfers given amount of asset to sender.
/// @param assetData Byte array encoded for the respective asset proxy.
/// @param amount Amount of asset to transfer to sender.
- function transferPurchasedAssetToSender(
+ function transferAssetToSender(
bytes memory assetData,
uint256 amount
)
diff --git a/packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol b/packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol
index 5a2def7e5..360dea0e4 100644
--- a/packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol
+++ b/packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol
@@ -60,7 +60,7 @@ contract MExchangeWrapper {
/// @param makerAssetFillAmount Desired amount of makerAsset to buy.
/// @param signatures Proofs that orders have been signed by makers.
/// @return Amounts filled and fees paid by makers and taker.
- function marketBuyWithWeth(
+ function marketBuyExactAmountWithWeth(
LibOrder.Order[] memory orders,
uint256 makerAssetFillAmount,
bytes[] memory signatures
@@ -77,7 +77,7 @@ contract MExchangeWrapper {
/// @param zrxBuyAmount Desired amount of ZRX to buy.
/// @param signatures Proofs that orders have been created by makers.
/// @return totalFillResults Amounts filled and fees paid by maker and taker.
- function marketBuyZrxWithWeth(
+ function marketBuyExactZrxWithWeth(
LibOrder.Order[] memory orders,
uint256 zrxBuyAmount,
bytes[] memory signatures
diff --git a/packages/contracts/src/2.0.0/forwarder/mixins/MForwarderCore.sol b/packages/contracts/src/2.0.0/forwarder/mixins/MForwarderCore.sol
deleted file mode 100644
index 0f5cd9c66..000000000
--- a/packages/contracts/src/2.0.0/forwarder/mixins/MForwarderCore.sol
+++ /dev/null
@@ -1,42 +0,0 @@
-/*
-
- Copyright 2018 ZeroEx Intl.
-
- Licensed under the Apache License, Version 2.0 (the "License");
- you may not use this file except in compliance with the License.
- You may obtain a copy of the License at
-
- http://www.apache.org/licenses/LICENSE-2.0
-
- Unless required by applicable law or agreed to in writing, software
- distributed under the License is distributed on an "AS IS" BASIS,
- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- See the License for the specific language governing permissions and
- limitations under the License.
-
-*/
-
-pragma solidity 0.4.24;
-pragma experimental ABIEncoderV2;
-
-import "../../protocol/Exchange/libs/LibOrder.sol";
-import "../../protocol/Exchange/libs/LibFillResults.sol";
-import "../interfaces/IForwarderCore.sol";
-
-
-contract MForwarderCore is
- IForwarderCore
-{
-
- /// @dev Ensures that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold.
- /// @param orderFillResults Amounts filled and fees paid for primary orders.
- /// @param feeOrderFillResults Amounts filled and fees paid for fee orders.
- /// @param zrxBuyAmount The amount of ZRX that needed to be repurchased after filling primary orders.
- function assertValidFillResults(
- LibFillResults.FillResults memory orderFillResults,
- LibFillResults.FillResults memory feeOrderFillResults,
- uint256 zrxBuyAmount
- )
- internal
- view;
-}
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol
index b5de1a5de..821d30279 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol
@@ -142,7 +142,6 @@ contract MixinTransactions is
// Compute hash
result := keccak256(memPtr, 128)
}
-
return result;
}
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibAbiEncoder.sol b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibAbiEncoder.sol
index 704c7061c..4aad37709 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibAbiEncoder.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibAbiEncoder.sol
@@ -24,20 +24,17 @@ import "./LibOrder.sol";
contract LibAbiEncoder {
- /// @dev ABI encodes calldata for `fillOrder` in memory and returns the address range.
- /// This range can be passed into `call` or `delegatecall` to invoke an external
- /// call to `fillOrder`.
+ /// @dev ABI encodes calldata for `fillOrder`.
/// @param order Order struct containing order specifications.
/// @param takerAssetFillAmount Desired amount of takerAsset to sell.
/// @param signature Proof that order has been created by maker.
- /// @return calldataBegin Memory address of ABI encoded calldata.
- /// @return calldataLength Lenfgth of ABI encoded calldata.
+ /// @return ABI encoded calldata for `fillOrder`.
function abiEncodeFillOrder(
LibOrder.Order memory order,
uint256 takerAssetFillAmount,
bytes memory signature
)
- public
+ internal
pure
returns (bytes memory fillOrderCalldata)
{
@@ -207,10 +204,10 @@ contract LibAbiEncoder {
}
// Set length of calldata
- mstore(
- fillOrderCalldata,
- sub(dataAreaEnd, add(fillOrderCalldata, 0x20))
- )
+ mstore(fillOrderCalldata, sub(dataAreaEnd, add(fillOrderCalldata, 0x20)))
+
+ // Increment free memory pointer
+ mstore(0x40, dataAreaEnd)
}
return fillOrderCalldata;
diff --git a/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol b/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol
index 923bac97d..d9cec9edc 100644
--- a/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol
+++ b/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol
@@ -22,6 +22,7 @@ pragma experimental ABIEncoderV2;
import "../../protocol/Exchange/Exchange.sol";
+// solhint-disable no-empty-blocks
contract TestExchangeInternals is
Exchange
{
diff --git a/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol b/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol
index 5a349527b..4a99dd9c1 100644
--- a/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol
+++ b/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol
@@ -22,13 +22,33 @@ pragma experimental ABIEncoderV2;
import "../../protocol/Exchange/libs/LibMath.sol";
import "../../protocol/Exchange/libs/LibOrder.sol";
import "../../protocol/Exchange/libs/LibFillResults.sol";
+import "../../protocol/Exchange/libs/LibAbiEncoder.sol";
contract TestLibs is
LibMath,
LibOrder,
- LibFillResults
+ LibFillResults,
+ LibAbiEncoder
{
+
+ function publicAbiEncodeFillOrder(
+ Order memory order,
+ uint256 takerAssetFillAmount,
+ bytes memory signature
+ )
+ public
+ pure
+ returns (bytes memory fillOrderCalldata)
+ {
+ fillOrderCalldata = abiEncodeFillOrder(
+ order,
+ takerAssetFillAmount,
+ signature
+ );
+ return fillOrderCalldata;
+ }
+
function publicGetPartialAmount(
uint256 numerator,
uint256 denominator,
diff --git a/packages/contracts/test/forwarder/forwarder.ts b/packages/contracts/test/forwarder/forwarder.ts
index 19639d3aa..28ffdeabe 100644
--- a/packages/contracts/test/forwarder/forwarder.ts
+++ b/packages/contracts/test/forwarder/forwarder.ts
@@ -36,6 +36,7 @@ describe(ContractName.Forwarder, () => {
let feeRecipientAddress: string;
let otherAddress: string;
let defaultMakerAssetAddress: string;
+ let zrxAssetData: string;
let weth: DummyERC20TokenContract;
let zrxToken: DummyERC20TokenContract;
@@ -90,7 +91,7 @@ describe(ContractName.Forwarder, () => {
erc20Wrapper.addDummyTokenContract(weth);
const wethAssetData = assetDataUtils.encodeERC20AssetData(wethContract.address);
- const zrxAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address);
+ zrxAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address);
const exchangeInstance = await ExchangeContract.deployFrom0xArtifactAsync(
artifacts.Exchange,
provider,
@@ -722,25 +723,18 @@ describe(ContractName.Forwarder, () => {
);
expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT);
});
- it('should not change balances if the amount of ETH sent is too low to fill the makerAssetAmount', async () => {
+ it('should revert if the amount of ETH sent is too low to fill the makerAssetAmount', async () => {
const ordersWithoutFee = [orderWithoutFee];
const feeOrders: SignedOrder[] = [];
const makerAssetFillAmount = orderWithoutFee.makerAssetAmount.dividedToIntegerBy(2);
const ethValue = orderWithoutFee.takerAssetAmount.dividedToIntegerBy(4);
-
- tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, {
- value: ethValue,
- from: takerAddress,
- });
- const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress);
- const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address);
- const newBalances = await erc20Wrapper.getBalancesAsync();
-
- const totalEthSpent = gasPrice.times(tx.gasUsed);
-
- expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent));
- expect(newBalances).to.deep.equal(erc20Balances);
- expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT);
+ return expectTransactionFailedAsync(
+ forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, {
+ value: ethValue,
+ from: takerAddress,
+ }),
+ RevertReason.CompleteFillFailed,
+ );
});
it('should buy an ERC721 asset from a single order', async () => {
const makerAssetId = erc721MakerAssetIds[0];
@@ -775,7 +769,7 @@ describe(ContractName.Forwarder, () => {
);
expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT);
});
- it('should buy an ERC721 asset and ignore later orders with different makerAssetData', async () => {
+ it('should revert if buying an ERC721 asset when later orders contain different makerAssetData', async () => {
const makerAssetId = erc721MakerAssetIds[0];
orderWithoutFee = await orderFactory.newSignedOrderAsync({
makerAssetAmount: new BigNumber(1),
@@ -786,33 +780,12 @@ describe(ContractName.Forwarder, () => {
const feeOrders: SignedOrder[] = [];
const makerAssetFillAmount = new BigNumber(1).plus(differentMakerAssetDataOrder.makerAssetAmount);
const ethValue = orderWithFee.takerAssetAmount;
-
- tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, {
- from: takerAddress,
- value: ethValue,
- });
- const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress);
- const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address);
- const newOwner = await erc721Token.ownerOf.callAsync(makerAssetId);
- const newBalances = await erc20Wrapper.getBalancesAsync();
-
- const primaryTakerAssetFillAmount = ethValue;
- const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed));
- expect(newOwner).to.be.bignumber.equal(takerAddress);
- expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent));
- expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal(
- erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount),
- );
- expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT);
- expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal(
- constants.ZERO_AMOUNT,
- );
- expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT);
- expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal(
- erc20Balances[makerAddress][defaultMakerAssetAddress],
- );
- expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal(
- erc20Balances[takerAddress][defaultMakerAssetAddress],
+ return expectTransactionFailedAsync(
+ forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, {
+ value: ethValue,
+ from: takerAddress,
+ }),
+ RevertReason.CompleteFillFailed,
);
});
it('should buy an ERC721 asset and pay ZRX fees from a single fee order', async () => {
@@ -998,6 +971,26 @@ describe(ContractName.Forwarder, () => {
);
});
});
+ describe('withdrawAsset', () => {
+ it('should allow owner to withdraw ERC20 tokens', async () => {
+ const zrxWithdrawAmount = erc20Balances[forwarderContract.address][zrxToken.address];
+ await forwarderWrapper.withdrawAssetAsync(zrxAssetData, zrxWithdrawAmount, { from: owner });
+ const newBalances = await erc20Wrapper.getBalancesAsync();
+ expect(newBalances[owner][zrxToken.address]).to.be.bignumber.equal(
+ erc20Balances[owner][zrxToken.address].plus(zrxWithdrawAmount),
+ );
+ expect(newBalances[forwarderContract.address][zrxToken.address]).to.be.bignumber.equal(
+ erc20Balances[forwarderContract.address][zrxToken.address].minus(zrxWithdrawAmount),
+ );
+ });
+ it('should revert if not called by owner', async () => {
+ const zrxWithdrawAmount = erc20Balances[forwarderContract.address][zrxToken.address];
+ await expectTransactionFailedAsync(
+ forwarderWrapper.withdrawAssetAsync(zrxAssetData, zrxWithdrawAmount, { from: makerAddress }),
+ RevertReason.OnlyContractOwner,
+ );
+ });
+ });
});
// tslint:disable:max-file-line-count
// tslint:enable:no-unnecessary-type-assertion
diff --git a/packages/contracts/test/utils/exchange_wrapper.ts b/packages/contracts/test/utils/exchange_wrapper.ts
index 490ea959b..d57592d6d 100644
--- a/packages/contracts/test/utils/exchange_wrapper.ts
+++ b/packages/contracts/test/utils/exchange_wrapper.ts
@@ -8,7 +8,7 @@ import { ExchangeContract } from '../../generated_contract_wrappers/exchange';
import { formatters } from './formatters';
import { LogDecoder } from './log_decoder';
import { orderUtils } from './order_utils';
-import { OrderInfo, SignedTransaction } from './types';
+import { FillResults, OrderInfo, SignedTransaction } from './types';
export class ExchangeWrapper {
private readonly _exchange: ExchangeContract;
@@ -243,4 +243,27 @@ export class ExchangeWrapper {
const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
return tx;
}
+ public async getFillOrderResultsAsync(
+ signedOrder: SignedOrder,
+ from: string,
+ opts: { takerAssetFillAmount?: BigNumber } = {},
+ ): Promise<FillResults> {
+ const params = orderUtils.createFill(signedOrder, opts.takerAssetFillAmount);
+ const fillResults = await this._exchange.fillOrder.callAsync(
+ params.order,
+ params.takerAssetFillAmount,
+ params.signature,
+ { from },
+ );
+ return fillResults;
+ }
+ public abiEncodeFillOrder(signedOrder: SignedOrder, opts: { takerAssetFillAmount?: BigNumber } = {}): string {
+ const params = orderUtils.createFill(signedOrder, opts.takerAssetFillAmount);
+ const data = this._exchange.fillOrder.getABIEncodedTransactionData(
+ params.order,
+ params.takerAssetFillAmount,
+ params.signature,
+ );
+ return data;
+ }
}
diff --git a/packages/contracts/test/utils/fill_order_combinatorial_utils.ts b/packages/contracts/test/utils/fill_order_combinatorial_utils.ts
index bdd1e62a2..284c4a2db 100644
--- a/packages/contracts/test/utils/fill_order_combinatorial_utils.ts
+++ b/packages/contracts/test/utils/fill_order_combinatorial_utils.ts
@@ -15,6 +15,7 @@ import * as _ from 'lodash';
import 'make-promises-safe';
import { ExchangeContract, ExchangeFillEventArgs } from '../../generated_contract_wrappers/exchange';
+import { TestLibsContract } from '../../generated_contract_wrappers/test_libs';
import { artifacts } from './artifacts';
import { expectTransactionFailedAsync } from './assertions';
@@ -123,6 +124,8 @@ export async function fillOrderCombinatorialUtilsFactoryAsync(
exchangeContract.address,
);
+ const testLibsContract = await TestLibsContract.deployFrom0xArtifactAsync(artifacts.TestLibs, provider, txDefaults);
+
const fillOrderCombinatorialUtils = new FillOrderCombinatorialUtils(
orderFactory,
ownerAddress,
@@ -132,6 +135,7 @@ export async function fillOrderCombinatorialUtilsFactoryAsync(
zrxAssetData,
exchangeWrapper,
assetWrapper,
+ testLibsContract,
);
return fillOrderCombinatorialUtils;
}
@@ -145,6 +149,7 @@ export class FillOrderCombinatorialUtils {
public zrxAssetData: string;
public exchangeWrapper: ExchangeWrapper;
public assetWrapper: AssetWrapper;
+ public testLibsContract: TestLibsContract;
public static generateFillOrderCombinations(): FillScenario[] {
const takerScenarios = [
TakerScenario.Unspecified,
@@ -329,6 +334,7 @@ export class FillOrderCombinatorialUtils {
zrxAssetData: string,
exchangeWrapper: ExchangeWrapper,
assetWrapper: AssetWrapper,
+ testLibsContract: TestLibsContract,
) {
this.orderFactory = orderFactory;
this.ownerAddress = ownerAddress;
@@ -338,6 +344,7 @@ export class FillOrderCombinatorialUtils {
this.zrxAssetData = zrxAssetData;
this.exchangeWrapper = exchangeWrapper;
this.assetWrapper = assetWrapper;
+ this.testLibsContract = testLibsContract;
}
public async testFillOrderScenarioAsync(
provider: Provider,
@@ -410,6 +417,8 @@ export class FillOrderCombinatorialUtils {
lazyStore,
fillRevertReasonIfExists,
);
+
+ await this._abiEncodeFillOrderAndAssertOutcomeAsync(signedOrder, takerAssetFillAmount);
}
private async _fillOrderAndAssertOutcomeAsync(
signedOrder: SignedOrder,
@@ -456,6 +465,29 @@ export class FillOrderCombinatorialUtils {
signedOrder.takerAssetAmount,
signedOrder.makerAssetAmount,
);
+ const expMakerFeePaid = orderUtils.getPartialAmount(
+ expFilledTakerAmount,
+ signedOrder.takerAssetAmount,
+ signedOrder.makerFee,
+ );
+ const expTakerFeePaid = orderUtils.getPartialAmount(
+ expFilledTakerAmount,
+ signedOrder.takerAssetAmount,
+ signedOrder.takerFee,
+ );
+ const fillResults = await this.exchangeWrapper.getFillOrderResultsAsync(signedOrder, this.takerAddress, {
+ takerAssetFillAmount,
+ });
+ expect(fillResults.takerAssetFilledAmount).to.be.bignumber.equal(
+ expFilledTakerAmount,
+ 'takerAssetFilledAmount',
+ );
+ expect(fillResults.makerAssetFilledAmount).to.be.bignumber.equal(
+ expFilledMakerAmount,
+ 'makerAssetFilledAmount',
+ );
+ expect(fillResults.takerFeePaid).to.be.bignumber.equal(expTakerFeePaid, 'takerFeePaid');
+ expect(fillResults.makerFeePaid).to.be.bignumber.equal(expMakerFeePaid, 'makerFeePaid');
// - Let's fill the order!
const txReceipt = await this.exchangeWrapper.fillOrderAsync(signedOrder, this.takerAddress, {
@@ -479,17 +511,7 @@ export class FillOrderCombinatorialUtils {
expFilledTakerAmount,
'log.args.takerAssetFilledAmount',
);
- const expMakerFeePaid = orderUtils.getPartialAmount(
- expFilledTakerAmount,
- signedOrder.takerAssetAmount,
- signedOrder.makerFee,
- );
expect(log.args.makerFeePaid).to.be.bignumber.equal(expMakerFeePaid, 'log.args.makerFeePaid');
- const expTakerFeePaid = orderUtils.getPartialAmount(
- expFilledTakerAmount,
- signedOrder.takerAssetAmount,
- signedOrder.takerFee,
- );
expect(log.args.takerFeePaid).to.be.bignumber.equal(expTakerFeePaid, 'logs.args.takerFeePaid');
expect(log.args.orderHash).to.be.equal(orderHash, 'log.args.orderHash');
expect(log.args.makerAssetData).to.be.equal(makerAssetData, 'log.args.makerAssetData');
@@ -571,6 +593,19 @@ export class FillOrderCombinatorialUtils {
'ZRXAssetBalanceOfFeeRecipient',
);
}
+ private async _abiEncodeFillOrderAndAssertOutcomeAsync(
+ signedOrder: SignedOrder,
+ takerAssetFillAmount: BigNumber,
+ ): Promise<void> {
+ const params = orderUtils.createFill(signedOrder, takerAssetFillAmount);
+ const expectedAbiEncodedData = this.exchangeWrapper.abiEncodeFillOrder(signedOrder, { takerAssetFillAmount });
+ const libsAbiEncodedData = await this.testLibsContract.publicAbiEncodeFillOrder.callAsync(
+ params.order,
+ params.takerAssetFillAmount,
+ params.signature,
+ );
+ expect(libsAbiEncodedData).to.be.equal(expectedAbiEncodedData, 'ABIEncodedFillOrderData');
+ }
private async _getTakerAssetFillAmountAsync(
signedOrder: SignedOrder,
takerAssetFillAmountScenario: TakerAssetFillAmountScenario,
diff --git a/packages/contracts/test/utils/forwarder_wrapper.ts b/packages/contracts/test/utils/forwarder_wrapper.ts
index ef7476e36..5b9a63ddf 100644
--- a/packages/contracts/test/utils/forwarder_wrapper.ts
+++ b/packages/contracts/test/utils/forwarder_wrapper.ts
@@ -106,12 +106,12 @@ export class ForwarderWrapper {
const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
return tx;
}
- public async withdrawERC20Async(
- tokenAddress: string,
+ public async withdrawAssetAsync(
+ assetData: string,
amount: BigNumber,
txData: TxDataPayable,
): Promise<TransactionReceiptWithDecodedLogs> {
- const txHash = await this._forwarderContract.withdrawERC20.sendTransactionAsync(tokenAddress, amount, txData);
+ const txHash = await this._forwarderContract.withdrawAsset.sendTransactionAsync(assetData, amount, txData);
const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
return tx;
}