aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAmir Bandeali <abandeali1@gmail.com>2018-08-24 07:43:46 +0800
committerAmir Bandeali <abandeali1@gmail.com>2018-08-25 08:30:56 +0800
commitc28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa (patch)
tree0e25c29cb9d639441d956505f49555f2ed391603
parenta09ee907396d35468e0ad632fb990cb1c8870b49 (diff)
downloaddexon-sol-tools-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.tar
dexon-sol-tools-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.tar.gz
dexon-sol-tools-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.tar.bz2
dexon-sol-tools-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.tar.lz
dexon-sol-tools-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.tar.xz
dexon-sol-tools-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.tar.zst
dexon-sol-tools-c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa.zip
Only use one nonReentrant modifier, remove modifier from fillOrderNoThrow variations
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol2
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol2
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol17
-rw-r--r--packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol33
-rw-r--r--packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol14
-rw-r--r--packages/contracts/test/utils/constants.ts4
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',