@0x/contracts-zero-ex: Add reentrancy guard to mtx functions

`@0x/contracts-zero-ex`: Add refund mechanism to mtxs
`@0x/contracts-zero-ex`: Pass sender to transfomers.
`@0x/contracts-zero-ex`: Refund protocol fees to `refundReceiver` in FQT.
`@0x/utils`: Add EP flavor of `IllegalReentrancyError`
`@0x/order-utils`: Add `refundReceiver` to FQT transform data.
`@0x/asset-swapper`: Add `refundReceiver` support to EP swap quote consumer.

contracts/zero-ex/CHANGELOG.json

@@ -1,4 +1,25 @@
 [
+    {
+        "version": "0.3.0",
+        "changes": [
+            {
+                "note": "Internal audit fixes",
+                "pr": 2657
+            },
+            {
+                "note": "Add refund mechanism to meta-transactions",
+                "pr": 2657
+            },
+            {
+                "note": "Pass sender address to transformers",
+                "pr": 2657
+            },
+            {
+                "note": "Refund unused protocol fees to `refundReceiver` in FQT",
+                "pr": 2657
+            }
+        ]
+    },
     {
         "version": "0.2.0",
         "changes": [

contracts/zero-ex/contracts/src/errors/LibCommonRichErrors.sol

@@ -34,13 +34,15 @@ library LibCommonRichErrors {
         );
     }
 
-    function IllegalReentrancyError()
+    function IllegalReentrancyError(bytes4 selector, uint256 reentrancyFlags)
         internal
         pure
         returns (bytes memory)
     {
         return abi.encodeWithSelector(
-            bytes4(keccak256("IllegalReentrancyError()"))
+            bytes4(keccak256("IllegalReentrancyError(bytes4,uint256)")),
+            selector,
+            reentrancyFlags
         );
     }
 }

contracts/zero-ex/contracts/src/features/MetaTransactions.sol

@@ -21,8 +21,10 @@ pragma experimental ABIEncoderV2;
 
 import "@0x/contracts-utils/contracts/src/v06/errors/LibRichErrorsV06.sol";
 import "@0x/contracts-utils/contracts/src/v06/LibBytesV06.sol";
+import "@0x/contracts-utils/contracts/src/v06/LibSafeMathV06.sol";
 import "../errors/LibMetaTransactionsRichErrors.sol";
 import "../fixins/FixinCommon.sol";
+import "../fixins/FixinReentrancyGuard.sol";
 import "../fixins/FixinEIP712.sol";
 import "../migrations/LibMigrate.sol";
 import "../storage/LibMetaTransactionsStorage.sol";
@@ -39,6 +41,7 @@ contract MetaTransactions is
     IFeature,
     IMetaTransactions,
     FixinCommon,
+    FixinReentrancyGuard,
     FixinEIP712
 {
     using LibBytesV06 for bytes;
@@ -92,6 +95,16 @@ contract MetaTransactions is
         ")"
     );
 
+    /// @dev Refunds up to `msg.value` leftover ETH at the end of the call.
+    modifier refundsAttachedEth() {
+        _;
+        uint256 remainingBalance =
+            LibSafeMathV06.min256(msg.value, address(this).balance);
+        if (remainingBalance > 0) {
+            msg.sender.transfer(remainingBalance);
+        }
+    }
+
     constructor(address zeroExAddress)
         public
         FixinCommon()
@@ -127,9 +140,11 @@ contract MetaTransactions is
         public
         payable
         override
+        nonReentrant(REENTRANCY_MTX)
+        refundsAttachedEth
         returns (bytes memory returnResult)
     {
-        return _executeMetaTransactionPrivate(
+        returnResult = _executeMetaTransactionPrivate(
             msg.sender,
             mtx,
             signature
@@ -147,6 +162,8 @@ contract MetaTransactions is
         public
         payable
         override
+        nonReentrant(REENTRANCY_MTX)
+        refundsAttachedEth
         returns (bytes[] memory returnResults)
     {
         if (mtxs.length != signatures.length) {
@@ -255,10 +272,19 @@ contract MetaTransactions is
         _validateMetaTransaction(state);
 
         // Mark the transaction executed.
-        assert(block.number > 0);
         LibMetaTransactionsStorage.getStorage()
             .mtxHashToExecutedBlockNumber[state.hash] = block.number;
 
+        // Pay the fee to the sender.
+        if (mtx.feeAmount > 0) {
+            ITokenSpender(address(this))._spendERC20Tokens(
+                mtx.feeToken,
+                mtx.signer, // From the signer.
+                sender, // To the sender.
+                mtx.feeAmount
+            );
+        }
+
         // Execute the call based on the selector.
         state.selector = mtx.callData.readBytes4(0);
         if (state.selector == ITransformERC20.transformERC20.selector) {
@@ -268,15 +294,6 @@ contract MetaTransactions is
                 .MetaTransactionUnsupportedFunctionError(state.hash, state.selector)
                 .rrevert();
         }
-        // Pay the fee to the sender.
-        if (mtx.feeAmount > 0) {
-            ITokenSpender(address(this))._spendERC20Tokens(
-                mtx.feeToken,
-                mtx.signer, // From the signer.
-                sender, // To the sender.
-                mtx.feeAmount
-            );
-        }
         emit MetaTransactionExecuted(
             state.hash,
             state.selector,
@@ -367,7 +384,7 @@ contract MetaTransactions is
         // since decoding a single struct arg consumes far less stack space than
         // decoding multiple struct args.
 
-        // Where the encoding for multiple args (with the seleector ommitted)
+        // Where the encoding for multiple args (with the selector ommitted)
         // would typically look like:
         // | argument                 |  offset |
         // |--------------------------|---------|
@@ -394,7 +411,7 @@ contract MetaTransactions is
             bytes memory encodedStructArgs = new bytes(state.mtx.callData.length - 4 + 32);
             // Copy the args data from the original, after the new struct offset prefix.
             bytes memory fromCallData = state.mtx.callData;
-            assert(fromCallData.length >= 4);
+            assert(fromCallData.length >= 160);
             uint256 fromMem;
             uint256 toMem;
             assembly {

contracts/zero-ex/contracts/src/features/SignatureValidator.sol

@@ -37,6 +37,13 @@ contract SignatureValidator is
     using LibBytesV06 for bytes;
     using LibRichErrorsV06 for bytes;
 
+    /// @dev Exclusive upper limit on ECDSA signatures 'R' values.
+    ///      The valid range is given by fig (282) of the yellow paper.
+    uint256 private constant ECDSA_SIGNATURE_R_LIMIT =
+        uint256(0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141);
+    /// @dev Exclusive upper limit on ECDSA signatures 'S' values.
+    ///      The valid range is given by fig (283) of the yellow paper.
+    uint256 private constant ECDSA_SIGNATURE_S_LIMIT = ECDSA_SIGNATURE_R_LIMIT / 2 + 1;
     /// @dev Name of this feature.
     string public constant override FEATURE_NAME = "SignatureValidator";
     /// @dev Version of this feature.
@@ -160,12 +167,18 @@ contract SignatureValidator is
             uint8 v = uint8(signature[0]);
             bytes32 r = signature.readBytes32(1);
             bytes32 s = signature.readBytes32(33);
-            recovered = ecrecover(
+            if (v < 27) {
-                hash,
+                // Handle clients that encode v as 0 or 1.
-                v,
+                v += 27;
-                r,
+            }
-                s
+            if (uint256(r) < ECDSA_SIGNATURE_R_LIMIT && uint256(s) < ECDSA_SIGNATURE_S_LIMIT) {
-            );
+                recovered = ecrecover(
+                    hash,
+                    v,
+                    r,
+                    s
+                );
+            }
         } else if (signatureType == SignatureType.EthSign) {
             // Signed using `eth_sign`
             if (signature.length != 66) {
@@ -179,15 +192,21 @@ contract SignatureValidator is
             uint8 v = uint8(signature[0]);
             bytes32 r = signature.readBytes32(1);
             bytes32 s = signature.readBytes32(33);
-            recovered = ecrecover(
+            if (v < 27) {
-                keccak256(abi.encodePacked(
+                // Handle clients that encode v as 0 or 1.
-                    "\x19Ethereum Signed Message:\n32",
+                v += 27;
-                    hash
+            }
-                )),
+            if (uint256(r) < ECDSA_SIGNATURE_R_LIMIT && uint256(s) < ECDSA_SIGNATURE_S_LIMIT) {
-                v,
+                recovered = ecrecover(
-                r,
+                    keccak256(abi.encodePacked(
-                s
+                        "\x19Ethereum Signed Message:\n32",
-            );
+                        hash
+                    )),
+                    v,
+                    r,
+                    s
+                );
+            }
         } else {
             // This should never happen.
             revert('SignatureValidator/ILLEGAL_CODE_PATH');

contracts/zero-ex/contracts/src/features/TransformERC20.sol

@@ -360,9 +360,12 @@ contract TransformERC20 is
             // Call data.
             abi.encodeWithSelector(
                 IERC20Transformer.transform.selector,
-                callDataHash,
+                IERC20Transformer.TransformContext({
-                taker,
+                    callDataHash: callDataHash,
-                transformation.data
+                    sender: msg.sender,
+                    taker: taker,
+                    data: transformation.data
+                })
             )
         );
         // Ensure the transformer returned the magic bytes.

contracts/zero-ex/contracts/src/fixins/FixinReentrancyGuard.sol

@@ -0,0 +1,60 @@
+/*
+
+  Copyright 2020 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.6.5;
+pragma experimental ABIEncoderV2;
+
+import "@0x/contracts-utils/contracts/src/v06/LibBytesV06.sol";
+import "@0x/contracts-utils/contracts/src/v06/errors/LibRichErrorsV06.sol";
+import "../errors/LibCommonRichErrors.sol";
+import "../storage/LibReentrancyGuardStorage.sol";
+
+
+/// @dev Common feature utilities.
+abstract contract FixinReentrancyGuard {
+
+    using LibRichErrorsV06 for bytes;
+    using LibBytesV06 for bytes;
+
+    // Combinable reentrancy flags.
+    /// @dev Reentrancy guard flag for meta-transaction functions.
+    uint256 constant internal REENTRANCY_MTX = 0x1;
+
+    /// @dev Cannot reenter a function with the same reentrancy guard flags.
+    modifier nonReentrant(uint256 reentrancyFlags) virtual {
+        LibReentrancyGuardStorage.Storage storage stor =
+            LibReentrancyGuardStorage.getStorage();
+        {
+            uint256 currentFlags = stor.reentrancyFlags;
+            // Revert if any bits in `reentrancyFlags` has already been set.
+            if ((currentFlags & reentrancyFlags) != 0) {
+                LibCommonRichErrors.IllegalReentrancyError(
+                    msg.data.readBytes4(0),
+                    reentrancyFlags
+                ).rrevert();
+            }
+            // Update reentrancy flags.
+            stor.reentrancyFlags = currentFlags | reentrancyFlags;
+        }
+
+        _;
+
+        // Clear reentrancy flags.
+        stor.reentrancyFlags = stor.reentrancyFlags & (~reentrancyFlags);
+    }
+}

contracts/zero-ex/contracts/src/storage/LibReentrancyGuardStorage.sol

@@ -0,0 +1,46 @@
+/*
+
+  Copyright 2020 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.6.5;
+pragma experimental ABIEncoderV2;
+
+import "./LibStorage.sol";
+import "../external/IFlashWallet.sol";
+
+
+/// @dev Storage helpers for the `FixinReentrancyGuard` mixin.
+library LibReentrancyGuardStorage {
+
+    /// @dev Storage bucket for this feature.
+    struct Storage {
+        // Reentrancy flags set whenever a non-reentrant function is entered
+        // and cleared when it is exited.
+        uint256 reentrancyFlags;
+    }
+
+    /// @dev Get the storage bucket for this contract.
+    function getStorage() internal pure returns (Storage storage stor) {
+        uint256 storageSlot = LibStorage.getStorageSlot(
+            LibStorage.StorageId.ReentrancyGuard
+        );
+        // Dip into assembly to change the slot pointed to by the local
+        // variable `stor`.
+        // See https://solidity.readthedocs.io/en/v0.6.8/assembly.html?highlight=slot#access-to-external-variables-functions-and-libraries
+        assembly { stor_slot := storageSlot }
+    }
+}

contracts/zero-ex/contracts/src/storage/LibStorage.sol

@@ -35,7 +35,8 @@ library LibStorage {
         Ownable,
         TokenSpender,
         TransformERC20,
-        MetaTransactions
+        MetaTransactions,
+        ReentrancyGuard
     }
 
     /// @dev Get the storage slot given a storage ID. We assign unique, well-spaced

contracts/zero-ex/contracts/src/transformers/AffiliateFeeTransformer.sol

@@ -58,18 +58,14 @@ contract AffiliateFeeTransformer is
     {}
 
     /// @dev Transfers tokens to recipients.
-    /// @param data ABI-encoded `TokenFee[]`, indicating which tokens to transfer.
+    /// @param context Context information.
     /// @return success The success bytes (`LibERC20Transformer.TRANSFORMER_SUCCESS`).
-    function transform(
+    function transform(TransformContext calldata context)
-        bytes32, // callDataHash,
-        address payable, // taker,
-        bytes calldata data
-    )
         external
         override
         returns (bytes4 success)
     {
-        TokenFee[] memory fees = abi.decode(data, (TokenFee[]));
+        TokenFee[] memory fees = abi.decode(context.data, (TokenFee[]));
 
         // Transfer tokens to recipients.
         for (uint256 i = 0; i < fees.length; ++i) {

contracts/zero-ex/contracts/src/transformers/FillQuoteTransformer.sol

@@ -71,6 +71,12 @@ contract FillQuoteTransformer is
         // For sells, this may be `uint256(-1)` to sell the entire balance of
         // `sellToken`.
         uint256 fillAmount;
+        // Who to transfer unused protocol fees to.
+        // May be a valid address or one of:
+        // `address(0)`: Stay in flash wallet.
+        // `address(1)`: Send to the taker.
+        // `address(2)`: Send to the sender (caller of `transformERC20()`).
+        address payable refundReceiver;
     }
 
     /// @dev Results of a call to `_fillOrder()`.
@@ -108,6 +114,12 @@ contract FillQuoteTransformer is
     bytes4 private constant ERC20_BRIDGE_PROXY_ID = 0xdc1600f3;
     /// @dev Maximum uint256 value.
     uint256 private constant MAX_UINT256 = uint256(-1);
+    /// @dev If `refundReceiver` is set to this address, unpsent
+    ///      protocol fees will be sent to the taker.
+    address private constant REFUND_RECEIVER_TAKER = address(1);
+    /// @dev If `refundReceiver` is set to this address, unpsent
+    ///      protocol fees will be sent to the sender.
+    address private constant REFUND_RECEIVER_SENDER = address(2);
 
     /// @dev The Exchange contract.
     IExchange public immutable exchange;
@@ -130,31 +142,27 @@ contract FillQuoteTransformer is
     /// @dev Sell this contract's entire balance of of `sellToken` in exchange
     ///      for `buyToken` by filling `orders`. Protocol fees should be attached
     ///      to this call. `buyToken` and excess ETH will be transferred back to the caller.
-    /// @param data_ ABI-encoded `TransformData`.
+    /// @param context Context information.
     /// @return success The success bytes (`LibERC20Transformer.TRANSFORMER_SUCCESS`).
-    function transform(
+    function transform(TransformContext calldata context)
-        bytes32, // callDataHash,
-        address payable, // taker,
-        bytes calldata data_
-    )
         external
         override
         returns (bytes4 success)
     {
-        TransformData memory data = abi.decode(data_, (TransformData));
+        TransformData memory data = abi.decode(context.data, (TransformData));
         FillState memory state;
 
         // Validate data fields.
         if (data.sellToken.isTokenETH() || data.buyToken.isTokenETH()) {
             LibTransformERC20RichErrors.InvalidTransformDataError(
                 LibTransformERC20RichErrors.InvalidTransformDataErrorCode.INVALID_TOKENS,
-                data_
+                context.data
             ).rrevert();
         }
         if (data.orders.length != data.signatures.length) {
             LibTransformERC20RichErrors.InvalidTransformDataError(
                 LibTransformERC20RichErrors.InvalidTransformDataErrorCode.INVALID_ARRAY_LENGTH,
-                data_
+                context.data
             ).rrevert();
         }
 
@@ -248,6 +256,17 @@ contract FillQuoteTransformer is
                     ).rrevert();
             }
         }
+
+        // Refund unspent protocol fees.
+        if (state.ethRemaining > 0 && data.refundReceiver != address(0)) {
+            if (data.refundReceiver == REFUND_RECEIVER_TAKER) {
+                context.taker.transfer(state.ethRemaining);
+            } else if (data.refundReceiver == REFUND_RECEIVER_SENDER) {
+                context.sender.transfer(state.ethRemaining);
+            } else {
+                data.refundReceiver.transfer(state.ethRemaining);
+            }
+        }
         return LibERC20Transformer.TRANSFORMER_SUCCESS;
     }
 

contracts/zero-ex/contracts/src/transformers/IERC20Transformer.sol

@@ -25,17 +25,24 @@ import "@0x/contracts-erc20/contracts/src/v06/IERC20TokenV06.sol";
 /// @dev A transformation callback used in `TransformERC20.transformERC20()`.
 interface IERC20Transformer {
 
+    /// @dev Context information to pass into `transform()` by `TransformERC20.transformERC20()`.
+    struct TransformContext {
+        // The hash of the `TransformERC20.transformERC20()` calldata.
+        bytes32 callDataHash;
+        // The caller of `TransformERC20.transformERC20()`.
+        address payable sender;
+        // taker The taker address, which may be distinct from `sender` in the case
+        // meta-transactions.
+        address payable taker;
+        // Arbitrary data to pass to the transformer.
+        bytes data;
+    }
+
     /// @dev Called from `TransformERC20.transformERC20()`. This will be
     ///      delegatecalled in the context of the FlashWallet instance being used.
-    /// @param callDataHash The hash of the `TransformERC20.transformERC20()` calldata.
+    /// @param context Context information.
-    /// @param taker The taker address (caller of `TransformERC20.transformERC20()`).
-    /// @param data Arbitrary data to pass to the transformer.
     /// @return success The success bytes (`LibERC20Transformer.TRANSFORMER_SUCCESS`).
-    function transform(
+    function transform(TransformContext calldata context)
-        bytes32 callDataHash,
-        address payable taker,
-        bytes calldata data
-    )
         external
         returns (bytes4 success);
 }

contracts/zero-ex/contracts/src/transformers/PayTakerTransformer.sol

@@ -56,19 +56,14 @@ contract PayTakerTransformer is
     {}
 
     /// @dev Forwards tokens to the taker.
-    /// @param taker The taker address (caller of `TransformERC20.transformERC20()`).
+    /// @param context Context information.
-    /// @param data_ ABI-encoded `TransformData`, indicating which tokens to transfer.
     /// @return success The success bytes (`LibERC20Transformer.TRANSFORMER_SUCCESS`).
-    function transform(
+    function transform(TransformContext calldata context)
-        bytes32, // callDataHash,
-        address payable taker,
-        bytes calldata data_
-    )
         external
         override
         returns (bytes4 success)
     {
-        TransformData memory data = abi.decode(data_, (TransformData));
+        TransformData memory data = abi.decode(context.data, (TransformData));
 
         // Transfer tokens directly to the taker.
         for (uint256 i = 0; i < data.tokens.length; ++i) {
@@ -79,7 +74,7 @@ contract PayTakerTransformer is
                 amount = data.tokens[i].getTokenBalanceOf(address(this));
             }
             if (amount != 0) {
-                data.tokens[i].transformerTransfer(taker, amount);
+                data.tokens[i].transformerTransfer(context.taker, amount);
             }
         }
         return LibERC20Transformer.TRANSFORMER_SUCCESS;

contracts/zero-ex/contracts/src/transformers/WethTransformer.sol

@@ -59,22 +59,18 @@ contract WethTransformer is
     }
 
     /// @dev Wraps and unwraps WETH.
-    /// @param data_ ABI-encoded `TransformData`, indicating which token to wrap/umwrap.
+    /// @param context Context information.
     /// @return success The success bytes (`LibERC20Transformer.TRANSFORMER_SUCCESS`).
-    function transform(
+    function transform(TransformContext calldata context)
-        bytes32, // callDataHash,
-        address payable, // taker,
-        bytes calldata data_
-    )
         external
         override
         returns (bytes4 success)
     {
-        TransformData memory data = abi.decode(data_, (TransformData));
+        TransformData memory data = abi.decode(context.data, (TransformData));
         if (!data.token.isTokenETH() && data.token != weth) {
             LibTransformERC20RichErrors.InvalidTransformDataError(
                 LibTransformERC20RichErrors.InvalidTransformDataErrorCode.INVALID_TOKENS,
-                data_
+                context.data
             ).rrevert();
         }
 

contracts/zero-ex/contracts/test/TestFillQuoteTransformerHost.sol

@@ -31,6 +31,8 @@ contract TestFillQuoteTransformerHost is
         IERC20Transformer transformer,
         TestMintableERC20Token inputToken,
         uint256 inputTokenAmount,
+        address payable sender,
+        address payable taker,
         bytes calldata data
     )
         external
@@ -40,6 +42,14 @@ contract TestFillQuoteTransformerHost is
             inputToken.mint(address(this), inputTokenAmount);
         }
         // Have to make this call externally because transformers aren't payable.
-        this.rawExecuteTransform(transformer, bytes32(0), msg.sender, data);
+        this.rawExecuteTransform(
+            transformer,
+            IERC20Transformer.TransformContext({
+                callDataHash: bytes32(0),
+                sender: sender,
+                taker: taker,
+                data: data
+            })
+        );
     }
 }

contracts/zero-ex/contracts/test/TestMetaTransactionsTransformERC20Feature.sol

@@ -20,6 +20,7 @@ pragma solidity ^0.6.5;
 pragma experimental ABIEncoderV2;
 
 import "../src/features/TransformERC20.sol";
+import "../src/features/IMetaTransactions.sol";
 
 
 contract TestMetaTransactionsTransformERC20Feature is
@@ -48,6 +49,49 @@ contract TestMetaTransactionsTransformERC20Feature is
             revert('FAIL');
         }
 
+        if (msg.value == 777) {
+            // Try to reenter `executeMetaTransaction()`
+            IMetaTransactions(address(this)).executeMetaTransaction(
+                IMetaTransactions.MetaTransactionData({
+                    signer: address(0),
+                    sender: address(0),
+                    minGasPrice: 0,
+                    maxGasPrice: 0,
+                    expirationTimeSeconds: 0,
+                    salt: 0,
+                    callData: "",
+                    value: 0,
+                    feeToken: IERC20TokenV06(0),
+                    feeAmount: 0
+                }),
+                ""
+            );
+        }
+
+        if (msg.value == 888) {
+            // Try to reenter `batchExecuteMetaTransactions()`
+            IMetaTransactions.MetaTransactionData[] memory mtxs =
+                new IMetaTransactions.MetaTransactionData[](1);
+            bytes[] memory signatures = new bytes[](1);
+            mtxs[0] = IMetaTransactions.MetaTransactionData({
+                signer: address(0),
+                sender: address(0),
+                minGasPrice: 0,
+                maxGasPrice: 0,
+                expirationTimeSeconds: 0,
+                salt: 0,
+                callData: "",
+                value: 0,
+                feeToken: IERC20TokenV06(0),
+                feeAmount: 0
+            });
+            signatures[0] = "";
+            IMetaTransactions(address(this)).batchExecuteMetaTransactions(
+                mtxs,
+                signatures
+            );
+        }
+
         emit TransformERC20Called(
             msg.sender,
             msg.value,

contracts/zero-ex/contracts/test/TestMintTokenERC20Transformer.sol

@@ -40,28 +40,26 @@ contract TestMintTokenERC20Transformer is
         address context,
         address caller,
         bytes32 callDataHash,
+        address sender,
         address taker,
         bytes data,
         uint256 inputTokenBalance,
         uint256 ethBalance
     );
 
-    function transform(
+    function transform(TransformContext calldata context)
-        bytes32 callDataHash,
-        address payable taker,
-        bytes calldata data_
-    )
         external
         override
         returns (bytes4 success)
     {
-        TransformData memory data = abi.decode(data_, (TransformData));
+        TransformData memory data = abi.decode(context.data, (TransformData));
         emit MintTransform(
             address(this),
             msg.sender,
-            callDataHash,
+            context.callDataHash,
-            taker,
+            context.sender,
-            data_,
+            context.taker,
+            context.data,
             data.inputToken.balanceOf(address(this)),
             address(this).balance
         );
@@ -69,14 +67,14 @@ contract TestMintTokenERC20Transformer is
         data.inputToken.transfer(address(0), data.burnAmount);
         // Mint output tokens.
         if (LibERC20Transformer.isTokenETH(IERC20TokenV06(address(data.outputToken)))) {
-            taker.transfer(data.mintAmount);
+            context.taker.transfer(data.mintAmount);
         } else {
             data.outputToken.mint(
-                taker,
+                context.taker,
                 data.mintAmount
             );
             // Burn fees from output.
-            data.outputToken.burn(taker, data.feeAmount);
+            data.outputToken.burn(context.taker, data.feeAmount);
         }
         return LibERC20Transformer.TRANSFORMER_SUCCESS;
     }

contracts/zero-ex/contracts/test/TestTransformerBase.sol

@@ -20,17 +20,15 @@ pragma solidity ^0.6.5;
 pragma experimental ABIEncoderV2;
 
 import "../src/transformers/Transformer.sol";
+import "../src/transformers/IERC20Transformer.sol";
 import "../src/transformers/LibERC20Transformer.sol";
 
 
 contract TestTransformerBase is
+    IERC20Transformer,
     Transformer
 {
-    function transform(
+    function transform(TransformContext calldata context)
-        bytes32,
-        address payable,
-        bytes calldata
-    )
         external
         override
         returns (bytes4 success)

contracts/zero-ex/contracts/test/TestTransformerHost.sol

@@ -32,18 +32,14 @@ contract TestTransformerHost {
 
     function rawExecuteTransform(
         IERC20Transformer transformer,
-        bytes32 callDataHash,
+        IERC20Transformer.TransformContext calldata context
-        address taker,
-        bytes calldata data
     )
         external
     {
         (bool _success, bytes memory resultData) =
             address(transformer).delegatecall(abi.encodeWithSelector(
                 transformer.transform.selector,
-                callDataHash,
+                context
-                taker,
-                data
             ));
         if (!_success) {
             resultData.rrevert();

contracts/zero-ex/contracts/test/TestWethTransformerHost.sol

@@ -48,6 +48,14 @@ contract TestWethTransformerHost is
             _weth.deposit{value: wethAmount}();
         }
         // Have to make this call externally because transformers aren't payable.
-        this.rawExecuteTransform(transformer, bytes32(0), msg.sender, data);
+        this.rawExecuteTransform(
+            transformer,
+            IERC20Transformer.TransformContext({
+                callDataHash: bytes32(0),
+                sender: msg.sender,
+                taker: msg.sender,
+                data: data
+            })
+        );
     }
 }

contracts/zero-ex/test/artifacts.ts

@@ -42,6 +42,7 @@ import * as LibOwnableRichErrors from '../test/generated-artifacts/LibOwnableRic
 import * as LibOwnableStorage from '../test/generated-artifacts/LibOwnableStorage.json';
 import * as LibProxyRichErrors from '../test/generated-artifacts/LibProxyRichErrors.json';
 import * as LibProxyStorage from '../test/generated-artifacts/LibProxyStorage.json';
+import * as LibReentrancyGuardStorage from '../test/generated-artifacts/LibReentrancyGuardStorage.json';
 import * as LibSignatureRichErrors from '../test/generated-artifacts/LibSignatureRichErrors.json';
 import * as LibSignedCallData from '../test/generated-artifacts/LibSignedCallData.json';
 import * as LibSimpleFunctionRegistryRichErrors from '../test/generated-artifacts/LibSimpleFunctionRegistryRichErrors.json';
@@ -147,6 +148,7 @@ export const artifacts = {
     LibMetaTransactionsStorage: LibMetaTransactionsStorage as ContractArtifact,
     LibOwnableStorage: LibOwnableStorage as ContractArtifact,
     LibProxyStorage: LibProxyStorage as ContractArtifact,
+    LibReentrancyGuardStorage: LibReentrancyGuardStorage as ContractArtifact,
     LibSimpleFunctionRegistryStorage: LibSimpleFunctionRegistryStorage as ContractArtifact,
     LibStorage: LibStorage as ContractArtifact,
     LibTokenSpenderStorage: LibTokenSpenderStorage as ContractArtifact,

contracts/zero-ex/test/features/meta_transactions_test.ts

@@ -36,6 +36,10 @@ blockchainTests.resets('MetaTransactions feature', env => {
     let allowanceTarget: string;
 
     const MAX_FEE_AMOUNT = new BigNumber('1e18');
+    const TRANSFORM_ERC20_FAILING_VALUE = new BigNumber(666);
+    const TRANSFORM_ERC20_REENTER_VALUE = new BigNumber(777);
+    const TRANSFORM_ERC20_BATCH_REENTER_VALUE = new BigNumber(888);
+    const REENTRANCY_FLAG_MTX = 0x1;
 
     before(async () => {
         [owner, sender, ...signers] = await env.getAccountAddressesAsync();
@@ -263,7 +267,7 @@ blockchainTests.resets('MetaTransactions feature', env => {
         it('fails if the translated call fails', async () => {
             const args = getRandomTransformERC20Args();
             const mtx = getRandomMetaTransaction({
-                value: new BigNumber(666),
+                value: new BigNumber(TRANSFORM_ERC20_FAILING_VALUE),
                 callData: transformERC20Feature
                     .transformERC20(
                         args.inputToken,
@@ -469,6 +473,72 @@ blockchainTests.resets('MetaTransactions feature', env => {
                 ),
             );
         });
+
+        it('cannot reenter `executeMetaTransaction()`', async () => {
+            const args = getRandomTransformERC20Args();
+            const mtx = getRandomMetaTransaction({
+                callData: transformERC20Feature
+                    .transformERC20(
+                        args.inputToken,
+                        args.outputToken,
+                        args.inputTokenAmount,
+                        args.minOutputTokenAmount,
+                        args.transformations,
+                    )
+                    .getABIEncodedTransactionData(),
+                value: TRANSFORM_ERC20_REENTER_VALUE,
+            });
+            const mtxHash = getExchangeProxyMetaTransactionHash(mtx);
+            const signature = await signMetaTransactionAsync(mtx);
+            const callOpts = {
+                gasPrice: mtx.maxGasPrice,
+                value: mtx.value,
+            };
+            const tx = feature.executeMetaTransaction(mtx, signature).awaitTransactionSuccessAsync(callOpts);
+            return expect(tx).to.revertWith(
+                new ZeroExRevertErrors.MetaTransactions.MetaTransactionCallFailedError(
+                    mtxHash,
+                    undefined,
+                    new ZeroExRevertErrors.Common.IllegalReentrancyError(
+                        feature.getSelector('executeMetaTransaction'),
+                        REENTRANCY_FLAG_MTX,
+                    ).encode(),
+                ),
+            );
+        });
+
+        it('cannot reenter `batchExecuteMetaTransactions()`', async () => {
+            const args = getRandomTransformERC20Args();
+            const mtx = getRandomMetaTransaction({
+                callData: transformERC20Feature
+                    .transformERC20(
+                        args.inputToken,
+                        args.outputToken,
+                        args.inputTokenAmount,
+                        args.minOutputTokenAmount,
+                        args.transformations,
+                    )
+                    .getABIEncodedTransactionData(),
+                value: TRANSFORM_ERC20_BATCH_REENTER_VALUE,
+            });
+            const mtxHash = getExchangeProxyMetaTransactionHash(mtx);
+            const signature = await signMetaTransactionAsync(mtx);
+            const callOpts = {
+                gasPrice: mtx.maxGasPrice,
+                value: mtx.value,
+            };
+            const tx = feature.executeMetaTransaction(mtx, signature).awaitTransactionSuccessAsync(callOpts);
+            return expect(tx).to.revertWith(
+                new ZeroExRevertErrors.MetaTransactions.MetaTransactionCallFailedError(
+                    mtxHash,
+                    undefined,
+                    new ZeroExRevertErrors.Common.IllegalReentrancyError(
+                        feature.getSelector('batchExecuteMetaTransactions'),
+                        REENTRANCY_FLAG_MTX,
+                    ).encode(),
+                ),
+            );
+        });
     });
 
     describe('batchExecuteMetaTransactions()', () => {
@@ -526,6 +596,102 @@ blockchainTests.resets('MetaTransactions feature', env => {
                 new ZeroExRevertErrors.MetaTransactions.MetaTransactionAlreadyExecutedError(mtxHash, block),
             );
         });
+
+        it('fails if a meta-transaction fails', async () => {
+            const args = getRandomTransformERC20Args();
+            const mtx = getRandomMetaTransaction({
+                value: new BigNumber(TRANSFORM_ERC20_FAILING_VALUE),
+                callData: transformERC20Feature
+                    .transformERC20(
+                        args.inputToken,
+                        args.outputToken,
+                        args.inputTokenAmount,
+                        args.minOutputTokenAmount,
+                        args.transformations,
+                    )
+                    .getABIEncodedTransactionData(),
+            });
+            const mtxHash = getExchangeProxyMetaTransactionHash(mtx);
+            const signature = await signMetaTransactionAsync(mtx);
+            const callOpts = {
+                gasPrice: mtx.minGasPrice,
+                value: mtx.value,
+            };
+            const tx = feature.batchExecuteMetaTransactions([mtx], [signature]).callAsync(callOpts);
+            return expect(tx).to.revertWith(
+                new ZeroExRevertErrors.MetaTransactions.MetaTransactionCallFailedError(
+                    mtxHash,
+                    undefined,
+                    new StringRevertError('FAIL').encode(),
+                ),
+            );
+        });
+
+        it('cannot reenter `executeMetaTransaction()`', async () => {
+            const args = getRandomTransformERC20Args();
+            const mtx = getRandomMetaTransaction({
+                callData: transformERC20Feature
+                    .transformERC20(
+                        args.inputToken,
+                        args.outputToken,
+                        args.inputTokenAmount,
+                        args.minOutputTokenAmount,
+                        args.transformations,
+                    )
+                    .getABIEncodedTransactionData(),
+                value: TRANSFORM_ERC20_REENTER_VALUE,
+            });
+            const mtxHash = getExchangeProxyMetaTransactionHash(mtx);
+            const signature = await signMetaTransactionAsync(mtx);
+            const callOpts = {
+                gasPrice: mtx.maxGasPrice,
+                value: mtx.value,
+            };
+            const tx = feature.batchExecuteMetaTransactions([mtx], [signature]).awaitTransactionSuccessAsync(callOpts);
+            return expect(tx).to.revertWith(
+                new ZeroExRevertErrors.MetaTransactions.MetaTransactionCallFailedError(
+                    mtxHash,
+                    undefined,
+                    new ZeroExRevertErrors.Common.IllegalReentrancyError(
+                        feature.getSelector('executeMetaTransaction'),
+                        REENTRANCY_FLAG_MTX,
+                    ).encode(),
+                ),
+            );
+        });
+
+        it('cannot reenter `batchExecuteMetaTransactions()`', async () => {
+            const args = getRandomTransformERC20Args();
+            const mtx = getRandomMetaTransaction({
+                callData: transformERC20Feature
+                    .transformERC20(
+                        args.inputToken,
+                        args.outputToken,
+                        args.inputTokenAmount,
+                        args.minOutputTokenAmount,
+                        args.transformations,
+                    )
+                    .getABIEncodedTransactionData(),
+                value: TRANSFORM_ERC20_BATCH_REENTER_VALUE,
+            });
+            const mtxHash = getExchangeProxyMetaTransactionHash(mtx);
+            const signature = await signMetaTransactionAsync(mtx);
+            const callOpts = {
+                gasPrice: mtx.maxGasPrice,
+                value: mtx.value,
+            };
+            const tx = feature.batchExecuteMetaTransactions([mtx], [signature]).awaitTransactionSuccessAsync(callOpts);
+            return expect(tx).to.revertWith(
+                new ZeroExRevertErrors.MetaTransactions.MetaTransactionCallFailedError(
+                    mtxHash,
+                    undefined,
+                    new ZeroExRevertErrors.Common.IllegalReentrancyError(
+                        feature.getSelector('batchExecuteMetaTransactions'),
+                        REENTRANCY_FLAG_MTX,
+                    ).encode(),
+                ),
+            );
+        });
     });
 
     describe('getMetaTransactionExecutedBlock()', () => {

contracts/zero-ex/test/features/transform_erc20_test.ts

@@ -37,6 +37,7 @@ blockchainTests.resets('TransformERC20 feature', env => {
     const callDataSigner = ethjs.bufferToHex(ethjs.privateToAddress(ethjs.toBuffer(callDataSignerKey)));
     let owner: string;
     let taker: string;
+    let sender: string;
     let transformerDeployer: string;
     let zeroEx: ZeroExContract;
     let feature: TransformERC20Contract;
@@ -44,7 +45,7 @@ blockchainTests.resets('TransformERC20 feature', env => {
     let allowanceTarget: string;
 
     before(async () => {
-        [owner, taker, transformerDeployer] = await env.getAccountAddressesAsync();
+        [owner, taker, sender, transformerDeployer] = await env.getAccountAddressesAsync();
         zeroEx = await fullMigrateAsync(
             owner,
             env.provider,
@@ -59,12 +60,12 @@ blockchainTests.resets('TransformERC20 feature', env => {
             },
             { transformerDeployer },
         );
-        feature = new TransformERC20Contract(zeroEx.address, env.provider, env.txDefaults, abis);
+        feature = new TransformERC20Contract(zeroEx.address, env.provider, { ...env.txDefaults, from: sender }, abis);
         wallet = new FlashWalletContract(await feature.getTransformWallet().callAsync(), env.provider, env.txDefaults);
         allowanceTarget = await new ITokenSpenderContract(zeroEx.address, env.provider, env.txDefaults)
             .getAllowanceTarget()
             .callAsync();
-        await feature.setQuoteSigner(callDataSigner).awaitTransactionSuccessAsync();
+        await feature.setQuoteSigner(callDataSigner).awaitTransactionSuccessAsync({ from: owner });
     });
 
     const { MAX_UINT256, ZERO_AMOUNT } = constants;
@@ -73,7 +74,7 @@ blockchainTests.resets('TransformERC20 feature', env => {
         it('createTransformWallet() replaces the current wallet', async () => {
             const newWalletAddress = await feature.createTransformWallet().callAsync({ from: owner });
             expect(newWalletAddress).to.not.eq(wallet.address);
-            await feature.createTransformWallet().awaitTransactionSuccessAsync();
+            await feature.createTransformWallet().awaitTransactionSuccessAsync({ from: owner });
             return expect(feature.getTransformWallet().callAsync()).to.eventually.eq(newWalletAddress);
         });
 
@@ -264,8 +265,9 @@ blockchainTests.resets('TransformERC20 feature', env => {
                     receipt.logs,
                     [
                         {
-                            callDataHash: NULL_BYTES32,
+                            sender,
                             taker,
+                            callDataHash: NULL_BYTES32,
                             context: wallet.address,
                             caller: zeroEx.address,
                             data: transformation.data,
@@ -320,8 +322,9 @@ blockchainTests.resets('TransformERC20 feature', env => {
                     receipt.logs,
                     [
                         {
-                            callDataHash: NULL_BYTES32,
                             taker,
+                            sender,
+                            callDataHash: NULL_BYTES32,
                             context: wallet.address,
                             caller: zeroEx.address,
                             data: transformation.data,
@@ -379,8 +382,9 @@ blockchainTests.resets('TransformERC20 feature', env => {
                     receipt.logs,
                     [
                         {
-                            callDataHash: NULL_BYTES32,
+                            sender,
                             taker,
+                            callDataHash: NULL_BYTES32,
                             context: wallet.address,
                             caller: zeroEx.address,
                             data: transformation.data,
@@ -496,8 +500,9 @@ blockchainTests.resets('TransformERC20 feature', env => {
                     receipt.logs,
                     [
                         {
-                            callDataHash: NULL_BYTES32,
+                            sender,
                             taker,
+                            callDataHash: NULL_BYTES32,
                             context: wallet.address,
                             caller: zeroEx.address,
                             data: transformations[0].data,
@@ -505,8 +510,9 @@ blockchainTests.resets('TransformERC20 feature', env => {
                             ethBalance: callValue,
                         },
                         {
-                            callDataHash: NULL_BYTES32,
+                            sender,
                             taker,
+                            callDataHash: NULL_BYTES32,
                             context: wallet.address,
                             caller: zeroEx.address,
                             data: transformations[1].data,

contracts/zero-ex/test/transformers/affiliate_fee_transformer_test.ts

@@ -86,7 +86,12 @@ blockchainTests.resets('AffiliateFeeTransformer', env => {
         await mintHostTokensAsync(amounts[0]);
         await sendEtherAsync(host.address, amounts[1]);
         await host
-            .rawExecuteTransform(transformer.address, hexUtils.random(), randomAddress(), data)
+            .rawExecuteTransform(transformer.address, {
+                data,
+                callDataHash: hexUtils.random(),
+                sender: randomAddress(),
+                taker: randomAddress(),
+            })
             .awaitTransactionSuccessAsync();
         expect(await getBalancesAsync(host.address)).to.deep.eq(ZERO_BALANCES);
         expect(await getBalancesAsync(recipients[0])).to.deep.eq({
@@ -112,7 +117,12 @@ blockchainTests.resets('AffiliateFeeTransformer', env => {
         await mintHostTokensAsync(amounts[0]);
         await sendEtherAsync(host.address, amounts[1]);
         await host
-            .rawExecuteTransform(transformer.address, hexUtils.random(), randomAddress(), data)
+            .rawExecuteTransform(transformer.address, {
+                data,
+                callDataHash: hexUtils.random(),
+                sender: randomAddress(),
+                taker: randomAddress(),
+            })
             .awaitTransactionSuccessAsync();
         expect(await getBalancesAsync(host.address)).to.deep.eq(ZERO_BALANCES);
         expect(await getBalancesAsync(recipients[0])).to.deep.eq({
@@ -138,7 +148,12 @@ blockchainTests.resets('AffiliateFeeTransformer', env => {
         await mintHostTokensAsync(amounts[0]);
         await sendEtherAsync(host.address, amounts[1]);
         await host
-            .rawExecuteTransform(transformer.address, hexUtils.random(), randomAddress(), data)
+            .rawExecuteTransform(transformer.address, {
+                data,
+                callDataHash: hexUtils.random(),
+                sender: randomAddress(),
+                taker: randomAddress(),
+            })
             .awaitTransactionSuccessAsync();
         expect(await getBalancesAsync(host.address)).to.deep.eq({
             tokenBalance: new BigNumber(1),

contracts/zero-ex/test/transformers/fill_quote_transformer_test.ts

@@ -32,6 +32,8 @@ const { NULL_ADDRESS, NULL_BYTES, MAX_UINT256, ZERO_AMOUNT } = constants;
 blockchainTests.resets('FillQuoteTransformer', env => {
     let maker: string;
     let feeRecipient: string;
+    let sender: string;
+    let taker: string;
     let exchange: TestFillQuoteTransformerExchangeContract;
     let bridge: TestFillQuoteTransformerBridgeContract;
     let transformer: FillQuoteTransformerContract;
@@ -44,7 +46,7 @@ blockchainTests.resets('FillQuoteTransformer', env => {
     const GAS_PRICE = 1337;
 
     before(async () => {
-        [maker, feeRecipient] = await env.getAccountAddressesAsync();
+        [maker, feeRecipient, sender, taker] = await env.getAccountAddressesAsync();
         exchange = await TestFillQuoteTransformerExchangeContract.deployFrom0xArtifactAsync(
             artifacts.TestFillQuoteTransformerExchange,
             env.provider,
@@ -92,7 +94,7 @@ blockchainTests.resets('FillQuoteTransformer', env => {
         bridge = await TestFillQuoteTransformerBridgeContract.deployFrom0xArtifactAsync(
             artifacts.TestFillQuoteTransformerBridge,
             env.provider,
-            env.txDefaults,
+            { ...env.txDefaults, from: sender },
             artifacts,
         );
         [makerToken, takerToken, takerFeeToken] = await Promise.all(
@@ -270,6 +272,7 @@ blockchainTests.resets('FillQuoteTransformer', env => {
             signatures: [],
             maxOrderFillAmounts: [],
             fillAmount: MAX_UINT256,
+            refundReceiver: NULL_ADDRESS,
             ...fields,
         });
     }
@@ -313,6 +316,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -334,6 +339,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -358,6 +365,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -380,6 +389,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -404,6 +415,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -430,6 +443,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -462,6 +477,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -486,6 +503,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -511,6 +530,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     takerTokenBalance,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -535,6 +556,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     takerTokenBalance,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -564,6 +587,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -586,6 +611,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -608,6 +635,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -627,6 +656,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -640,6 +671,80 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                 makerAssetBalance: qfr.makerAssetBought,
             });
         });
+
+        it('can refund unspent protocol fee to the `refundReceiver`', async () => {
+            const orders = _.times(2, () => createOrder());
+            const signatures = orders.map(() => encodeExchangeBehavior());
+            const qfr = getExpectedSellQuoteFillResults(orders);
+            const protocolFee = qfr.protocolFeePaid.plus(1);
+            const refundReceiver = randomAddress();
+            await host
+                .executeTransform(
+                    transformer.address,
+                    takerToken.address,
+                    qfr.takerAssetSpent,
+                    sender,
+                    taker,
+                    encodeTransformData({
+                        orders,
+                        signatures,
+                        refundReceiver,
+                    }),
+                )
+                .awaitTransactionSuccessAsync({ value: protocolFee });
+            const receiverBalancer = await env.web3Wrapper.getBalanceInWeiAsync(refundReceiver);
+            expect(receiverBalancer).to.bignumber.eq(1);
+        });
+
+        it('can refund unspent protocol fee to the taker', async () => {
+            const orders = _.times(2, () => createOrder());
+            const signatures = orders.map(() => encodeExchangeBehavior());
+            const qfr = getExpectedSellQuoteFillResults(orders);
+            const protocolFee = qfr.protocolFeePaid.plus(1);
+            const refundReceiver = randomAddress();
+            await host
+                .executeTransform(
+                    transformer.address,
+                    takerToken.address,
+                    qfr.takerAssetSpent,
+                    sender,
+                    refundReceiver, // taker = refundReceiver
+                    encodeTransformData({
+                        orders,
+                        signatures,
+                        // address(1) indicates taker
+                        refundReceiver: hexUtils.leftPad(1, 20),
+                    }),
+                )
+                .awaitTransactionSuccessAsync({ value: protocolFee });
+            const receiverBalancer = await env.web3Wrapper.getBalanceInWeiAsync(refundReceiver);
+            expect(receiverBalancer).to.bignumber.eq(1);
+        });
+
+        it('can refund unspent protocol fee to the sender', async () => {
+            const orders = _.times(2, () => createOrder());
+            const signatures = orders.map(() => encodeExchangeBehavior());
+            const qfr = getExpectedSellQuoteFillResults(orders);
+            const protocolFee = qfr.protocolFeePaid.plus(1);
+            const refundReceiver = randomAddress();
+            await host
+                .executeTransform(
+                    transformer.address,
+                    takerToken.address,
+                    qfr.takerAssetSpent,
+                    refundReceiver, // sender = refundReceiver
+                    taker,
+                    encodeTransformData({
+                        orders,
+                        signatures,
+                        // address(2) indicates sender
+                        refundReceiver: hexUtils.leftPad(2, 20),
+                    }),
+                )
+                .awaitTransactionSuccessAsync({ value: protocolFee });
+            const receiverBalancer = await env.web3Wrapper.getBalanceInWeiAsync(refundReceiver);
+            expect(receiverBalancer).to.bignumber.eq(1);
+        });
     });
 
     describe('buy quotes', () => {
@@ -652,6 +757,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -675,6 +782,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -701,6 +810,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -725,6 +836,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -751,6 +864,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -774,6 +889,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -804,6 +921,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -828,6 +947,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -852,6 +973,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -873,6 +996,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -900,6 +1025,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -926,6 +1053,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -952,6 +1081,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,
@@ -980,6 +1111,8 @@ blockchainTests.resets('FillQuoteTransformer', env => {
                     transformer.address,
                     takerToken.address,
                     qfr.takerAssetSpent,
+                    sender,
+                    taker,
                     encodeTransformData({
                         orders,
                         signatures,

contracts/zero-ex/test/transformers/pay_taker_transformer_test.ts

@@ -78,7 +78,12 @@ blockchainTests.resets('PayTakerTransformer', env => {
         await mintHostTokensAsync(amounts[0]);
         await sendEtherAsync(host.address, amounts[1]);
         await host
-            .rawExecuteTransform(transformer.address, hexUtils.random(), taker, data)
+            .rawExecuteTransform(transformer.address, {
+                data,
+                callDataHash: hexUtils.random(),
+                taker,
+                sender: randomAddress(),
+            })
             .awaitTransactionSuccessAsync();
         expect(await getBalancesAsync(host.address)).to.deep.eq(ZERO_BALANCES);
         expect(await getBalancesAsync(taker)).to.deep.eq({
@@ -96,7 +101,12 @@ blockchainTests.resets('PayTakerTransformer', env => {
         await mintHostTokensAsync(amounts[0]);
         await sendEtherAsync(host.address, amounts[1]);
         await host
-            .rawExecuteTransform(transformer.address, hexUtils.random(), taker, data)
+            .rawExecuteTransform(transformer.address, {
+                data,
+                callDataHash: hexUtils.random(),
+                taker,
+                sender: randomAddress(),
+            })
             .awaitTransactionSuccessAsync();
         expect(await getBalancesAsync(host.address)).to.deep.eq(ZERO_BALANCES);
         expect(await getBalancesAsync(taker)).to.deep.eq({
@@ -114,7 +124,12 @@ blockchainTests.resets('PayTakerTransformer', env => {
         await mintHostTokensAsync(amounts[0]);
         await sendEtherAsync(host.address, amounts[1]);
         await host
-            .rawExecuteTransform(transformer.address, hexUtils.random(), taker, data)
+            .rawExecuteTransform(transformer.address, {
+                data,
+                callDataHash: hexUtils.random(),
+                taker,
+                sender: randomAddress(),
+            })
             .awaitTransactionSuccessAsync();
         expect(await getBalancesAsync(host.address)).to.deep.eq(ZERO_BALANCES);
         expect(await getBalancesAsync(taker)).to.deep.eq({
@@ -132,7 +147,12 @@ blockchainTests.resets('PayTakerTransformer', env => {
         await mintHostTokensAsync(amounts[0]);
         await sendEtherAsync(host.address, amounts[1]);
         await host
-            .rawExecuteTransform(transformer.address, hexUtils.random(), taker, data)
+            .rawExecuteTransform(transformer.address, {
+                data,
+                callDataHash: hexUtils.random(),
+                taker,
+                sender: randomAddress(),
+            })
             .awaitTransactionSuccessAsync();
         expect(await getBalancesAsync(host.address)).to.deep.eq({
             tokenBalance: amounts[0].minus(amounts[0].dividedToIntegerBy(2)),

contracts/zero-ex/test/wrappers.ts

@@ -40,6 +40,7 @@ export * from '../test/generated-wrappers/lib_ownable_rich_errors';
 export * from '../test/generated-wrappers/lib_ownable_storage';
 export * from '../test/generated-wrappers/lib_proxy_rich_errors';
 export * from '../test/generated-wrappers/lib_proxy_storage';
+export * from '../test/generated-wrappers/lib_reentrancy_guard_storage';
 export * from '../test/generated-wrappers/lib_signature_rich_errors';
 export * from '../test/generated-wrappers/lib_signed_call_data';
 export * from '../test/generated-wrappers/lib_simple_function_registry_rich_errors';

contracts/zero-ex/tsconfig.json

@@ -32,6 +32,7 @@
         "test/generated-artifacts/FillQuoteTransformer.json",
         "test/generated-artifacts/FixinCommon.json",
         "test/generated-artifacts/FixinEIP712.json",
+        "test/generated-artifacts/FixinReentrancyGuard.json",
         "test/generated-artifacts/FlashWallet.json",
         "test/generated-artifacts/FullMigration.json",
         "test/generated-artifacts/IAllowanceTarget.json",
@@ -62,6 +63,7 @@
         "test/generated-artifacts/LibOwnableStorage.json",
         "test/generated-artifacts/LibProxyRichErrors.json",
         "test/generated-artifacts/LibProxyStorage.json",
+        "test/generated-artifacts/LibReentrancyGuardStorage.json",
         "test/generated-artifacts/LibSignatureRichErrors.json",
         "test/generated-artifacts/LibSignedCallData.json",
         "test/generated-artifacts/LibSimpleFunctionRegistryRichErrors.json",

packages/asset-swapper/CHANGELOG.json

@@ -85,6 +85,10 @@
             {
                 "note": "Enable Quote Report to be generated with an option `shouldGenerateQuoteReport`. Default is `false`",
                 "pr": 2687
+            },
+            {
+                "note": "Add `refundReceiver` to `ExchangeProxySwapQuoteConsumer` options.",
+                "pr": 2657
             }
         ]
     },

packages/asset-swapper/src/index.ts

@@ -89,6 +89,7 @@ export {
     AffiliateFee,
     CalldataInfo,
     ExchangeProxyContractOpts,
+    ExchangeProxyRefundReceiver,
     ExtensionContractType,
     ForwarderExtensionContractOpts,
     GetExtensionContractTypeOpts,

packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts

@@ -84,7 +84,7 @@ export class ExchangeProxySwapQuoteConsumer implements SwapQuoteConsumerBase {
     ): Promise<CalldataInfo> {
         assert.isValidSwapQuote('quote', quote);
         // tslint:disable-next-line:no-object-literal-type-assertion
-        const { affiliateFee, isFromETH, isToETH } = {
+        const { refundReceiver, affiliateFee, isFromETH, isToETH } = {
             ...constants.DEFAULT_EXCHANGE_PROXY_EXTENSION_CONTRACT_OPTS,
             ...opts.extensionContractOpts,
         } as ExchangeProxyContractOpts;
@@ -116,6 +116,7 @@ export class ExchangeProxySwapQuoteConsumer implements SwapQuoteConsumerBase {
                     sellToken,
                     buyToken: intermediateToken,
                     side: FillQuoteTransformerSide.Sell,
+                    refundReceiver: refundReceiver || NULL_ADDRESS,
                     fillAmount: firstHopOrder.takerAssetAmount,
                     maxOrderFillAmounts: [],
                     orders: [firstHopOrder],
@@ -125,8 +126,9 @@ export class ExchangeProxySwapQuoteConsumer implements SwapQuoteConsumerBase {
             transforms.push({
                 deploymentNonce: this.transformerNonces.fillQuoteTransformer,
                 data: encodeFillQuoteTransformerData({
-                    sellToken: intermediateToken,
                     buyToken,
+                    sellToken: intermediateToken,
+                    refundReceiver: refundReceiver || NULL_ADDRESS,
                     side: FillQuoteTransformerSide.Sell,
                     fillAmount: MAX_UINT256,
                     maxOrderFillAmounts: [],
@@ -140,6 +142,7 @@ export class ExchangeProxySwapQuoteConsumer implements SwapQuoteConsumerBase {
                 data: encodeFillQuoteTransformerData({
                     sellToken,
                     buyToken,
+                    refundReceiver: refundReceiver || NULL_ADDRESS,
                     side: isBuyQuote(quote) ? FillQuoteTransformerSide.Buy : FillQuoteTransformerSide.Sell,
                     fillAmount: isBuyQuote(quote) ? quote.makerAssetFillAmount : quote.takerAssetFillAmount,
                     maxOrderFillAmounts: [],

packages/asset-swapper/src/types.ts

@@ -135,15 +135,31 @@ export interface AffiliateFee {
     sellTokenFeeAmount: BigNumber;
 }
 
+/**
+ * Automatically resolved protocol fee refund receiver addresses.
+ */
+export enum ExchangeProxyRefundReceiver {
+    // Refund to the taker address.
+    Taker = '0x0000000000000000000000000000000000000001',
+    // Refund to the sender address.
+    Sender = '0x0000000000000000000000000000000000000002',
+}
+
 /**
  * @param isFromETH Whether the input token is ETH.
  * @param isToETH Whether the output token is ETH.
  * @param affiliateFee Fee denominated in taker or maker asset to send to specified recipient.
+ * @param refundReceiver The receiver of unspent protocol fees.
+ *        May be a valid address or one of:
+ *        `address(0)`: Stay in flash wallet.
+ *        `address(1)`: Send to the taker.
+ *        `address(2)`: Send to the sender (caller of `transformERC20()`).
  */
 export interface ExchangeProxyContractOpts {
     isFromETH: boolean;
     isToETH: boolean;
     affiliateFee: AffiliateFee;
+    refundReceiver: string | ExchangeProxyRefundReceiver;
 }
 
 export interface GetExtensionContractTypeOpts {

packages/order-utils/CHANGELOG.json

@@ -1,6 +1,6 @@
 [
     {
-        "version": "10.3.1",
+        "version": "10.4.0",
         "changes": [
             {
                 "note": "Add gitpkg.",
@@ -9,6 +9,10 @@
             {
                 "note": "Fix `decodeAffiliateFeeTransformerData`",
                 "pr": 2658
+            },
+            {
+                "note": "Add `refundReceiver` field to `FillQuoteTransformer.TransformData`.",
+                "pr": 2657
             }
         ]
     },

packages/order-utils/src/transformer_data_encoders.ts

@@ -37,6 +37,7 @@ export const fillQuoteTransformerDataEncoder = AbiEncoder.create([
             { name: 'signatures', type: 'bytes[]' },
             { name: 'maxOrderFillAmounts', type: 'uint256[]' },
             { name: 'fillAmount', type: 'uint256' },
+            { name: 'refundReceiver', type: 'address' },
         ],
     },
 ]);
@@ -60,6 +61,7 @@ export interface FillQuoteTransformerData {
     signatures: string[];
     maxOrderFillAmounts: BigNumber[];
     fillAmount: BigNumber;
+    refundReceiver: string;
 }
 
 /**

packages/utils/CHANGELOG.json

@@ -5,6 +5,10 @@
             {
                 "note": "Added support for nested rich revert decoding",
                 "pr": 2668
+            },
+            {
+                "note": "Add EP flavor of `IllegalReentrancyError`.",
+                "pr": 2657
             }
         ]
     },

packages/utils/src/revert_errors/zero-ex/common_revert_errors.ts

@@ -1,4 +1,5 @@
 import { RevertError } from '../../revert_error';
+import { Numberish } from '../../types';
 
 // tslint:disable:max-classes-per-file
 export class OnlyCallableBySelfError extends RevertError {
@@ -9,14 +10,16 @@ export class OnlyCallableBySelfError extends RevertError {
     }
 }
 
-// This is identical to the one in utils.
+export class IllegalReentrancyError extends RevertError {
-// export class IllegalReentrancyError extends RevertError {
+    constructor(selector?: string, reentrancyFlags?: Numberish) {
-//     constructor() {
+        super('IllegalReentrancyError', 'IllegalReentrancyError(bytes4 selector, uint256 reentrancyFlags)', {
-//         super('IllegalReentrancyError', 'IllegalReentrancyError()', {});
+            selector,
-//     }
+            reentrancyFlags,
-// }
+        });
+    }
+}
 
-const types = [OnlyCallableBySelfError];
+const types = [OnlyCallableBySelfError, IllegalReentrancyError];
 
 // Register the types we've defined.
 for (const type of types) {