crowetic/protocol
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'fix/contracts/robustMatching' of github.com:0xProject/0x.js into fix/contracts/robustMatching

Browse Source
This commit is contained in:
Remco Bloemen 2018-08-24 18:54:15 -07:00
commit d652deea23
32 changed files with 1082 additions and 301 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
packages/contracts/compiler.json
View File

@ -40,6 +40,7 @@
"MultiSigWallet", "MultiSigWallet",
"MultiSigWalletWithTimeLock", "MultiSigWalletWithTimeLock",
"OrderValidator", "OrderValidator",
"ReentrantERC20Token",
"TestAssetProxyOwner", "TestAssetProxyOwner",
"TestAssetProxyDispatcher", "TestAssetProxyDispatcher",
"TestConstants", "TestConstants",

11
packages/contracts/package.json
View File

@ -20,11 +20,14 @@
"test:coverage": "SOLIDITY_COVERAGE=true run-s build run_mocha coverage:report:text coverage:report:lcov", "test:coverage": "SOLIDITY_COVERAGE=true run-s build run_mocha coverage:report:text coverage:report:lcov",
"test:profiler": "SOLIDITY_PROFILER=true run-s build run_mocha profiler:report:html", "test:profiler": "SOLIDITY_PROFILER=true run-s build run_mocha profiler:report:html",
"test:trace": "SOLIDITY_REVERT_TRACE=true run-s build run_mocha", "test:trace": "SOLIDITY_REVERT_TRACE=true run-s build run_mocha",
"run_mocha": "mocha --require source-map-support/register --require make-promises-safe 'lib/test/**/*.js' --timeout 100000 --bail --exit", "run_mocha":
"mocha --require source-map-support/register --require make-promises-safe 'lib/test/**/*.js' --timeout 100000 --bail --exit",
"compile": "sol-compiler --contracts-dir src", "compile": "sol-compiler --contracts-dir src",
"clean": "shx rm -rf lib generated_contract_wrappers", "clean": "shx rm -rf lib generated_contract_wrappers",
"generate_contract_wrappers": "abi-gen --abis ${npm_package_config_abis} --template ../contract_templates/contract.handlebars --partials '../contract_templates/partials/**/*.handlebars' --output generated_contract_wrappers --backend ethers", "generate_contract_wrappers":
"lint": "tslint --project . --exclude **/src/generated_contract_wrappers/**/* --exclude **/lib/**/* && yarn lint-contracts", "abi-gen --abis ${npm_package_config_abis} --template ../contract_templates/contract.handlebars --partials '../contract_templates/partials/**/*.handlebars' --output generated_contract_wrappers --backend ethers",
"lint":
"tslint --project . --exclude **/src/generated_contract_wrappers/**/* --exclude **/lib/**/* && yarn lint-contracts",
"coverage:report:text": "istanbul report text", "coverage:report:text": "istanbul report text",
"coverage:report:html": "istanbul report html && open coverage/index.html", "coverage:report:html": "istanbul report html && open coverage/index.html",
"profiler:report:html": "istanbul report html && open coverage/index.html", "profiler:report:html": "istanbul report html && open coverage/index.html",
@ -34,7 +37,7 @@
}, },
"config": { "config": {
"abis": "abis":
"../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|DummyNoReturnERC20Token|ERC20Proxy|ERC721Proxy|Forwarder|Exchange|ExchangeWrapper|IAssetData|IAssetProxy|InvalidERC721Receiver|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|OrderValidator|TestAssetProxyOwner|TestAssetProxyDispatcher|TestConstants|TestExchangeInternals|TestLibBytes|TestLibs|TestSignatureValidator|TestStaticCallReceiver|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|DummyNoReturnERC20Token|ERC20Proxy|ERC721Proxy|Forwarder|Exchange|ExchangeWrapper|IAssetData|IAssetProxy|InvalidERC721Receiver|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|OrderValidator|ReentrantERC20Token|TestAssetProxyOwner|TestAssetProxyDispatcher|TestConstants|TestExchangeInternals|TestLibBytes|TestLibs|TestSignatureValidator|TestStaticCallReceiver|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
}, },
"repository": { "repository": {
"type": "git", "type": "git",

4
packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol vendored
View File

@ -163,7 +163,7 @@ contract MixinExchangeWrapper is
// Convert the remaining amount of makerAsset to buy into remaining amount // 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 // of takerAsset to sell, assuming entire amount can be sold in the current order
uint256 remainingTakerAssetFillAmount = getPartialAmount( uint256 remainingTakerAssetFillAmount = getPartialAmountFloor(
orders[i].takerAssetAmount, orders[i].takerAssetAmount,
orders[i].makerAssetAmount, orders[i].makerAssetAmount,
remainingMakerAssetFillAmount remainingMakerAssetFillAmount
@ -231,7 +231,7 @@ contract MixinExchangeWrapper is
// Convert the remaining amount of ZRX to buy into remaining amount // Convert the remaining amount of ZRX to buy into remaining amount
// of WETH to sell, assuming entire amount can be sold in the current order. // of WETH to sell, assuming entire amount can be sold in the current order.
uint256 remainingWethSellAmount = getPartialAmount( uint256 remainingWethSellAmount = getPartialAmountFloor(
orders[i].takerAssetAmount, orders[i].takerAssetAmount,
safeSub(orders[i].makerAssetAmount, orders[i].takerFee), // our exchange rate after fees safeSub(orders[i].makerAssetAmount, orders[i].takerFee), // our exchange rate after fees
remainingZrxBuyAmount remainingZrxBuyAmount

4
packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol vendored
View File

@ -87,7 +87,7 @@ contract MixinForwarderCore is
uint256 makerAssetAmountPurchased; uint256 makerAssetAmountPurchased;
if (orders[0].makerAssetData.equals(ZRX_ASSET_DATA)) { if (orders[0].makerAssetData.equals(ZRX_ASSET_DATA)) {
// Calculate amount of WETH that won't be spent on ETH fees. // Calculate amount of WETH that won't be spent on ETH fees.
wethSellAmount = getPartialAmount( wethSellAmount = getPartialAmountFloor(
PERCENTAGE_DENOMINATOR, PERCENTAGE_DENOMINATOR,
safeAdd(PERCENTAGE_DENOMINATOR, feePercentage), safeAdd(PERCENTAGE_DENOMINATOR, feePercentage),
msg.value msg.value
@ -103,7 +103,7 @@ contract MixinForwarderCore is
makerAssetAmountPurchased = safeSub(orderFillResults.makerAssetFilledAmount, orderFillResults.takerFeePaid); makerAssetAmountPurchased = safeSub(orderFillResults.makerAssetFilledAmount, orderFillResults.takerFeePaid);
} else { } else {
// 5% of WETH is reserved for filling feeOrders and paying feeRecipient. // 5% of WETH is reserved for filling feeOrders and paying feeRecipient.
wethSellAmount = getPartialAmount( wethSellAmount = getPartialAmountFloor(
MAX_WETH_FILL_PERCENTAGE, MAX_WETH_FILL_PERCENTAGE,
PERCENTAGE_DENOMINATOR, PERCENTAGE_DENOMINATOR,
msg.value msg.value

2
packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol vendored
View File

@ -82,7 +82,7 @@ contract MixinWeth is
uint256 wethRemaining = safeSub(msg.value, wethSold); uint256 wethRemaining = safeSub(msg.value, wethSold);
// Calculate ETH fee to pay to feeRecipient. // Calculate ETH fee to pay to feeRecipient.
uint256 ethFee = getPartialAmount( uint256 ethFee = getPartialAmountFloor(
feePercentage, feePercentage,
PERCENTAGE_DENOMINATOR, PERCENTAGE_DENOMINATOR,
wethSoldExcludingFeeOrders wethSoldExcludingFeeOrders

2
packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol vendored
View File

@ -83,7 +83,7 @@ contract MixinAssetProxyDispatcher is
internal internal
{ {
// Do nothing if no amount should be transferred. // Do nothing if no amount should be transferred.
if (amount > 0) { if (amount > 0 && from != to) {
// Ensure assetData length is valid // Ensure assetData length is valid
require( require(
assetData.length > 3, assetData.length > 3,

112
packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol vendored
View File

@ -19,6 +19,7 @@
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2; pragma experimental ABIEncoderV2;
import "../../utils/ReentrancyGuard/ReentrancyGuard.sol";
import "./libs/LibConstants.sol"; import "./libs/LibConstants.sol";
import "./libs/LibFillResults.sol"; import "./libs/LibFillResults.sol";
import "./libs/LibOrder.sol"; import "./libs/LibOrder.sol";
@ -30,6 +31,7 @@ import "./mixins/MAssetProxyDispatcher.sol";
contract MixinExchangeCore is contract MixinExchangeCore is
ReentrancyGuard,
LibConstants, LibConstants,
LibMath, LibMath,
LibOrder, LibOrder,
@ -54,6 +56,7 @@ contract MixinExchangeCore is
/// @param targetOrderEpoch Orders created with a salt less or equal to this value will be cancelled. /// @param targetOrderEpoch Orders created with a salt less or equal to this value will be cancelled.
function cancelOrdersUpTo(uint256 targetOrderEpoch) function cancelOrdersUpTo(uint256 targetOrderEpoch)
external external
nonReentrant
{ {
address makerAddress = getCurrentContextAddress(); address makerAddress = getCurrentContextAddress();
// If this function is called via `executeTransaction`, we only update the orderEpoch for the makerAddress/msg.sender combination. // If this function is called via `executeTransaction`, we only update the orderEpoch for the makerAddress/msg.sender combination.
@ -86,50 +89,14 @@ contract MixinExchangeCore is
bytes memory signature bytes memory signature
) )
public public
nonReentrant
returns (FillResults memory fillResults) returns (FillResults memory fillResults)
{ {
// Fetch order info fillResults = fillOrderInternal(
OrderInfo memory orderInfo = getOrderInfo(order);
// Fetch taker address
address takerAddress = getCurrentContextAddress();
// Assert that the order is fillable by taker
assertFillableOrder(
order, order,
orderInfo, takerAssetFillAmount,
takerAddress,
signature signature
); );
// Get amount of takerAsset to fill
uint256 remainingTakerAssetAmount = safeSub(order.takerAssetAmount, orderInfo.orderTakerAssetFilledAmount);
uint256 takerAssetFilledAmount = min256(takerAssetFillAmount, remainingTakerAssetAmount);
// Compute proportional fill amounts
fillResults = calculateFillResults(order, takerAssetFilledAmount);
// Validate context
assertValidFill(
order,
orderInfo,
takerAssetFillAmount,
takerAssetFilledAmount,
fillResults.makerAssetFilledAmount
);
// Update exchange internal state
updateFilledState(
order,
takerAddress,
orderInfo.orderHash,
orderInfo.orderTakerAssetFilledAmount,
fillResults
);
// Settle order
settleOrder(order, takerAddress, fillResults);
return fillResults; return fillResults;
} }
@ -138,6 +105,7 @@ contract MixinExchangeCore is
/// @param order Order to cancel. Order must be OrderStatus.FILLABLE. /// @param order Order to cancel. Order must be OrderStatus.FILLABLE.
function cancelOrder(Order memory order) function cancelOrder(Order memory order)
public public
nonReentrant
{ {
// Fetch current order status // Fetch current order status
OrderInfo memory orderInfo = getOrderInfo(order); OrderInfo memory orderInfo = getOrderInfo(order);
@ -210,6 +178,64 @@ contract MixinExchangeCore is
return orderInfo; return orderInfo;
} }
/// @dev Fills the input order.
/// @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 Amounts filled and fees paid by maker and taker.
function fillOrderInternal(
Order memory order,
uint256 takerAssetFillAmount,
bytes memory signature
)
internal
returns (FillResults memory fillResults)
{
// Fetch order info
OrderInfo memory orderInfo = getOrderInfo(order);
// Fetch taker address
address takerAddress = getCurrentContextAddress();
// Assert that the order is fillable by taker
assertFillableOrder(
order,
orderInfo,
takerAddress,
signature
);
// Get amount of takerAsset to fill
uint256 remainingTakerAssetAmount = safeSub(order.takerAssetAmount, orderInfo.orderTakerAssetFilledAmount);
uint256 takerAssetFilledAmount = min256(takerAssetFillAmount, remainingTakerAssetAmount);
// Validate context
assertValidFill(
order,
orderInfo,
takerAssetFillAmount,
takerAssetFilledAmount,
fillResults.makerAssetFilledAmount
);
// Compute proportional fill amounts
fillResults = calculateFillResults(order, takerAssetFilledAmount);
// Update exchange internal state
updateFilledState(
order,
takerAddress,
orderInfo.orderHash,
orderInfo.orderTakerAssetFilledAmount,
fillResults
);
// Settle order
settleOrder(order, takerAddress, fillResults);
return fillResults;
}
/// @dev Updates state with results of a fill order. /// @dev Updates state with results of a fill order.
/// @param order that was filled. /// @param order that was filled.
/// @param takerAddress Address of taker who filled the order. /// @param takerAddress Address of taker who filled the order.
@ -381,7 +407,7 @@ contract MixinExchangeCore is
// Validate fill order rounding // Validate fill order rounding
require( require(
!isRoundingError( !isRoundingErrorFloor(
takerAssetFilledAmount, takerAssetFilledAmount,
order.takerAssetAmount, order.takerAssetAmount,
order.makerAssetAmount order.makerAssetAmount
@ -437,17 +463,17 @@ contract MixinExchangeCore is
{ {
// Compute proportional transfer amounts // Compute proportional transfer amounts
fillResults.takerAssetFilledAmount = takerAssetFilledAmount; fillResults.takerAssetFilledAmount = takerAssetFilledAmount;
fillResults.makerAssetFilledAmount = getPartialAmount( fillResults.makerAssetFilledAmount = getPartialAmountFloor(
takerAssetFilledAmount, takerAssetFilledAmount,
order.takerAssetAmount, order.takerAssetAmount,
order.makerAssetAmount order.makerAssetAmount
); );
fillResults.makerFeePaid = getPartialAmount( fillResults.makerFeePaid = getPartialAmountFloor(
takerAssetFilledAmount, takerAssetFilledAmount,
order.takerAssetAmount, order.takerAssetAmount,
order.makerFee order.makerFee
); );
fillResults.takerFeePaid = getPartialAmount( fillResults.takerFeePaid = getPartialAmountFloor(
takerAssetFilledAmount, takerAssetFilledAmount,
order.takerAssetAmount, order.takerAssetAmount,
order.takerFee order.takerFee

7
packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol vendored
View File

@ -14,6 +14,7 @@
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2; pragma experimental ABIEncoderV2;
import "../../utils/ReentrancyGuard/ReentrancyGuard.sol";
import "./libs/LibConstants.sol"; import "./libs/LibConstants.sol";
import "./libs/LibMath.sol"; import "./libs/LibMath.sol";
import "./libs/LibOrder.sol"; import "./libs/LibOrder.sol";
@ -25,6 +26,7 @@ import "./mixins/MAssetProxyDispatcher.sol";
contract MixinMatchOrders is contract MixinMatchOrders is
ReentrancyGuard,
LibConstants, LibConstants,
LibMath, LibMath,
MAssetProxyDispatcher, MAssetProxyDispatcher,
@ -48,6 +50,7 @@ contract MixinMatchOrders is
bytes memory rightSignature bytes memory rightSignature
) )
public public
nonReentrant
returns (LibFillResults.MatchedFillResults memory matchedFillResults) returns (LibFillResults.MatchedFillResults memory matchedFillResults)
{ {
// We assume that rightOrder.takerAssetData == leftOrder.makerAssetData and rightOrder.makerAssetData == leftOrder.takerAssetData. // We assume that rightOrder.takerAssetData == leftOrder.makerAssetData and rightOrder.makerAssetData == leftOrder.takerAssetData.
@ -193,7 +196,7 @@ contract MixinMatchOrders is
leftTakerAssetFilledAmount = leftTakerAssetAmountRemaining; leftTakerAssetFilledAmount = leftTakerAssetAmountRemaining;
// The right order receives an amount proportional to how much was spent. // The right order receives an amount proportional to how much was spent.
rightTakerAssetFilledAmount = getPartialAmount( rightTakerAssetFilledAmount = getPartialAmountFloor(
rightOrder.takerAssetAmount, rightOrder.takerAssetAmount,
rightOrder.makerAssetAmount, rightOrder.makerAssetAmount,
leftTakerAssetFilledAmount leftTakerAssetFilledAmount
@ -203,7 +206,7 @@ contract MixinMatchOrders is
rightTakerAssetFilledAmount = rightTakerAssetAmountRemaining; rightTakerAssetFilledAmount = rightTakerAssetAmountRemaining;
// The left order receives an amount proportional to how much was spent. // The left order receives an amount proportional to how much was spent.
leftTakerAssetFilledAmount = getPartialAmount( leftTakerAssetFilledAmount = getPartialAmountFloor(
rightOrder.makerAssetAmount, rightOrder.makerAssetAmount,
rightOrder.takerAssetAmount, rightOrder.takerAssetAmount,
rightTakerAssetFilledAmount rightTakerAssetFilledAmount

3
packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol vendored
View File

@ -19,6 +19,7 @@
pragma solidity 0.4.24; pragma solidity 0.4.24;
import "../../utils/LibBytes/LibBytes.sol"; import "../../utils/LibBytes/LibBytes.sol";
import "../../utils/ReentrancyGuard/ReentrancyGuard.sol";
import "./mixins/MSignatureValidator.sol"; import "./mixins/MSignatureValidator.sol";
import "./mixins/MTransactions.sol"; import "./mixins/MTransactions.sol";
import "./interfaces/IWallet.sol"; import "./interfaces/IWallet.sol";
@ -26,6 +27,7 @@ import "./interfaces/IValidator.sol";
contract MixinSignatureValidator is contract MixinSignatureValidator is
ReentrancyGuard,
MSignatureValidator, MSignatureValidator,
MTransactions MTransactions
{ {
@ -69,6 +71,7 @@ contract MixinSignatureValidator is
bool approval bool approval
) )
external external
nonReentrant
{ {
address signerAddress = getCurrentContextAddress(); address signerAddress = getCurrentContextAddress();
allowedValidators[signerAddress][validatorAddress] = approval; allowedValidators[signerAddress][validatorAddress] = approval;

3
packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol vendored
View File

@ -155,7 +155,8 @@ contract MixinTransactions is
view view
returns (address) returns (address)
{ {
address contextAddress = currentContextAddress == address(0) ? msg.sender : currentContextAddress; address currentContextAddress_ = currentContextAddress;
address contextAddress = currentContextAddress_ == address(0) ? msg.sender : currentContextAddress_;
return contextAddress; return contextAddress;
} }
} }

62
packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol vendored
View File

@ -19,18 +19,22 @@
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2; pragma experimental ABIEncoderV2;
import "../../utils/ReentrancyGuard/ReentrancyGuard.sol";
import "./libs/LibMath.sol"; import "./libs/LibMath.sol";
import "./libs/LibOrder.sol"; import "./libs/LibOrder.sol";
import "./libs/LibFillResults.sol"; import "./libs/LibFillResults.sol";
import "./libs/LibAbiEncoder.sol"; import "./libs/LibAbiEncoder.sol";
import "./mixins/MExchangeCore.sol"; import "./mixins/MExchangeCore.sol";
import "./mixins/MWrapperFunctions.sol";
contract MixinWrapperFunctions is contract MixinWrapperFunctions is
ReentrancyGuard,
LibMath, LibMath,
LibFillResults, LibFillResults,
LibAbiEncoder, LibAbiEncoder,
MExchangeCore MExchangeCore,
MWrapperFunctions
{ {
/// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled. /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled.
@ -43,17 +47,14 @@ contract MixinWrapperFunctions is
bytes memory signature bytes memory signature
) )
public public
nonReentrant
returns (FillResults memory fillResults) returns (FillResults memory fillResults)
{ {
fillResults = fillOrder( fillResults = fillOrKillOrderInternal(
order, order,
takerAssetFillAmount, takerAssetFillAmount,
signature signature
); );
require(
fillResults.takerAssetFilledAmount == takerAssetFillAmount,
"COMPLETE_FILL_FAILED"
);
return fillResults; return fillResults;
} }
@ -88,14 +89,7 @@ contract MixinWrapperFunctions is
fillOrderCalldata, // write output over input fillOrderCalldata, // write output over input
128 // output size is 128 bytes 128 // output size is 128 bytes
) )
switch success if success {
case 0 {
mstore(fillResults, 0)
mstore(add(fillResults, 32), 0)
mstore(add(fillResults, 64), 0)
mstore(add(fillResults, 96), 0)
}
case 1 {
mstore(fillResults, mload(fillOrderCalldata)) mstore(fillResults, mload(fillOrderCalldata))
mstore(add(fillResults, 32), mload(add(fillOrderCalldata, 32))) mstore(add(fillResults, 32), mload(add(fillOrderCalldata, 32)))
mstore(add(fillResults, 64), mload(add(fillOrderCalldata, 64))) mstore(add(fillResults, 64), mload(add(fillOrderCalldata, 64)))
@ -117,11 +111,12 @@ contract MixinWrapperFunctions is
bytes[] memory signatures bytes[] memory signatures
) )
public public
nonReentrant
returns (FillResults memory totalFillResults) returns (FillResults memory totalFillResults)
{ {
uint256 ordersLength = orders.length; uint256 ordersLength = orders.length;
for (uint256 i = 0; i != ordersLength; i++) { for (uint256 i = 0; i != ordersLength; i++) {
FillResults memory singleFillResults = fillOrder( FillResults memory singleFillResults = fillOrderInternal(
orders[i], orders[i],
takerAssetFillAmounts[i], takerAssetFillAmounts[i],
signatures[i] signatures[i]
@ -143,11 +138,12 @@ contract MixinWrapperFunctions is
bytes[] memory signatures bytes[] memory signatures
) )
public public
nonReentrant
returns (FillResults memory totalFillResults) returns (FillResults memory totalFillResults)
{ {
uint256 ordersLength = orders.length; uint256 ordersLength = orders.length;
for (uint256 i = 0; i != ordersLength; i++) { for (uint256 i = 0; i != ordersLength; i++) {
FillResults memory singleFillResults = fillOrKillOrder( FillResults memory singleFillResults = fillOrKillOrderInternal(
orders[i], orders[i],
takerAssetFillAmounts[i], takerAssetFillAmounts[i],
signatures[i] signatures[i]
@ -195,6 +191,7 @@ contract MixinWrapperFunctions is
bytes[] memory signatures bytes[] memory signatures
) )
public public
nonReentrant
returns (FillResults memory totalFillResults) returns (FillResults memory totalFillResults)
{ {
bytes memory takerAssetData = orders[0].takerAssetData; bytes memory takerAssetData = orders[0].takerAssetData;
@ -210,7 +207,7 @@ contract MixinWrapperFunctions is
uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount);
// Attempt to sell the remaining amount of takerAsset // Attempt to sell the remaining amount of takerAsset
FillResults memory singleFillResults = fillOrder( FillResults memory singleFillResults = fillOrderInternal(
orders[i], orders[i],
remainingTakerAssetFillAmount, remainingTakerAssetFillAmount,
signatures[i] signatures[i]
@ -282,6 +279,7 @@ contract MixinWrapperFunctions is
bytes[] memory signatures bytes[] memory signatures
) )
public public
nonReentrant
returns (FillResults memory totalFillResults) returns (FillResults memory totalFillResults)
{ {
bytes memory makerAssetData = orders[0].makerAssetData; bytes memory makerAssetData = orders[0].makerAssetData;
@ -298,14 +296,14 @@ contract MixinWrapperFunctions is
// Convert the remaining amount of makerAsset to buy into remaining amount // 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 // of takerAsset to sell, assuming entire amount can be sold in the current order
uint256 remainingTakerAssetFillAmount = getPartialAmount( uint256 remainingTakerAssetFillAmount = getPartialAmountFloor(
orders[i].takerAssetAmount, orders[i].takerAssetAmount,
orders[i].makerAssetAmount, orders[i].makerAssetAmount,
remainingMakerAssetFillAmount remainingMakerAssetFillAmount
); );
// Attempt to sell the remaining amount of takerAsset // Attempt to sell the remaining amount of takerAsset
FillResults memory singleFillResults = fillOrder( FillResults memory singleFillResults = fillOrderInternal(
orders[i], orders[i],
remainingTakerAssetFillAmount, remainingTakerAssetFillAmount,
signatures[i] signatures[i]
@ -350,7 +348,7 @@ contract MixinWrapperFunctions is
// Convert the remaining amount of makerAsset to buy into remaining amount // 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 // of takerAsset to sell, assuming entire amount can be sold in the current order
uint256 remainingTakerAssetFillAmount = getPartialAmount( uint256 remainingTakerAssetFillAmount = getPartialAmountFloor(
orders[i].takerAssetAmount, orders[i].takerAssetAmount,
orders[i].makerAssetAmount, orders[i].makerAssetAmount,
remainingMakerAssetFillAmount remainingMakerAssetFillAmount
@ -400,4 +398,28 @@ contract MixinWrapperFunctions is
} }
return ordersInfo; return ordersInfo;
} }
/// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled.
/// @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.
function fillOrKillOrderInternal(
LibOrder.Order memory order,
uint256 takerAssetFillAmount,
bytes memory signature
)
internal
returns (FillResults memory fillResults)
{
fillResults = fillOrderInternal(
order,
takerAssetFillAmount,
signature
);
require(
fillResults.takerAssetFilledAmount == takerAssetFillAmount,
"COMPLETE_FILL_FAILED"
);
return fillResults;
}
} }

118
packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol vendored
View File

@ -25,12 +25,12 @@ contract LibMath is
SafeMath SafeMath
{ {
/// @dev Calculates partial value given a numerator and denominator. /// @dev Calculates partial value given a numerator and denominator rounded down.
/// @param numerator Numerator. /// @param numerator Numerator.
/// @param denominator Denominator. /// @param denominator Denominator.
/// @param target Value to calculate partial of. /// @param target Value to calculate partial of.
/// @return Partial value of target. /// @return Partial value of target rounded down.
function getPartialAmount( function getPartialAmountFloor(
uint256 numerator, uint256 numerator,
uint256 denominator, uint256 denominator,
uint256 target uint256 target
@ -39,6 +39,11 @@ contract LibMath is
pure pure
returns (uint256 partialAmount) returns (uint256 partialAmount)
{ {
require(
denominator > 0,
"DIVISION_BY_ZERO"
);
partialAmount = safeDiv( partialAmount = safeDiv(
safeMul(numerator, target), safeMul(numerator, target),
denominator denominator
@ -46,12 +51,44 @@ contract LibMath is
return partialAmount; return partialAmount;
} }
/// @dev Checks if rounding error > 0.1%. /// @dev Calculates partial value given a numerator and denominator rounded down.
/// @param numerator Numerator.
/// @param denominator Denominator.
/// @param target Value to calculate partial of.
/// @return Partial value of target rounded up.
function getPartialAmountCeil(
uint256 numerator,
uint256 denominator,
uint256 target
)
internal
pure
returns (uint256 partialAmount)
{
require(
denominator > 0,
"DIVISION_BY_ZERO"
);
// safeDiv computes `floor(a / b)`. We use the identity (a, b integer):
// ceil(a / b) = floor((a + b - 1) / b)
// To implement `ceil(a / b)` using safeDiv.
partialAmount = safeDiv(
safeAdd(
safeMul(numerator, target),
safeSub(denominator, 1)
),
denominator
);
return partialAmount;
}
/// @dev Checks if rounding error >= 0.1% when rounding down.
/// @param numerator Numerator. /// @param numerator Numerator.
/// @param denominator Denominator. /// @param denominator Denominator.
/// @param target Value to multiply with numerator/denominator. /// @param target Value to multiply with numerator/denominator.
/// @return Rounding error is present. /// @return Rounding error is present.
function isRoundingError( function isRoundingErrorFloor(
uint256 numerator, uint256 numerator,
uint256 denominator, uint256 denominator,
uint256 target uint256 target
@ -60,16 +97,73 @@ contract LibMath is
pure pure
returns (bool isError) returns (bool isError)
{ {
uint256 remainder = mulmod(target, numerator, denominator); require(
if (remainder == 0) { denominator > 0,
return false; // No rounding error. "DIVISION_BY_ZERO"
);
// The absolute rounding error is the difference between the rounded
// value and the ideal value. The relative rounding error is the
// absolute rounding error divided by the absolute value of the
// ideal value. This is undefined when the ideal value is zero.
//
// The ideal value is `numerator * target / denominator`.
// Let's call `numerator * target % denominator` the remainder.
// The absolute error is `remainder / denominator`.
//
// When the ideal value is zero, we require the absolute error to
// be zero. Fortunately, this is always the case. The ideal value is
// zero iff `numerator == 0` and/or `target == 0`. In this case the
// remainder and absolute error are also zero.
if (target == 0 || numerator == 0) {
return false;
} }
uint256 errPercentageTimes1000000 = safeDiv( // Otherwise, we want the relative rounding error to be strictly
safeMul(remainder, 1000000), // less than 0.1%.
safeMul(numerator, target) // The relative error is `remainder / (numerator * target)`.
// We want the relative error less than 1 / 1000:
// remainder / (numerator * denominator) < 1 / 1000
// or equivalently:
// 1000 * remainder < numerator * target
// so we have a rounding error iff:
// 1000 * remainder >= numerator * target
uint256 remainder = mulmod(target, numerator, denominator);
isError = safeMul(1000, remainder) >= safeMul(numerator, target);
return isError;
}
/// @dev Checks if rounding error >= 0.1% when rounding up.
/// @param numerator Numerator.
/// @param denominator Denominator.
/// @param target Value to multiply with numerator/denominator.
/// @return Rounding error is present.
function isRoundingErrorCeil(
uint256 numerator,
uint256 denominator,
uint256 target
)
internal
pure
returns (bool isError)
{
require(
denominator > 0,
"DIVISION_BY_ZERO"
); );
isError = errPercentageTimes1000000 > 1000;
// See the comments in `isRoundingError`.
if (target == 0 || numerator == 0) {
// When either is zero, the ideal value and rounded value are zero
// and there is no rounding error. (Although the relative error
// is undefined.)
return false;
}
// Compute remainder as before
uint256 remainder = mulmod(target, numerator, denominator);
// TODO: safeMod
remainder = safeSub(denominator, remainder) % denominator;
isError = safeMul(1000, remainder) >= safeMul(numerator, target);
return isError; return isError;
} }
} }

13
packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol vendored
View File

@ -59,6 +59,19 @@ contract MExchangeCore is
uint256 orderEpoch // Orders with specified makerAddress and senderAddress with a salt less than this value are considered cancelled. uint256 orderEpoch // Orders with specified makerAddress and senderAddress with a salt less than this value are considered cancelled.
); );
/// @dev Fills the input order.
/// @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 Amounts filled and fees paid by maker and taker.
function fillOrderInternal(
LibOrder.Order memory order,
uint256 takerAssetFillAmount,
bytes memory signature
)
internal
returns (LibFillResults.FillResults memory fillResults);
/// @dev Updates state with results of a fill order. /// @dev Updates state with results of a fill order.
/// @param order that was filled. /// @param order that was filled.
/// @param takerAddress Address of taker who filled the order. /// @param takerAddress Address of taker who filled the order.

40
packages/contracts/src/2.0.0/protocol/Exchange/mixins/MWrapperFunctions.sol vendored Normal file
View File

@ -0,0 +1,40 @@
/*
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 "../libs/LibOrder.sol";
import "../libs/LibFillResults.sol";
import "../interfaces/IWrapperFunctions.sol";
contract MWrapperFunctions {
/// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled.
/// @param order LibOrder.Order struct containing order specifications.
/// @param takerAssetFillAmount Desired amount of takerAsset to sell.
/// @param signature Proof that order has been created by maker.
function fillOrKillOrderInternal(
LibOrder.Order memory order,
uint256 takerAssetFillAmount,
bytes memory signature
)
internal
returns (LibFillResults.FillResults memory fillResults);
}

182
packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol vendored Normal file
View File

@ -0,0 +1,182 @@
/*
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 "../../utils/LibBytes/LibBytes.sol";
import "../../tokens/ERC20Token/ERC20Token.sol";
import "../../protocol/Exchange/interfaces/IExchange.sol";
import "../../protocol/Exchange/libs/LibOrder.sol";
contract ReentrantERC20Token is
ERC20Token
{
using LibBytes for bytes;
// solhint-disable-next-line var-name-mixedcase
IExchange internal EXCHANGE;
bytes internal constant REENTRANCY_ILLEGAL_REVERT_REASON = abi.encodeWithSelector(
bytes4(keccak256("Error(string)")),
"REENTRANCY_ILLEGAL"
);
// All of these functions are potentially vulnerable to reentrancy
// We do not test any "noThrow" functions because `fillOrderNoThrow` makes a delegatecall to `fillOrder`
enum ExchangeFunction {
FILL_ORDER,
FILL_OR_KILL_ORDER,
BATCH_FILL_ORDERS,
BATCH_FILL_OR_KILL_ORDERS,
MARKET_BUY_ORDERS,
MARKET_SELL_ORDERS,
MATCH_ORDERS,
CANCEL_ORDER,
CANCEL_ORDERS_UP_TO,
SET_SIGNATURE_VALIDATOR_APPROVAL
}
uint8 internal currentFunctionId = 0;
constructor (address _exchange)
public
{
EXCHANGE = IExchange(_exchange);
}
/// @dev Set the current function that will be called when `transferFrom` is called.
/// @param _currentFunctionId Id that corresponds to function name.
function setCurrentFunction(uint8 _currentFunctionId)
external
{
currentFunctionId = _currentFunctionId;
}
/// @dev A version of `transferFrom` that attempts to reenter the Exchange contract.
/// @param _from The address of the sender
/// @param _to The address of the recipient
/// @param _value The amount of token to be transferred
function transferFrom(
address _from,
address _to,
uint256 _value
)
external
returns (bool)
{
// This order would normally be invalid, but it will be used strictly for testing reentrnacy.
// Any reentrancy checks will happen before any other checks that invalidate the order.
LibOrder.Order memory order;
// Initialize remaining null parameters
bytes memory signature;
LibOrder.Order[] memory orders;
uint256[] memory takerAssetFillAmounts;
bytes[] memory signatures;
bytes memory calldata;
// Create calldata for function that corresponds to currentFunctionId
if (currentFunctionId == uint8(ExchangeFunction.FILL_ORDER)) {
calldata = abi.encodeWithSelector(
EXCHANGE.fillOrder.selector,
order,
0,
signature
);
} else if (currentFunctionId == uint8(ExchangeFunction.FILL_OR_KILL_ORDER)) {
calldata = abi.encodeWithSelector(
EXCHANGE.fillOrKillOrder.selector,
order,
0,
signature
);
} else if (currentFunctionId == uint8(ExchangeFunction.BATCH_FILL_ORDERS)) {
calldata = abi.encodeWithSelector(
EXCHANGE.batchFillOrders.selector,
orders,
takerAssetFillAmounts,
signatures
);
} else if (currentFunctionId == uint8(ExchangeFunction.BATCH_FILL_OR_KILL_ORDERS)) {
calldata = abi.encodeWithSelector(
EXCHANGE.batchFillOrKillOrders.selector,
orders,
takerAssetFillAmounts,
signatures
);
} else if (currentFunctionId == uint8(ExchangeFunction.MARKET_BUY_ORDERS)) {
calldata = abi.encodeWithSelector(
EXCHANGE.marketBuyOrders.selector,
orders,
0,
signatures
);
} else if (currentFunctionId == uint8(ExchangeFunction.MARKET_SELL_ORDERS)) {
calldata = abi.encodeWithSelector(
EXCHANGE.marketSellOrders.selector,
orders,
0,
signatures
);
} else if (currentFunctionId == uint8(ExchangeFunction.MATCH_ORDERS)) {
calldata = abi.encodeWithSelector(
EXCHANGE.matchOrders.selector,
order,
order,
signature,
signature
);
} else if (currentFunctionId == uint8(ExchangeFunction.CANCEL_ORDER)) {
calldata = abi.encodeWithSelector(
EXCHANGE.cancelOrder.selector,
order
);
} else if (currentFunctionId == uint8(ExchangeFunction.CANCEL_ORDERS_UP_TO)) {
calldata = abi.encodeWithSelector(
EXCHANGE.cancelOrdersUpTo.selector,
0
);
} else if (currentFunctionId == uint8(ExchangeFunction.SET_SIGNATURE_VALIDATOR_APPROVAL)) {
calldata = abi.encodeWithSelector(
EXCHANGE.setSignatureValidatorApproval.selector,
address(0),
false
);
}
// Call Exchange function, swallow error
address(EXCHANGE).call(calldata);
// Revert reason is 100 bytes
bytes memory returnData = new bytes(100);
// Copy return data
assembly {
returndatacopy(add(returnData, 32), 0, 100)
}
// Revert if function reverted with REENTRANCY_ILLEGAL error
require(!REENTRANCY_ILLEGAL_REVERT_REASON.equals(returnData));
// Transfer will return true if function failed for any other reason
return true;
}
}

44
packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol vendored
View File

@ -67,7 +67,7 @@ contract TestExchangeInternals is
/// @param denominator Denominator. /// @param denominator Denominator.
/// @param target Value to calculate partial of. /// @param target Value to calculate partial of.
/// @return Partial value of target. /// @return Partial value of target.
function publicGetPartialAmount( function publicGetPartialAmountFloor(
uint256 numerator, uint256 numerator,
uint256 denominator, uint256 denominator,
uint256 target uint256 target
@ -76,15 +76,32 @@ contract TestExchangeInternals is
pure pure
returns (uint256 partialAmount) returns (uint256 partialAmount)
{ {
return getPartialAmount(numerator, denominator, target); return getPartialAmountFloor(numerator, denominator, target);
} }
/// @dev Checks if rounding error > 0.1%. /// @dev Calculates partial value given a numerator and denominator.
/// @param numerator Numerator.
/// @param denominator Denominator.
/// @param target Value to calculate partial of.
/// @return Partial value of target.
function publicGetPartialAmountCeil(
uint256 numerator,
uint256 denominator,
uint256 target
)
public
pure
returns (uint256 partialAmount)
{
return getPartialAmountCeil(numerator, denominator, target);
}
/// @dev Checks if rounding error >= 0.1%.
/// @param numerator Numerator. /// @param numerator Numerator.
/// @param denominator Denominator. /// @param denominator Denominator.
/// @param target Value to multiply with numerator/denominator. /// @param target Value to multiply with numerator/denominator.
/// @return Rounding error is present. /// @return Rounding error is present.
function publicIsRoundingError( function publicIsRoundingErrorFloor(
uint256 numerator, uint256 numerator,
uint256 denominator, uint256 denominator,
uint256 target uint256 target
@ -93,7 +110,24 @@ contract TestExchangeInternals is
pure pure
returns (bool isError) returns (bool isError)
{ {
return isRoundingError(numerator, denominator, target); return isRoundingErrorFloor(numerator, denominator, target);
}
/// @dev Checks if rounding error >= 0.1%.
/// @param numerator Numerator.
/// @param denominator Denominator.
/// @param target Value to multiply with numerator/denominator.
/// @return Rounding error is present.
function publicIsRoundingErrorCeil(
uint256 numerator,
uint256 denominator,
uint256 target
)
public
pure
returns (bool isError)
{
return isRoundingErrorCeil(numerator, denominator, target);
} }
/// @dev Updates state with results of a fill order. /// @dev Updates state with results of a fill order.

42
packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol vendored
View File

@ -49,7 +49,7 @@ contract TestLibs is
return fillOrderCalldata; return fillOrderCalldata;
} }
function publicGetPartialAmount( function publicGetPartialAmountFloor(
uint256 numerator, uint256 numerator,
uint256 denominator, uint256 denominator,
uint256 target uint256 target
@ -58,7 +58,7 @@ contract TestLibs is
pure pure
returns (uint256 partialAmount) returns (uint256 partialAmount)
{ {
partialAmount = getPartialAmount( partialAmount = getPartialAmountFloor(
numerator, numerator,
denominator, denominator,
target target
@ -66,7 +66,24 @@ contract TestLibs is
return partialAmount; return partialAmount;
} }
function publicIsRoundingError( function publicGetPartialAmountCeil(
uint256 numerator,
uint256 denominator,
uint256 target
)
public
pure
returns (uint256 partialAmount)
{
partialAmount = getPartialAmountCeil(
numerator,
denominator,
target
);
return partialAmount;
}
function publicIsRoundingErrorFloor(
uint256 numerator, uint256 numerator,
uint256 denominator, uint256 denominator,
uint256 target uint256 target
@ -75,7 +92,24 @@ contract TestLibs is
pure pure
returns (bool isError) returns (bool isError)
{ {
isError = isRoundingError( isError = isRoundingErrorFloor(
numerator,
denominator,
target
);
return isError;
}
function publicIsRoundingErrorCeil(
uint256 numerator,
uint256 denominator,
uint256 target
)
public
pure
returns (bool isError)
{
isError = isRoundingErrorCeil(
numerator, numerator,
denominator, denominator,
target target

44
packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol vendored Normal file
View File

@ -0,0 +1,44 @@
/*
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;
contract ReentrancyGuard {
// Locked state of mutex
bool private locked = false;
/// @dev Functions with this modifer cannot be reentered. The mutex will be locked
/// before function execution and unlocked after.
modifier nonReentrant() {
// Ensure mutex is unlocked
require(
!locked,
"REENTRANCY_ILLEGAL"
);
// Lock mutex before function call
locked = true;
// Perform function call
_;
// Unlock mutex after function call
locked = false;
}
}

30
packages/contracts/test/exchange/core.ts
View File

@ -14,6 +14,7 @@ import { DummyNoReturnERC20TokenContract } from '../../generated_contract_wrappe
import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy';
import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy';
import { ExchangeCancelEventArgs, ExchangeContract } from '../../generated_contract_wrappers/exchange'; import { ExchangeCancelEventArgs, ExchangeContract } from '../../generated_contract_wrappers/exchange';
import { ReentrantERC20TokenContract } from '../../generated_contract_wrappers/reentrant_erc20_token';
import { TestStaticCallReceiverContract } from '../../generated_contract_wrappers/test_static_call_receiver'; import { TestStaticCallReceiverContract } from '../../generated_contract_wrappers/test_static_call_receiver';
import { artifacts } from '../utils/artifacts'; import { artifacts } from '../utils/artifacts';
import { expectTransactionFailedAsync } from '../utils/assertions'; import { expectTransactionFailedAsync } from '../utils/assertions';
@ -42,6 +43,7 @@ describe('Exchange core', () => {
let zrxToken: DummyERC20TokenContract; let zrxToken: DummyERC20TokenContract;
let erc721Token: DummyERC721TokenContract; let erc721Token: DummyERC721TokenContract;
let noReturnErc20Token: DummyNoReturnERC20TokenContract; let noReturnErc20Token: DummyNoReturnERC20TokenContract;
let reentrantErc20Token: ReentrantERC20TokenContract;
let exchange: ExchangeContract; let exchange: ExchangeContract;
let erc20Proxy: ERC20ProxyContract; let erc20Proxy: ERC20ProxyContract;
let erc721Proxy: ERC721ProxyContract; let erc721Proxy: ERC721ProxyContract;
@ -117,6 +119,12 @@ describe('Exchange core', () => {
provider, provider,
txDefaults, txDefaults,
); );
reentrantErc20Token = await ReentrantERC20TokenContract.deployFrom0xArtifactAsync(
artifacts.ReentrantERC20Token,
provider,
txDefaults,
exchange.address,
);
defaultMakerAssetAddress = erc20TokenA.address; defaultMakerAssetAddress = erc20TokenA.address;
defaultTakerAssetAddress = erc20TokenB.address; defaultTakerAssetAddress = erc20TokenB.address;
@ -144,6 +152,26 @@ describe('Exchange core', () => {
signedOrder = await orderFactory.newSignedOrderAsync(); signedOrder = await orderFactory.newSignedOrderAsync();
}); });
const reentrancyTest = (functionNames: string[]) => {
_.forEach(functionNames, async (functionName: string, functionId: number) => {
const description = `should not allow fillOrder to reenter the Exchange contract via ${functionName}`;
it(description, async () => {
signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address),
});
await web3Wrapper.awaitTransactionSuccessAsync(
await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await expectTransactionFailedAsync(
exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
RevertReason.TransferFailed,
);
});
});
};
describe('fillOrder reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX));
it('should throw if signature is invalid', async () => { it('should throw if signature is invalid', async () => {
signedOrder = await orderFactory.newSignedOrderAsync({ signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
@ -502,7 +530,7 @@ describe('Exchange core', () => {
// HACK(albrow): We need to hardcode the gas estimate here because // HACK(albrow): We need to hardcode the gas estimate here because
// the Geth gas estimator doesn't work with the way we use // the Geth gas estimator doesn't work with the way we use
// delegatecall and swallow errors. // delegatecall and swallow errors.
gas: 490000, gas: 600000,
}); });
const newBalances = await erc20Wrapper.getBalancesAsync(); const newBalances = await erc20Wrapper.getBalancesAsync();

158
packages/contracts/test/exchange/internal.ts
View File

@ -1,6 +1,7 @@
import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { BlockchainLifecycle } from '@0xproject/dev-utils';
import { Order, RevertReason, SignedOrder } from '@0xproject/types'; import { Order, RevertReason, SignedOrder } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils'; import { BigNumber } from '@0xproject/utils';
import * as chai from 'chai';
import * as _ from 'lodash'; import * as _ from 'lodash';
import { TestExchangeInternalsContract } from '../../generated_contract_wrappers/test_exchange_internals'; import { TestExchangeInternalsContract } from '../../generated_contract_wrappers/test_exchange_internals';
@ -16,6 +17,8 @@ import { FillResults } from '../utils/types';
import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper';
chaiSetup.configure(); chaiSetup.configure();
const expect = chai.expect;
const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
const MAX_UINT256 = new BigNumber(2).pow(256).minus(1); const MAX_UINT256 = new BigNumber(2).pow(256).minus(1);
@ -43,26 +46,11 @@ const emptySignedOrder: SignedOrder = {
const overflowErrorForCall = new Error(RevertReason.Uint256Overflow); const overflowErrorForCall = new Error(RevertReason.Uint256Overflow);
async function referenceGetPartialAmountAsync(
numerator: BigNumber,
denominator: BigNumber,
target: BigNumber,
): Promise<BigNumber> {
const invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync());
const product = numerator.mul(target);
if (product.greaterThan(MAX_UINT256)) {
throw overflowErrorForCall;
}
if (denominator.eq(0)) {
throw invalidOpcodeErrorForCall;
}
return product.dividedToIntegerBy(denominator);
}
describe('Exchange core internal functions', () => { describe('Exchange core internal functions', () => {
let testExchange: TestExchangeInternalsContract; let testExchange: TestExchangeInternalsContract;
let invalidOpcodeErrorForCall: Error | undefined; let invalidOpcodeErrorForCall: Error | undefined;
let overflowErrorForSendTransaction: Error | undefined; let overflowErrorForSendTransaction: Error | undefined;
let divisionByZeroErrorForCall: Error | undefined;
before(async () => { before(async () => {
await blockchainLifecycle.startAsync(); await blockchainLifecycle.startAsync();
@ -79,11 +67,29 @@ describe('Exchange core internal functions', () => {
overflowErrorForSendTransaction = new Error( overflowErrorForSendTransaction = new Error(
await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.Uint256Overflow), await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.Uint256Overflow),
); );
divisionByZeroErrorForCall = new Error(
await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.DivisionByZero),
);
invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync()); invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync());
}); });
// Note(albrow): Don't forget to add beforeEach and afterEach calls to reset // Note(albrow): Don't forget to add beforeEach and afterEach calls to reset
// the blockchain state for any tests which modify it! // the blockchain state for any tests which modify it!
async function referenceGetPartialAmountFloorAsync(
numerator: BigNumber,
denominator: BigNumber,
target: BigNumber,
): Promise<BigNumber> {
if (denominator.eq(0)) {
throw divisionByZeroErrorForCall;
}
const product = numerator.mul(target);
if (product.greaterThan(MAX_UINT256)) {
throw overflowErrorForCall;
}
return product.dividedToIntegerBy(denominator);
}
describe('addFillResults', async () => { describe('addFillResults', async () => {
function makeFillResults(value: BigNumber): FillResults { function makeFillResults(value: BigNumber): FillResults {
return { return {
@ -159,18 +165,18 @@ describe('Exchange core internal functions', () => {
// implementation or the Solidity implementation of // implementation or the Solidity implementation of
// calculateFillResults. // calculateFillResults.
return { return {
makerAssetFilledAmount: await referenceGetPartialAmountAsync( makerAssetFilledAmount: await referenceGetPartialAmountFloorAsync(
takerAssetFilledAmount, takerAssetFilledAmount,
orderTakerAssetAmount, orderTakerAssetAmount,
otherAmount, otherAmount,
), ),
takerAssetFilledAmount, takerAssetFilledAmount,
makerFeePaid: await referenceGetPartialAmountAsync( makerFeePaid: await referenceGetPartialAmountFloorAsync(
takerAssetFilledAmount, takerAssetFilledAmount,
orderTakerAssetAmount, orderTakerAssetAmount,
otherAmount, otherAmount,
), ),
takerFeePaid: await referenceGetPartialAmountAsync( takerFeePaid: await referenceGetPartialAmountFloorAsync(
takerAssetFilledAmount, takerAssetFilledAmount,
orderTakerAssetAmount, orderTakerAssetAmount,
otherAmount, otherAmount,
@ -193,18 +199,55 @@ describe('Exchange core internal functions', () => {
); );
}); });
describe('getPartialAmount', async () => { describe('getPartialAmountFloor', async () => {
async function testGetPartialAmountAsync( async function testGetPartialAmountFloorAsync(
numerator: BigNumber, numerator: BigNumber,
denominator: BigNumber, denominator: BigNumber,
target: BigNumber, target: BigNumber,
): Promise<BigNumber> { ): Promise<BigNumber> {
return testExchange.publicGetPartialAmount.callAsync(numerator, denominator, target); return testExchange.publicGetPartialAmountFloor.callAsync(numerator, denominator, target);
} }
await testCombinatoriallyWithReferenceFuncAsync( await testCombinatoriallyWithReferenceFuncAsync(
'getPartialAmount', 'getPartialAmount',
referenceGetPartialAmountAsync, referenceGetPartialAmountFloorAsync,
testGetPartialAmountAsync, testGetPartialAmountFloorAsync,
[uint256Values, uint256Values, uint256Values],
);
});
describe('getPartialAmountCeil', async () => {
async function referenceGetPartialAmountCeilAsync(
numerator: BigNumber,
denominator: BigNumber,
target: BigNumber,
): Promise<BigNumber> {
if (denominator.eq(0)) {
throw divisionByZeroErrorForCall;
}
const product = numerator.mul(target);
const offset = product.add(denominator.sub(1));
if (offset.greaterThan(MAX_UINT256)) {
throw overflowErrorForCall;
}
const result = offset.dividedToIntegerBy(denominator);
if (product.mod(denominator).eq(0)) {
expect(result.mul(denominator)).to.be.bignumber.eq(product);
} else {
expect(result.mul(denominator)).to.be.bignumber.gt(product);
}
return result;
}
async function testGetPartialAmountCeilAsync(
numerator: BigNumber,
denominator: BigNumber,
target: BigNumber,
): Promise<BigNumber> {
return testExchange.publicGetPartialAmountCeil.callAsync(numerator, denominator, target);
}
await testCombinatoriallyWithReferenceFuncAsync(
'getPartialAmountCeil',
referenceGetPartialAmountCeilAsync,
testGetPartialAmountCeilAsync,
[uint256Values, uint256Values, uint256Values], [uint256Values, uint256Values, uint256Values],
); );
}); });
@ -215,33 +258,33 @@ describe('Exchange core internal functions', () => {
denominator: BigNumber, denominator: BigNumber,
target: BigNumber, target: BigNumber,
): Promise<boolean> { ): Promise<boolean> {
const product = numerator.mul(target);
if (denominator.eq(0)) { if (denominator.eq(0)) {
throw invalidOpcodeErrorForCall; throw divisionByZeroErrorForCall;
} }
const remainder = product.mod(denominator); if (numerator.eq(0)) {
if (remainder.eq(0)) {
return false; return false;
} }
if (target.eq(0)) {
return false;
}
const product = numerator.mul(target);
const remainder = product.mod(denominator);
const remainderTimes1000 = remainder.mul('1000');
const isError = remainderTimes1000.gt(product);
if (product.greaterThan(MAX_UINT256)) { if (product.greaterThan(MAX_UINT256)) {
throw overflowErrorForCall; throw overflowErrorForCall;
} }
if (product.eq(0)) { if (remainderTimes1000.greaterThan(MAX_UINT256)) {
throw invalidOpcodeErrorForCall;
}
const remainderTimes1000000 = remainder.mul('1000000');
if (remainderTimes1000000.greaterThan(MAX_UINT256)) {
throw overflowErrorForCall; throw overflowErrorForCall;
} }
const errPercentageTimes1000000 = remainderTimes1000000.dividedToIntegerBy(product); return isError;
return errPercentageTimes1000000.greaterThan('1000');
} }
async function testIsRoundingErrorAsync( async function testIsRoundingErrorAsync(
numerator: BigNumber, numerator: BigNumber,
denominator: BigNumber, denominator: BigNumber,
target: BigNumber, target: BigNumber,
): Promise<boolean> { ): Promise<boolean> {
return testExchange.publicIsRoundingError.callAsync(numerator, denominator, target); return testExchange.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target);
} }
await testCombinatoriallyWithReferenceFuncAsync( await testCombinatoriallyWithReferenceFuncAsync(
'isRoundingError', 'isRoundingError',
@ -251,6 +294,49 @@ describe('Exchange core internal functions', () => {
); );
}); });
describe('isRoundingErrorCeil', async () => {
async function referenceIsRoundingErrorAsync(
numerator: BigNumber,
denominator: BigNumber,
target: BigNumber,
): Promise<boolean> {
if (denominator.eq(0)) {
throw divisionByZeroErrorForCall;
}
if (numerator.eq(0)) {
return false;
}
if (target.eq(0)) {
return false;
}
const product = numerator.mul(target);
const remainder = product.mod(denominator);
const error = denominator.sub(remainder).mod(denominator);
const errorTimes1000 = error.mul('1000');
const isError = errorTimes1000.gt(product);
if (product.greaterThan(MAX_UINT256)) {
throw overflowErrorForCall;
}
if (errorTimes1000.greaterThan(MAX_UINT256)) {
throw overflowErrorForCall;
}
return isError;
}
async function testIsRoundingErrorCeilAsync(
numerator: BigNumber,
denominator: BigNumber,
target: BigNumber,
): Promise<boolean> {
return testExchange.publicIsRoundingErrorCeil.callAsync(numerator, denominator, target);
}
await testCombinatoriallyWithReferenceFuncAsync(
'isRoundingErrorCeil',
referenceIsRoundingErrorAsync,
testIsRoundingErrorCeilAsync,
[uint256Values, uint256Values, uint256Values],
);
});
describe('updateFilledState', async () => { describe('updateFilledState', async () => {
// Note(albrow): Since updateFilledState modifies the state by calling // Note(albrow): Since updateFilledState modifies the state by calling
// sendTransaction, we must reset the state after each test. // sendTransaction, we must reset the state after each test.

38
packages/contracts/test/exchange/libs.ts
View File

@ -71,20 +71,21 @@ describe('Exchange libs', () => {
// combinatorial tests in test/exchange/internal. They test specific edge // combinatorial tests in test/exchange/internal. They test specific edge
// cases that are not covered by the combinatorial tests. // cases that are not covered by the combinatorial tests.
describe('LibMath', () => { describe('LibMath', () => {
it('should return false if there is a rounding error of 0.1%', async () => { describe('isRoundingError', () => {
it('should return true if there is a rounding error of 0.1%', async () => {
const numerator = new BigNumber(20); const numerator = new BigNumber(20);
const denominator = new BigNumber(999); const denominator = new BigNumber(999);
const target = new BigNumber(50); const target = new BigNumber(50);
// rounding error = ((20*50/999) - floor(20*50/999)) / (20*50/999) = 0.1% // rounding error = ((20*50/999) - floor(20*50/999)) / (20*50/999) = 0.1%
const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); const isRoundingError = await libs.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target);
expect(isRoundingError).to.be.false(); expect(isRoundingError).to.be.true();
}); });
it('should return false if there is a rounding of 0.09%', async () => { it('should return false if there is a rounding of 0.09%', async () => {
const numerator = new BigNumber(20); const numerator = new BigNumber(20);
const denominator = new BigNumber(9991); const denominator = new BigNumber(9991);
const target = new BigNumber(500); const target = new BigNumber(500);
// rounding error = ((20*500/9991) - floor(20*500/9991)) / (20*500/9991) = 0.09% // rounding error = ((20*500/9991) - floor(20*500/9991)) / (20*500/9991) = 0.09%
const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); const isRoundingError = await libs.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target);
expect(isRoundingError).to.be.false(); expect(isRoundingError).to.be.false();
}); });
it('should return true if there is a rounding error of 0.11%', async () => { it('should return true if there is a rounding error of 0.11%', async () => {
@ -92,10 +93,37 @@ describe('Exchange libs', () => {
const denominator = new BigNumber(9989); const denominator = new BigNumber(9989);
const target = new BigNumber(500); const target = new BigNumber(500);
// rounding error = ((20*500/9989) - floor(20*500/9989)) / (20*500/9989) = 0.011% // rounding error = ((20*500/9989) - floor(20*500/9989)) / (20*500/9989) = 0.011%
const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); const isRoundingError = await libs.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target);
expect(isRoundingError).to.be.true(); expect(isRoundingError).to.be.true();
}); });
}); });
describe('isRoundingErrorCeil', () => {
it('should return true if there is a rounding error of 0.1%', async () => {
const numerator = new BigNumber(20);
const denominator = new BigNumber(1001);
const target = new BigNumber(50);
// rounding error = (ceil(20*50/1001) - (20*50/1001)) / (20*50/1001) = 0.1%
const isRoundingError = await libs.publicIsRoundingErrorCeil.callAsync(numerator, denominator, target);
expect(isRoundingError).to.be.true();
});
it('should return false if there is a rounding of 0.09%', async () => {
const numerator = new BigNumber(20);
const denominator = new BigNumber(10009);
const target = new BigNumber(500);
// rounding error = (ceil(20*500/10009) - (20*500/10009)) / (20*500/10009) = 0.09%
const isRoundingError = await libs.publicIsRoundingErrorCeil.callAsync(numerator, denominator, target);
expect(isRoundingError).to.be.false();
});
it('should return true if there is a rounding error of 0.11%', async () => {
const numerator = new BigNumber(20);
const denominator = new BigNumber(10011);
const target = new BigNumber(500);
// rounding error = (ceil(20*500/10011) - (20*500/10011)) / (20*500/10011) = 0.11%
const isRoundingError = await libs.publicIsRoundingErrorCeil.callAsync(numerator, denominator, target);
expect(isRoundingError).to.be.true();
});
});
});
describe('LibOrder', () => { describe('LibOrder', () => {
describe('getOrderSchema', () => { describe('getOrderSchema', () => {

173
packages/contracts/test/exchange/match_orders.ts
View File

@ -11,6 +11,7 @@ import { DummyERC721TokenContract } from '../../generated_contract_wrappers/dumm
import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy';
import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy';
import { ExchangeContract } from '../../generated_contract_wrappers/exchange'; import { ExchangeContract } from '../../generated_contract_wrappers/exchange';
import { ReentrantERC20TokenContract } from '../../generated_contract_wrappers/reentrant_erc20_token';
import { artifacts } from '../utils/artifacts'; import { artifacts } from '../utils/artifacts';
import { expectTransactionFailedAsync } from '../utils/assertions'; import { expectTransactionFailedAsync } from '../utils/assertions';
import { chaiSetup } from '../utils/chai_setup'; import { chaiSetup } from '../utils/chai_setup';
@ -39,6 +40,7 @@ describe('matchOrders', () => {
let erc20TokenB: DummyERC20TokenContract; let erc20TokenB: DummyERC20TokenContract;
let zrxToken: DummyERC20TokenContract; let zrxToken: DummyERC20TokenContract;
let erc721Token: DummyERC721TokenContract; let erc721Token: DummyERC721TokenContract;
let reentrantErc20Token: ReentrantERC20TokenContract;
let exchange: ExchangeContract; let exchange: ExchangeContract;
let erc20Proxy: ERC20ProxyContract; let erc20Proxy: ERC20ProxyContract;
let erc721Proxy: ERC721ProxyContract; let erc721Proxy: ERC721ProxyContract;
@ -127,21 +129,39 @@ describe('matchOrders', () => {
}), }),
constants.AWAIT_TRANSACTION_MINED_MS, constants.AWAIT_TRANSACTION_MINED_MS,
); );
reentrantErc20Token = await ReentrantERC20TokenContract.deployFrom0xArtifactAsync(
artifacts.ReentrantERC20Token,
provider,
txDefaults,
exchange.address,
);
// Set default addresses // Set default addresses
defaultERC20MakerAssetAddress = erc20TokenA.address; defaultERC20MakerAssetAddress = erc20TokenA.address;
defaultERC20TakerAssetAddress = erc20TokenB.address; defaultERC20TakerAssetAddress = erc20TokenB.address;
defaultERC721AssetAddress = erc721Token.address; defaultERC721AssetAddress = erc721Token.address;
// Create default order parameters // Create default order parameters
const defaultOrderParams = { const defaultOrderParamsLeft = {
...constants.STATIC_ORDER_PARAMS, ...constants.STATIC_ORDER_PARAMS,
makerAddress: makerAddressLeft,
exchangeAddress: exchange.address, exchangeAddress: exchange.address,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
feeRecipientAddress: feeRecipientAddressLeft,
};
const defaultOrderParamsRight = {
...constants.STATIC_ORDER_PARAMS,
makerAddress: makerAddressRight,
exchangeAddress: exchange.address,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
feeRecipientAddress: feeRecipientAddressRight,
}; };
const privateKeyLeft = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressLeft)]; const privateKeyLeft = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressLeft)];
orderFactoryLeft = new OrderFactory(privateKeyLeft, defaultOrderParams); orderFactoryLeft = new OrderFactory(privateKeyLeft, defaultOrderParamsLeft);
const privateKeyRight = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressRight)]; const privateKeyRight = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressRight)];
orderFactoryRight = new OrderFactory(privateKeyRight, defaultOrderParams); orderFactoryRight = new OrderFactory(privateKeyRight, defaultOrderParamsRight);
// Set match order tester // Set match order tester
matchOrderTester = new MatchOrderTester(exchangeWrapper, erc20Wrapper, erc721Wrapper, zrxToken.address); matchOrderTester = new MatchOrderTester(exchangeWrapper, erc20Wrapper, erc721Wrapper, zrxToken.address);
}); });
@ -157,22 +177,45 @@ describe('matchOrders', () => {
erc721TokenIdsByOwner = await erc721Wrapper.getBalancesAsync(); erc721TokenIdsByOwner = await erc721Wrapper.getBalancesAsync();
}); });
it('should transfer the correct amounts when orders completely fill each other', async () => { const reentrancyTest = (functionNames: string[]) => {
// Create orders to match _.forEach(functionNames, async (functionName: string, functionId: number) => {
const description = `should not allow matchOrders to reenter the Exchange contract via ${functionName}`;
it(description, async () => {
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft, makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight, makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), takerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress: feeRecipientAddressRight, feeRecipientAddress: feeRecipientAddressRight,
}); });
await web3Wrapper.awaitTransactionSuccessAsync(
await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await expectTransactionFailedAsync(
exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress),
RevertReason.TransferFailed,
);
});
});
};
describe('matchOrders reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX));
it('should transfer the correct amounts when orders completely fill each other', async () => {
// Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
});
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
});
// Match signedOrderLeft with signedOrderRight // Match signedOrderLeft with signedOrderRight
await matchOrderTester.matchOrdersAndVerifyBalancesAsync( await matchOrderTester.matchOrdersAndVerifyBalancesAsync(
signedOrderLeft, signedOrderLeft,
@ -192,18 +235,12 @@ describe('matchOrders', () => {
it('should transfer the correct amounts when orders completely fill each other and taker doesnt take a profit', async () => { it('should transfer the correct amounts when orders completely fill each other and taker doesnt take a profit', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Store original taker balance // Store original taker balance
const takerInitialBalances = _.cloneDeep(erc20BalancesByOwner[takerAddress][defaultERC20MakerAssetAddress]); const takerInitialBalances = _.cloneDeep(erc20BalancesByOwner[takerAddress][defaultERC20MakerAssetAddress]);
@ -237,18 +274,12 @@ describe('matchOrders', () => {
it('should transfer the correct amounts when left order is completely filled and right order is partially filled', async () => { it('should transfer the correct amounts when left order is completely filled and right order is partially filled', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match orders // Match orders
await matchOrderTester.matchOrdersAndVerifyBalancesAsync( await matchOrderTester.matchOrdersAndVerifyBalancesAsync(
@ -269,18 +300,12 @@ describe('matchOrders', () => {
it('should transfer the correct amounts when right order is completely filled and left order is partially filled', async () => { it('should transfer the correct amounts when right order is completely filled and left order is partially filled', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match orders // Match orders
await matchOrderTester.matchOrdersAndVerifyBalancesAsync( await matchOrderTester.matchOrdersAndVerifyBalancesAsync(
@ -301,18 +326,12 @@ describe('matchOrders', () => {
it('should transfer the correct amounts when consecutive calls are used to completely fill the left order', async () => { it('should transfer the correct amounts when consecutive calls are used to completely fill the left order', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match orders // Match orders
let newERC20BalancesByOwner: ERC20BalancesByOwner; let newERC20BalancesByOwner: ERC20BalancesByOwner;
@ -340,12 +359,8 @@ describe('matchOrders', () => {
// However, we use 100/50 to ensure a partial fill as we want to go down the "left fill" // However, we use 100/50 to ensure a partial fill as we want to go down the "left fill"
// branch in the contract twice for this test. // branch in the contract twice for this test.
const signedOrderRight2 = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight2 = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match signedOrderLeft with signedOrderRight2 // Match signedOrderLeft with signedOrderRight2
const leftTakerAssetFilledAmount = signedOrderRight.makerAssetAmount; const leftTakerAssetFilledAmount = signedOrderRight.makerAssetAmount;
@ -370,19 +385,13 @@ describe('matchOrders', () => {
it('should transfer the correct amounts when consecutive calls are used to completely fill the right order', async () => { it('should transfer the correct amounts when consecutive calls are used to completely fill the right order', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match orders // Match orders
let newERC20BalancesByOwner: ERC20BalancesByOwner; let newERC20BalancesByOwner: ERC20BalancesByOwner;
@ -410,10 +419,8 @@ describe('matchOrders', () => {
// However, we use 100/50 to ensure a partial fill as we want to go down the "right fill" // However, we use 100/50 to ensure a partial fill as we want to go down the "right fill"
// branch in the contract twice for this test. // branch in the contract twice for this test.
const signedOrderLeft2 = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft2 = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(50), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
// Match signedOrderLeft2 with signedOrderRight // Match signedOrderLeft2 with signedOrderRight
const leftTakerAssetFilledAmount = new BigNumber(0); const leftTakerAssetFilledAmount = new BigNumber(0);
@ -441,15 +448,11 @@ describe('matchOrders', () => {
it('should transfer the correct amounts if fee recipient is the same across both matched orders', async () => { it('should transfer the correct amounts if fee recipient is the same across both matched orders', async () => {
const feeRecipientAddress = feeRecipientAddressLeft; const feeRecipientAddress = feeRecipientAddressLeft;
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress, feeRecipientAddress,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress, feeRecipientAddress,
@ -467,18 +470,12 @@ describe('matchOrders', () => {
it('should transfer the correct amounts if taker is also the left order maker', async () => { it('should transfer the correct amounts if taker is also the left order maker', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match orders // Match orders
takerAddress = signedOrderLeft.makerAddress; takerAddress = signedOrderLeft.makerAddress;
@ -494,18 +491,12 @@ describe('matchOrders', () => {
it('should transfer the correct amounts if taker is also the right order maker', async () => { it('should transfer the correct amounts if taker is also the right order maker', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match orders // Match orders
takerAddress = signedOrderRight.makerAddress; takerAddress = signedOrderRight.makerAddress;
@ -521,18 +512,12 @@ describe('matchOrders', () => {
it('should transfer the correct amounts if taker is also the left fee recipient', async () => { it('should transfer the correct amounts if taker is also the left fee recipient', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match orders // Match orders
takerAddress = feeRecipientAddressLeft; takerAddress = feeRecipientAddressLeft;
@ -548,18 +533,12 @@ describe('matchOrders', () => {
it('should transfer the correct amounts if taker is also the right fee recipient', async () => { it('should transfer the correct amounts if taker is also the right fee recipient', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match orders // Match orders
takerAddress = feeRecipientAddressRight; takerAddress = feeRecipientAddressRight;
@ -575,18 +554,12 @@ describe('matchOrders', () => {
it('should transfer the correct amounts if left maker is the left fee recipient and right maker is the right fee recipient', async () => { it('should transfer the correct amounts if left maker is the left fee recipient and right maker is the right fee recipient', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: makerAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress: makerAddressRight,
}); });
// Match orders // Match orders
await matchOrderTester.matchOrdersAndVerifyBalancesAsync( await matchOrderTester.matchOrdersAndVerifyBalancesAsync(
@ -601,18 +574,12 @@ describe('matchOrders', () => {
it('Should throw if left order is not fillable', async () => { it('Should throw if left order is not fillable', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Cancel left order // Cancel left order
await exchangeWrapper.cancelOrderAsync(signedOrderLeft, signedOrderLeft.makerAddress); await exchangeWrapper.cancelOrderAsync(signedOrderLeft, signedOrderLeft.makerAddress);
@ -626,18 +593,12 @@ describe('matchOrders', () => {
it('Should throw if right order is not fillable', async () => { it('Should throw if right order is not fillable', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Cancel right order // Cancel right order
await exchangeWrapper.cancelOrderAsync(signedOrderRight, signedOrderRight.makerAddress); await exchangeWrapper.cancelOrderAsync(signedOrderRight, signedOrderRight.makerAddress);
@ -651,18 +612,12 @@ describe('matchOrders', () => {
it('should throw if there is not a positive spread', async () => { it('should throw if there is not a positive spread', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(200), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(200), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match orders // Match orders
return expectTransactionFailedAsync( return expectTransactionFailedAsync(
@ -674,18 +629,13 @@ describe('matchOrders', () => {
it('should throw if the left maker asset is not equal to the right taker asset ', async () => { it('should throw if the left maker asset is not equal to the right taker asset ', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress), takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match orders // Match orders
return expectTransactionFailedAsync( return expectTransactionFailedAsync(
@ -701,20 +651,13 @@ describe('matchOrders', () => {
it('should throw if the right maker asset is not equal to the left taker asset', async () => { it('should throw if the right maker asset is not equal to the left taker asset', async () => {
// Create orders to match // Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress), takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match orders // Match orders
return expectTransactionFailedAsync( return expectTransactionFailedAsync(
@ -727,20 +670,14 @@ describe('matchOrders', () => {
// Create orders to match // Create orders to match
const erc721TokenToTransfer = erc721LeftMakerAssetIds[0]; const erc721TokenToTransfer = erc721LeftMakerAssetIds[0];
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetData: assetDataUtils.encodeERC721AssetData(defaultERC721AssetAddress, erc721TokenToTransfer), makerAssetData: assetDataUtils.encodeERC721AssetData(defaultERC721AssetAddress, erc721TokenToTransfer),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
makerAssetAmount: new BigNumber(1), makerAssetAmount: new BigNumber(1),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC721AssetData(defaultERC721AssetAddress, erc721TokenToTransfer), takerAssetData: assetDataUtils.encodeERC721AssetData(defaultERC721AssetAddress, erc721TokenToTransfer),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: new BigNumber(1), takerAssetAmount: new BigNumber(1),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match orders // Match orders
await matchOrderTester.matchOrdersAndVerifyBalancesAsync( await matchOrderTester.matchOrdersAndVerifyBalancesAsync(
@ -762,20 +699,14 @@ describe('matchOrders', () => {
// Create orders to match // Create orders to match
const erc721TokenToTransfer = erc721RightMakerAssetIds[0]; const erc721TokenToTransfer = erc721RightMakerAssetIds[0];
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC721AssetData(defaultERC721AssetAddress, erc721TokenToTransfer), takerAssetData: assetDataUtils.encodeERC721AssetData(defaultERC721AssetAddress, erc721TokenToTransfer),
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
takerAssetAmount: new BigNumber(1), takerAssetAmount: new BigNumber(1),
feeRecipientAddress: feeRecipientAddressLeft,
}); });
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC721AssetData(defaultERC721AssetAddress, erc721TokenToTransfer), makerAssetData: assetDataUtils.encodeERC721AssetData(defaultERC721AssetAddress, erc721TokenToTransfer),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
makerAssetAmount: new BigNumber(1), makerAssetAmount: new BigNumber(1),
takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
feeRecipientAddress: feeRecipientAddressRight,
}); });
// Match orders // Match orders
await matchOrderTester.matchOrdersAndVerifyBalancesAsync( await matchOrderTester.matchOrdersAndVerifyBalancesAsync(

193
packages/contracts/test/exchange/wrapper.ts
View File

@ -11,6 +11,7 @@ import { DummyERC721TokenContract } from '../../generated_contract_wrappers/dumm
import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy';
import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy';
import { ExchangeContract } from '../../generated_contract_wrappers/exchange'; import { ExchangeContract } from '../../generated_contract_wrappers/exchange';
import { ReentrantERC20TokenContract } from '../../generated_contract_wrappers/reentrant_erc20_token';
import { artifacts } from '../utils/artifacts'; import { artifacts } from '../utils/artifacts';
import { expectTransactionFailedAsync } from '../utils/assertions'; import { expectTransactionFailedAsync } from '../utils/assertions';
import { getLatestBlockTimestampAsync, increaseTimeAndMineBlockAsync } from '../utils/block_timestamp'; import { getLatestBlockTimestampAsync, increaseTimeAndMineBlockAsync } from '../utils/block_timestamp';
@ -40,6 +41,7 @@ describe('Exchange wrappers', () => {
let exchange: ExchangeContract; let exchange: ExchangeContract;
let erc20Proxy: ERC20ProxyContract; let erc20Proxy: ERC20ProxyContract;
let erc721Proxy: ERC721ProxyContract; let erc721Proxy: ERC721ProxyContract;
let reentrantErc20Token: ReentrantERC20TokenContract;
let exchangeWrapper: ExchangeWrapper; let exchangeWrapper: ExchangeWrapper;
let erc20Wrapper: ERC20Wrapper; let erc20Wrapper: ERC20Wrapper;
@ -104,6 +106,13 @@ describe('Exchange wrappers', () => {
constants.AWAIT_TRANSACTION_MINED_MS, constants.AWAIT_TRANSACTION_MINED_MS,
); );
reentrantErc20Token = await ReentrantERC20TokenContract.deployFrom0xArtifactAsync(
artifacts.ReentrantERC20Token,
provider,
txDefaults,
exchange.address,
);
defaultMakerAssetAddress = erc20TokenA.address; defaultMakerAssetAddress = erc20TokenA.address;
defaultTakerAssetAddress = erc20TokenB.address; defaultTakerAssetAddress = erc20TokenB.address;
@ -126,6 +135,26 @@ describe('Exchange wrappers', () => {
await blockchainLifecycle.revertAsync(); await blockchainLifecycle.revertAsync();
}); });
describe('fillOrKillOrder', () => { describe('fillOrKillOrder', () => {
const reentrancyTest = (functionNames: string[]) => {
_.forEach(functionNames, async (functionName: string, functionId: number) => {
const description = `should not allow fillOrKillOrder to reenter the Exchange contract via ${functionName}`;
it(description, async () => {
const signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address),
});
await web3Wrapper.awaitTransactionSuccessAsync(
await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await expectTransactionFailedAsync(
exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress),
RevertReason.TransferFailed,
);
});
});
};
describe('fillOrKillOrder reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX));
it('should transfer the correct amounts', async () => { it('should transfer the correct amounts', async () => {
const signedOrder = await orderFactory.newSignedOrderAsync({ const signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
@ -197,6 +226,25 @@ describe('Exchange wrappers', () => {
}); });
describe('fillOrderNoThrow', () => { describe('fillOrderNoThrow', () => {
const reentrancyTest = (functionNames: string[]) => {
_.forEach(functionNames, async (functionName: string, functionId: number) => {
const description = `should not allow fillOrderNoThrow to reenter the Exchange contract via ${functionName}`;
it(description, async () => {
const signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address),
});
await web3Wrapper.awaitTransactionSuccessAsync(
await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await exchangeWrapper.fillOrderNoThrowAsync(signedOrder, takerAddress);
const newBalances = await erc20Wrapper.getBalancesAsync();
expect(erc20Balances).to.deep.equal(newBalances);
});
});
};
describe('fillOrderNoThrow reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX));
it('should transfer the correct amounts', async () => { it('should transfer the correct amounts', async () => {
const signedOrder = await orderFactory.newSignedOrderAsync({ const signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
@ -397,6 +445,26 @@ describe('Exchange wrappers', () => {
}); });
describe('batchFillOrders', () => { describe('batchFillOrders', () => {
const reentrancyTest = (functionNames: string[]) => {
_.forEach(functionNames, async (functionName: string, functionId: number) => {
const description = `should not allow batchFillOrders to reenter the Exchange contract via ${functionName}`;
it(description, async () => {
const signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address),
});
await web3Wrapper.awaitTransactionSuccessAsync(
await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await expectTransactionFailedAsync(
exchangeWrapper.batchFillOrdersAsync([signedOrder], takerAddress),
RevertReason.TransferFailed,
);
});
});
};
describe('batchFillOrders reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX));
it('should transfer the correct amounts', async () => { it('should transfer the correct amounts', async () => {
const takerAssetFillAmounts: BigNumber[] = []; const takerAssetFillAmounts: BigNumber[] = [];
const makerAssetAddress = erc20TokenA.address; const makerAssetAddress = erc20TokenA.address;
@ -446,6 +514,26 @@ describe('Exchange wrappers', () => {
}); });
describe('batchFillOrKillOrders', () => { describe('batchFillOrKillOrders', () => {
const reentrancyTest = (functionNames: string[]) => {
_.forEach(functionNames, async (functionName: string, functionId: number) => {
const description = `should not allow batchFillOrKillOrders to reenter the Exchange contract via ${functionName}`;
it(description, async () => {
const signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address),
});
await web3Wrapper.awaitTransactionSuccessAsync(
await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await expectTransactionFailedAsync(
exchangeWrapper.batchFillOrKillOrdersAsync([signedOrder], takerAddress),
RevertReason.TransferFailed,
);
});
});
};
describe('batchFillOrKillOrders reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX));
it('should transfer the correct amounts', async () => { it('should transfer the correct amounts', async () => {
const takerAssetFillAmounts: BigNumber[] = []; const takerAssetFillAmounts: BigNumber[] = [];
const makerAssetAddress = erc20TokenA.address; const makerAssetAddress = erc20TokenA.address;
@ -512,6 +600,25 @@ describe('Exchange wrappers', () => {
}); });
describe('batchFillOrdersNoThrow', async () => { describe('batchFillOrdersNoThrow', async () => {
const reentrancyTest = (functionNames: string[]) => {
_.forEach(functionNames, async (functionName: string, functionId: number) => {
const description = `should not allow batchFillOrdersNoThrow to reenter the Exchange contract via ${functionName}`;
it(description, async () => {
const signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address),
});
await web3Wrapper.awaitTransactionSuccessAsync(
await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await exchangeWrapper.batchFillOrdersNoThrowAsync([signedOrder], takerAddress);
const newBalances = await erc20Wrapper.getBalancesAsync();
expect(erc20Balances).to.deep.equal(newBalances);
});
});
};
describe('batchFillOrdersNoThrow reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX));
it('should transfer the correct amounts', async () => { it('should transfer the correct amounts', async () => {
const takerAssetFillAmounts: BigNumber[] = []; const takerAssetFillAmounts: BigNumber[] = [];
const makerAssetAddress = erc20TokenA.address; const makerAssetAddress = erc20TokenA.address;
@ -625,6 +732,28 @@ describe('Exchange wrappers', () => {
}); });
describe('marketSellOrders', () => { describe('marketSellOrders', () => {
const reentrancyTest = (functionNames: string[]) => {
_.forEach(functionNames, async (functionName: string, functionId: number) => {
const description = `should not allow marketSellOrders to reenter the Exchange contract via ${functionName}`;
it(description, async () => {
const signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address),
});
await web3Wrapper.awaitTransactionSuccessAsync(
await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await expectTransactionFailedAsync(
exchangeWrapper.marketSellOrdersAsync([signedOrder], takerAddress, {
takerAssetFillAmount: signedOrder.takerAssetAmount,
}),
RevertReason.TransferFailed,
);
});
});
};
describe('marketSellOrders reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX));
it('should stop when the entire takerAssetFillAmount is filled', async () => { it('should stop when the entire takerAssetFillAmount is filled', async () => {
const takerAssetFillAmount = signedOrders[0].takerAssetAmount.plus( const takerAssetFillAmount = signedOrders[0].takerAssetAmount.plus(
signedOrders[1].takerAssetAmount.div(2), signedOrders[1].takerAssetAmount.div(2),
@ -717,6 +846,27 @@ describe('Exchange wrappers', () => {
}); });
describe('marketSellOrdersNoThrow', () => { describe('marketSellOrdersNoThrow', () => {
const reentrancyTest = (functionNames: string[]) => {
_.forEach(functionNames, async (functionName: string, functionId: number) => {
const description = `should not allow marketSellOrdersNoThrow to reenter the Exchange contract via ${functionName}`;
it(description, async () => {
const signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address),
});
await web3Wrapper.awaitTransactionSuccessAsync(
await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await exchangeWrapper.marketSellOrdersNoThrowAsync([signedOrder], takerAddress, {
takerAssetFillAmount: signedOrder.takerAssetAmount,
});
const newBalances = await erc20Wrapper.getBalancesAsync();
expect(erc20Balances).to.deep.equal(newBalances);
});
});
};
describe('marketSellOrdersNoThrow reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX));
it('should stop when the entire takerAssetFillAmount is filled', async () => { it('should stop when the entire takerAssetFillAmount is filled', async () => {
const takerAssetFillAmount = signedOrders[0].takerAssetAmount.plus( const takerAssetFillAmount = signedOrders[0].takerAssetAmount.plus(
signedOrders[1].takerAssetAmount.div(2), signedOrders[1].takerAssetAmount.div(2),
@ -843,6 +993,28 @@ describe('Exchange wrappers', () => {
}); });
describe('marketBuyOrders', () => { describe('marketBuyOrders', () => {
const reentrancyTest = (functionNames: string[]) => {
_.forEach(functionNames, async (functionName: string, functionId: number) => {
const description = `should not allow marketBuyOrders to reenter the Exchange contract via ${functionName}`;
it(description, async () => {
const signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address),
});
await web3Wrapper.awaitTransactionSuccessAsync(
await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await expectTransactionFailedAsync(
exchangeWrapper.marketBuyOrdersAsync([signedOrder], takerAddress, {
makerAssetFillAmount: signedOrder.makerAssetAmount,
}),
RevertReason.TransferFailed,
);
});
});
};
describe('marketBuyOrders reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX));
it('should stop when the entire makerAssetFillAmount is filled', async () => { it('should stop when the entire makerAssetFillAmount is filled', async () => {
const makerAssetFillAmount = signedOrders[0].makerAssetAmount.plus( const makerAssetFillAmount = signedOrders[0].makerAssetAmount.plus(
signedOrders[1].makerAssetAmount.div(2), signedOrders[1].makerAssetAmount.div(2),
@ -933,6 +1105,27 @@ describe('Exchange wrappers', () => {
}); });
describe('marketBuyOrdersNoThrow', () => { describe('marketBuyOrdersNoThrow', () => {
const reentrancyTest = (functionNames: string[]) => {
_.forEach(functionNames, async (functionName: string, functionId: number) => {
const description = `should not allow marketBuyOrdersNoThrow to reenter the Exchange contract via ${functionName}`;
it(description, async () => {
const signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetData: assetDataUtils.encodeERC20AssetData(reentrantErc20Token.address),
});
await web3Wrapper.awaitTransactionSuccessAsync(
await reentrantErc20Token.setCurrentFunction.sendTransactionAsync(functionId),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await exchangeWrapper.marketBuyOrdersNoThrowAsync([signedOrder], takerAddress, {
makerAssetFillAmount: signedOrder.makerAssetAmount,
});
const newBalances = await erc20Wrapper.getBalancesAsync();
expect(erc20Balances).to.deep.equal(newBalances);
});
});
};
describe('marketBuyOrdersNoThrow reentrancy tests', () => reentrancyTest(constants.FUNCTIONS_WITH_MUTEX));
it('should stop when the entire makerAssetFillAmount is filled', async () => { it('should stop when the entire makerAssetFillAmount is filled', async () => {
const makerAssetFillAmount = signedOrders[0].makerAssetAmount.plus( const makerAssetFillAmount = signedOrders[0].makerAssetAmount.plus(
signedOrders[1].makerAssetAmount.div(2), signedOrders[1].makerAssetAmount.div(2),

2
packages/contracts/test/utils/artifacts.ts
View File

@ -16,6 +16,7 @@ import * as MixinAuthorizable from '../../artifacts/MixinAuthorizable.json';
import * as MultiSigWallet from '../../artifacts/MultiSigWallet.json'; import * as MultiSigWallet from '../../artifacts/MultiSigWallet.json';
import * as MultiSigWalletWithTimeLock from '../../artifacts/MultiSigWalletWithTimeLock.json'; import * as MultiSigWalletWithTimeLock from '../../artifacts/MultiSigWalletWithTimeLock.json';
import * as OrderValidator from '../../artifacts/OrderValidator.json'; import * as OrderValidator from '../../artifacts/OrderValidator.json';
import * as ReentrantERC20Token from '../../artifacts/ReentrantERC20Token.json';
import * as TestAssetProxyDispatcher from '../../artifacts/TestAssetProxyDispatcher.json'; import * as TestAssetProxyDispatcher from '../../artifacts/TestAssetProxyDispatcher.json';
import * as TestAssetProxyOwner from '../../artifacts/TestAssetProxyOwner.json'; import * as TestAssetProxyOwner from '../../artifacts/TestAssetProxyOwner.json';
import * as TestConstants from '../../artifacts/TestConstants.json'; import * as TestConstants from '../../artifacts/TestConstants.json';
@ -49,6 +50,7 @@ export const artifacts = {
MultiSigWallet: (MultiSigWallet as any) as ContractArtifact, MultiSigWallet: (MultiSigWallet as any) as ContractArtifact,
MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact, MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact,
OrderValidator: (OrderValidator as any) as ContractArtifact, OrderValidator: (OrderValidator as any) as ContractArtifact,
ReentrantERC20Token: (ReentrantERC20Token as any) as ContractArtifact,
TestAssetProxyOwner: (TestAssetProxyOwner as any) as ContractArtifact, TestAssetProxyOwner: (TestAssetProxyOwner as any) as ContractArtifact,
TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact, TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact,
TestConstants: (TestConstants as any) as ContractArtifact, TestConstants: (TestConstants as any) as ContractArtifact,

12
packages/contracts/test/utils/constants.ts
View File

@ -51,4 +51,16 @@ export const constants = {
WORD_LENGTH: 32, WORD_LENGTH: 32,
ZERO_AMOUNT: new BigNumber(0), ZERO_AMOUNT: new BigNumber(0),
PERCENTAGE_DENOMINATOR: new BigNumber(10).pow(18), PERCENTAGE_DENOMINATOR: new BigNumber(10).pow(18),
FUNCTIONS_WITH_MUTEX: [
'FILL_ORDER',
'FILL_OR_KILL_ORDER',
'BATCH_FILL_ORDERS',
'BATCH_FILL_OR_KILL_ORDERS',
'MARKET_BUY_ORDERS',
'MARKET_SELL_ORDERS',
'MATCH_ORDERS',
'CANCEL_ORDER',
'CANCEL_ORDERS_UP_TO',
'SET_SIGNATURE_VALIDATOR_APPROVAL',
],
}; };

12
packages/contracts/test/utils/fill_order_combinatorial_utils.ts
View File

@ -467,17 +467,17 @@ export class FillOrderCombinatorialUtils {
? remainingTakerAmountToFill ? remainingTakerAmountToFill
: alreadyFilledTakerAmount.add(takerAssetFillAmount); : alreadyFilledTakerAmount.add(takerAssetFillAmount);
const expFilledMakerAmount = orderUtils.getPartialAmount( const expFilledMakerAmount = orderUtils.getPartialAmountFloor(
expFilledTakerAmount, expFilledTakerAmount,
signedOrder.takerAssetAmount, signedOrder.takerAssetAmount,
signedOrder.makerAssetAmount, signedOrder.makerAssetAmount,
); );
const expMakerFeePaid = orderUtils.getPartialAmount( const expMakerFeePaid = orderUtils.getPartialAmountFloor(
expFilledTakerAmount, expFilledTakerAmount,
signedOrder.takerAssetAmount, signedOrder.takerAssetAmount,
signedOrder.makerFee, signedOrder.makerFee,
); );
const expTakerFeePaid = orderUtils.getPartialAmount( const expTakerFeePaid = orderUtils.getPartialAmountFloor(
expFilledTakerAmount, expFilledTakerAmount,
signedOrder.takerAssetAmount, signedOrder.takerAssetAmount,
signedOrder.takerFee, signedOrder.takerFee,
@ -668,7 +668,7 @@ export class FillOrderCombinatorialUtils {
signedOrder: SignedOrder, signedOrder: SignedOrder,
takerAssetFillAmount: BigNumber, takerAssetFillAmount: BigNumber,
): Promise<void> { ): Promise<void> {
const makerAssetFillAmount = orderUtils.getPartialAmount( const makerAssetFillAmount = orderUtils.getPartialAmountFloor(
takerAssetFillAmount, takerAssetFillAmount,
signedOrder.takerAssetAmount, signedOrder.takerAssetAmount,
signedOrder.makerAssetAmount, signedOrder.makerAssetAmount,
@ -705,7 +705,7 @@ export class FillOrderCombinatorialUtils {
); );
} }
const makerFee = orderUtils.getPartialAmount( const makerFee = orderUtils.getPartialAmountFloor(
takerAssetFillAmount, takerAssetFillAmount,
signedOrder.takerAssetAmount, signedOrder.takerAssetAmount,
signedOrder.makerFee, signedOrder.makerFee,
@ -829,7 +829,7 @@ export class FillOrderCombinatorialUtils {
); );
} }
const takerFee = orderUtils.getPartialAmount( const takerFee = orderUtils.getPartialAmountFloor(
takerAssetFillAmount, takerAssetFillAmount,
signedOrder.takerAssetAmount, signedOrder.takerAssetAmount,
signedOrder.takerFee, signedOrder.takerFee,

2
packages/contracts/test/utils/order_utils.ts
View File

@ -5,7 +5,7 @@ import { constants } from './constants';
import { CancelOrder, MatchOrder } from './types'; import { CancelOrder, MatchOrder } from './types';
export const orderUtils = { export const orderUtils = {
getPartialAmount(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber { getPartialAmountFloor(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber {
const partialAmount = numerator const partialAmount = numerator
.mul(target) .mul(target)
.div(denominator) .div(denominator)

4
packages/order-utils/src/order_state_utils.ts
View File

@ -81,7 +81,7 @@ export class OrderStateUtils {
const remainingTakerAssetAmount = signedOrder.takerAssetAmount.minus( const remainingTakerAssetAmount = signedOrder.takerAssetAmount.minus(
sidedOrderRelevantState.filledTakerAssetAmount, sidedOrderRelevantState.filledTakerAssetAmount,
); );
const isRoundingError = OrderValidationUtils.isRoundingError( const isRoundingError = OrderValidationUtils.isRoundingErrorFloor(
remainingTakerAssetAmount, remainingTakerAssetAmount,
signedOrder.takerAssetAmount, signedOrder.takerAssetAmount,
signedOrder.makerAssetAmount, signedOrder.makerAssetAmount,
@ -191,7 +191,7 @@ export class OrderStateUtils {
); );
const remainingFillableTakerAssetAmountGivenMakersStatus = signedOrder.makerAssetAmount.eq(0) const remainingFillableTakerAssetAmountGivenMakersStatus = signedOrder.makerAssetAmount.eq(0)
? new BigNumber(0) ? new BigNumber(0)
: utils.getPartialAmount( : utils.getPartialAmountFloor(
orderRelevantMakerState.remainingFillableAssetAmount, orderRelevantMakerState.remainingFillableAssetAmount,
signedOrder.makerAssetAmount, signedOrder.makerAssetAmount,
signedOrder.takerAssetAmount, signedOrder.takerAssetAmount,

10
packages/order-utils/src/order_validation_utils.ts
View File

@ -24,7 +24,7 @@ export class OrderValidationUtils {
* @param denominator Denominator value. When used to check an order, pass in `order.takerAssetAmount` * @param denominator Denominator value. When used to check an order, pass in `order.takerAssetAmount`
* @param target Target value. When used to check an order, pass in `order.makerAssetAmount` * @param target Target value. When used to check an order, pass in `order.makerAssetAmount`
*/ */
public static isRoundingError(numerator: BigNumber, denominator: BigNumber, target: BigNumber): boolean { public static isRoundingErrorFloor(numerator: BigNumber, denominator: BigNumber, target: BigNumber): boolean {
// Solidity's mulmod() in JS // Solidity's mulmod() in JS
// Source: https://solidity.readthedocs.io/en/latest/units-and-global-variables.html#mathematical-and-cryptographic-functions // Source: https://solidity.readthedocs.io/en/latest/units-and-global-variables.html#mathematical-and-cryptographic-functions
if (denominator.eq(0)) { if (denominator.eq(0)) {
@ -58,7 +58,7 @@ export class OrderValidationUtils {
zrxAssetData: string, zrxAssetData: string,
): Promise<void> { ): Promise<void> {
try { try {
const fillMakerTokenAmount = utils.getPartialAmount( const fillMakerTokenAmount = utils.getPartialAmountFloor(
fillTakerAssetAmount, fillTakerAssetAmount,
signedOrder.takerAssetAmount, signedOrder.takerAssetAmount,
signedOrder.makerAssetAmount, signedOrder.makerAssetAmount,
@ -79,7 +79,7 @@ export class OrderValidationUtils {
TradeSide.Taker, TradeSide.Taker,
TransferType.Trade, TransferType.Trade,
); );
const makerFeeAmount = utils.getPartialAmount( const makerFeeAmount = utils.getPartialAmountFloor(
fillTakerAssetAmount, fillTakerAssetAmount,
signedOrder.takerAssetAmount, signedOrder.takerAssetAmount,
signedOrder.makerFee, signedOrder.makerFee,
@ -92,7 +92,7 @@ export class OrderValidationUtils {
TradeSide.Maker, TradeSide.Maker,
TransferType.Fee, TransferType.Fee,
); );
const takerFeeAmount = utils.getPartialAmount( const takerFeeAmount = utils.getPartialAmountFloor(
fillTakerAssetAmount, fillTakerAssetAmount,
signedOrder.takerAssetAmount, signedOrder.takerAssetAmount,
signedOrder.takerFee, signedOrder.takerFee,
@ -218,7 +218,7 @@ export class OrderValidationUtils {
zrxAssetData, zrxAssetData,
); );
const wouldRoundingErrorOccur = OrderValidationUtils.isRoundingError( const wouldRoundingErrorOccur = OrderValidationUtils.isRoundingErrorFloor(
desiredFillTakerTokenAmount, desiredFillTakerTokenAmount,
signedOrder.takerAssetAmount, signedOrder.takerAssetAmount,
signedOrder.makerAssetAmount, signedOrder.makerAssetAmount,

2
packages/order-utils/src/utils.ts
View File

@ -12,7 +12,7 @@ export const utils = {
const milisecondsInSecond = 1000; const milisecondsInSecond = 1000;
return new BigNumber(Date.now() / milisecondsInSecond).round(); return new BigNumber(Date.now() / milisecondsInSecond).round();
}, },
getPartialAmount(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber { getPartialAmountFloor(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber {
const fillMakerTokenAmount = numerator const fillMakerTokenAmount = numerator
.mul(target) .mul(target)
.div(denominator) .div(denominator)

12
packages/order-utils/test/order_validation_utils_test.ts
View File

@ -16,7 +16,7 @@ describe('OrderValidationUtils', () => {
const denominator = new BigNumber(999); const denominator = new BigNumber(999);
const target = new BigNumber(50); const target = new BigNumber(50);
// rounding error = ((20*50/999) - floor(20*50/999)) / (20*50/999) = 0.1% // rounding error = ((20*50/999) - floor(20*50/999)) / (20*50/999) = 0.1%
const isRoundingError = OrderValidationUtils.isRoundingError(numerator, denominator, target); const isRoundingError = OrderValidationUtils.isRoundingErrorFloor(numerator, denominator, target);
expect(isRoundingError).to.be.false(); expect(isRoundingError).to.be.false();
}); });
@ -25,7 +25,7 @@ describe('OrderValidationUtils', () => {
const denominator = new BigNumber(9991); const denominator = new BigNumber(9991);
const target = new BigNumber(500); const target = new BigNumber(500);
// rounding error = ((20*500/9991) - floor(20*500/9991)) / (20*500/9991) = 0.09% // rounding error = ((20*500/9991) - floor(20*500/9991)) / (20*500/9991) = 0.09%
const isRoundingError = OrderValidationUtils.isRoundingError(numerator, denominator, target); const isRoundingError = OrderValidationUtils.isRoundingErrorFloor(numerator, denominator, target);
expect(isRoundingError).to.be.false(); expect(isRoundingError).to.be.false();
}); });
@ -34,7 +34,7 @@ describe('OrderValidationUtils', () => {
const denominator = new BigNumber(9989); const denominator = new BigNumber(9989);
const target = new BigNumber(500); const target = new BigNumber(500);
// rounding error = ((20*500/9989) - floor(20*500/9989)) / (20*500/9989) = 0.011% // rounding error = ((20*500/9989) - floor(20*500/9989)) / (20*500/9989) = 0.011%
const isRoundingError = OrderValidationUtils.isRoundingError(numerator, denominator, target); const isRoundingError = OrderValidationUtils.isRoundingErrorFloor(numerator, denominator, target);
expect(isRoundingError).to.be.true(); expect(isRoundingError).to.be.true();
}); });
@ -43,7 +43,7 @@ describe('OrderValidationUtils', () => {
const denominator = new BigNumber(7); const denominator = new BigNumber(7);
const target = new BigNumber(10); const target = new BigNumber(10);
// rounding error = ((3*10/7) - floor(3*10/7)) / (3*10/7) = 6.67% // rounding error = ((3*10/7) - floor(3*10/7)) / (3*10/7) = 6.67%
const isRoundingError = OrderValidationUtils.isRoundingError(numerator, denominator, target); const isRoundingError = OrderValidationUtils.isRoundingErrorFloor(numerator, denominator, target);
expect(isRoundingError).to.be.true(); expect(isRoundingError).to.be.true();
}); });
@ -52,7 +52,7 @@ describe('OrderValidationUtils', () => {
const denominator = new BigNumber(2); const denominator = new BigNumber(2);
const target = new BigNumber(10); const target = new BigNumber(10);
const isRoundingError = OrderValidationUtils.isRoundingError(numerator, denominator, target); const isRoundingError = OrderValidationUtils.isRoundingErrorFloor(numerator, denominator, target);
expect(isRoundingError).to.be.false(); expect(isRoundingError).to.be.false();
}); });
@ -63,7 +63,7 @@ describe('OrderValidationUtils', () => {
const target = new BigNumber(105762562); const target = new BigNumber(105762562);
// rounding error = ((76564*105762562/676373677) - floor(76564*105762562/676373677)) / // rounding error = ((76564*105762562/676373677) - floor(76564*105762562/676373677)) /
// (76564*105762562/676373677) = 0.0007% // (76564*105762562/676373677) = 0.0007%
const isRoundingError = OrderValidationUtils.isRoundingError(numerator, denominator, target); const isRoundingError = OrderValidationUtils.isRoundingErrorFloor(numerator, denominator, target);
expect(isRoundingError).to.be.false(); expect(isRoundingError).to.be.false();
}); });
}); });

1
packages/types/src/index.ts
View File

@ -173,6 +173,7 @@ export enum RevertReason {
InvalidSender = 'INVALID_SENDER', InvalidSender = 'INVALID_SENDER',
InvalidOrderSignature = 'INVALID_ORDER_SIGNATURE', InvalidOrderSignature = 'INVALID_ORDER_SIGNATURE',
InvalidTakerAmount = 'INVALID_TAKER_AMOUNT', InvalidTakerAmount = 'INVALID_TAKER_AMOUNT',
DivisionByZero = 'DIVISION_BY_ZERO',
RoundingError = 'ROUNDING_ERROR', RoundingError = 'ROUNDING_ERROR',
InvalidSignature = 'INVALID_SIGNATURE', InvalidSignature = 'INVALID_SIGNATURE',
SignatureIllegal = 'SIGNATURE_ILLEGAL', SignatureIllegal = 'SIGNATURE_ILLEGAL',