diff options
author | Amir Bandeali <abandeali1@gmail.com> | 2018-08-24 07:43:46 +0800 |
---|---|---|
committer | Amir Bandeali <abandeali1@gmail.com> | 2018-08-25 08:30:56 +0800 |
commit | c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa (patch) | |
tree | 0e25c29cb9d639441d956505f49555f2ed391603 /packages/contracts | |
parent | a09ee907396d35468e0ad632fb990cb1c8870b49 (diff) | |
download | dexon-0x-contracts-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.tar dexon-0x-contracts-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.tar.gz dexon-0x-contracts-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.tar.bz2 dexon-0x-contracts-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.tar.lz dexon-0x-contracts-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.tar.xz dexon-0x-contracts-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.tar.zst dexon-0x-contracts-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.zip |
Only use one nonReentrant modifier, remove modifier from fillOrderNoThrow variations
Diffstat (limited to 'packages/contracts')
6 files changed, 11 insertions, 61 deletions
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol index f02514fc6..291af3792 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol @@ -89,7 +89,7 @@ contract MixinExchangeCore is bytes memory signature ) public - lockMutex + nonReentrant returns (FillResults memory fillResults) { fillResults = fillOrderInternal( diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index 39c47e6f9..25220a673 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -50,7 +50,7 @@ contract MixinMatchOrders is bytes memory rightSignature ) public - lockMutex + nonReentrant returns (LibFillResults.MatchedFillResults memory matchedFillResults) { // We assume that rightOrder.takerAssetData == leftOrder.makerAssetData and rightOrder.makerAssetData == leftOrder.takerAssetData. diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol index b0474b110..39fa724cc 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol @@ -33,7 +33,8 @@ contract MixinWrapperFunctions is LibMath, LibFillResults, LibAbiEncoder, - MExchangeCore + MExchangeCore, + MWrapperFunctions { /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled. @@ -46,7 +47,7 @@ contract MixinWrapperFunctions is bytes memory signature ) public - lockMutex + nonReentrant returns (FillResults memory fillResults) { fillResults = fillOrKillOrderInternal( @@ -69,7 +70,6 @@ contract MixinWrapperFunctions is bytes memory signature ) public - nonReentrant returns (FillResults memory fillResults) { // ABI encode calldata for `fillOrder` @@ -111,7 +111,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - lockMutex + nonReentrant returns (FillResults memory totalFillResults) { uint256 ordersLength = orders.length; @@ -138,7 +138,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - lockMutex + nonReentrant returns (FillResults memory totalFillResults) { uint256 ordersLength = orders.length; @@ -166,7 +166,6 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - nonReentrant returns (FillResults memory totalFillResults) { uint256 ordersLength = orders.length; @@ -192,7 +191,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - lockMutex + nonReentrant returns (FillResults memory totalFillResults) { bytes memory takerAssetData = orders[0].takerAssetData; @@ -237,7 +236,6 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - nonReentrant returns (FillResults memory totalFillResults) { bytes memory takerAssetData = orders[0].takerAssetData; @@ -281,7 +279,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - lockMutex + nonReentrant returns (FillResults memory totalFillResults) { bytes memory makerAssetData = orders[0].makerAssetData; @@ -334,7 +332,6 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - nonReentrant returns (FillResults memory totalFillResults) { bytes memory makerAssetData = orders[0].makerAssetData; diff --git a/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol b/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol index 8d575d09a..8bfdd2e66 100644 --- a/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol +++ b/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol @@ -40,17 +40,14 @@ contract ReentrantERC20Token is ); // 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, - FILL_ORDER_NO_THROW, BATCH_FILL_ORDERS, BATCH_FILL_OR_KILL_ORDERS, - BATCH_FILL_ORDERS_NO_THROW, MARKET_BUY_ORDERS, - MARKET_BUY_ORDERS_NO_THROW, MARKET_SELL_ORDERS, - MARKET_SELL_ORDERS_NO_THROW, MATCH_ORDERS, CANCEL_ORDER, CANCEL_ORDERS_UP_TO, @@ -111,13 +108,6 @@ contract ReentrantERC20Token is 0, signature ); - } else if (currentFunctionId == uint8(ExchangeFunction.FILL_ORDER_NO_THROW)) { - calldata = abi.encodeWithSelector( - EXCHANGE.fillOrderNoThrow.selector, - order, - 0, - signature - ); } else if (currentFunctionId == uint8(ExchangeFunction.BATCH_FILL_ORDERS)) { calldata = abi.encodeWithSelector( EXCHANGE.batchFillOrders.selector, @@ -132,13 +122,6 @@ contract ReentrantERC20Token is takerAssetFillAmounts, signatures ); - } else if (currentFunctionId == uint8(ExchangeFunction.BATCH_FILL_ORDERS_NO_THROW)) { - calldata = abi.encodeWithSelector( - EXCHANGE.batchFillOrdersNoThrow.selector, - orders, - takerAssetFillAmounts, - signatures - ); } else if (currentFunctionId == uint8(ExchangeFunction.MARKET_BUY_ORDERS)) { calldata = abi.encodeWithSelector( EXCHANGE.marketBuyOrders.selector, @@ -146,13 +129,6 @@ contract ReentrantERC20Token is 0, signatures ); - } else if (currentFunctionId == uint8(ExchangeFunction.MARKET_BUY_ORDERS_NO_THROW)) { - calldata = abi.encodeWithSelector( - EXCHANGE.marketBuyOrdersNoThrow.selector, - orders, - 0, - signatures - ); } else if (currentFunctionId == uint8(ExchangeFunction.MARKET_SELL_ORDERS)) { calldata = abi.encodeWithSelector( EXCHANGE.marketSellOrders.selector, @@ -160,13 +136,6 @@ contract ReentrantERC20Token is 0, signatures ); - } else if (currentFunctionId == uint8(ExchangeFunction.MARKET_SELL_ORDERS_NO_THROW)) { - calldata = abi.encodeWithSelector( - EXCHANGE.marketSellOrdersNoThrow.selector, - orders, - 0, - signatures - ); } else if (currentFunctionId == uint8(ExchangeFunction.MATCH_ORDERS)) { calldata = abi.encodeWithSelector( EXCHANGE.matchOrders.selector, diff --git a/packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol b/packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol index 2b980c7ca..1dee512d4 100644 --- a/packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol +++ b/packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol @@ -23,21 +23,9 @@ contract ReentrancyGuard { // Locked state of mutex bool private locked = false; - /// @dev Functions with this modifier cannot be reentered. - modifier nonReentrant() { - // Ensure mutex is unlocked - require( - !locked, - "REENTRANCY_ILLEGAL" - ); - - // Perform function call - _; - } - /// @dev Functions with this modifer cannot be reentered. The mutex will be locked /// before function execution and unlocked after. - modifier lockMutex() { + modifier nonReentrant() { // Ensure mutex is unlocked require( !locked, diff --git a/packages/contracts/test/utils/constants.ts b/packages/contracts/test/utils/constants.ts index 7060b5091..ee4378d2e 100644 --- a/packages/contracts/test/utils/constants.ts +++ b/packages/contracts/test/utils/constants.ts @@ -54,14 +54,10 @@ export const constants = { FUNCTIONS_WITH_MUTEX: [ 'FILL_ORDER', 'FILL_OR_KILL_ORDER', - 'FILL_ORDER_NO_THROW', 'BATCH_FILL_ORDERS', 'BATCH_FILL_OR_KILL_ORDERS', - 'BATCH_FILL_ORDERS_NO_THROW', 'MARKET_BUY_ORDERS', - 'MARKET_BUY_ORDERS_NO_THROW', 'MARKET_SELL_ORDERS', - 'MARKET_SELL_ORDERS_NO_THROW', 'MATCH_ORDERS', 'CANCEL_ORDER', 'CANCEL_ORDERS_UP_TO', |