diff --git a/contracts/zero-ex/CHANGELOG.json b/contracts/zero-ex/CHANGELOG.json index 69e75db6b6..ba26a2b504 100644 --- a/contracts/zero-ex/CHANGELOG.json +++ b/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": [ diff --git a/contracts/zero-ex/contracts/src/errors/LibCommonRichErrors.sol b/contracts/zero-ex/contracts/src/errors/LibCommonRichErrors.sol index 17a038c1d9..f5b6e5952f 100644 --- a/contracts/zero-ex/contracts/src/errors/LibCommonRichErrors.sol +++ b/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 ); } } diff --git a/contracts/zero-ex/contracts/src/features/MetaTransactions.sol b/contracts/zero-ex/contracts/src/features/MetaTransactions.sol index 975520a6a5..f9eca8960e 100644 --- a/contracts/zero-ex/contracts/src/features/MetaTransactions.sol +++ b/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 { diff --git a/contracts/zero-ex/contracts/src/features/SignatureValidator.sol b/contracts/zero-ex/contracts/src/features/SignatureValidator.sol index b7e6b1b71a..a8039af0e9 100644 --- a/contracts/zero-ex/contracts/src/features/SignatureValidator.sol +++ b/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( - hash, - v, - r, - s - ); + if (v < 27) { + // Handle clients that encode v as 0 or 1. + v += 27; + } + 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( - keccak256(abi.encodePacked( - "\x19Ethereum Signed Message:\n32", - hash - )), - v, - r, - s - ); + if (v < 27) { + // Handle clients that encode v as 0 or 1. + v += 27; + } + if (uint256(r) < ECDSA_SIGNATURE_R_LIMIT && uint256(s) < ECDSA_SIGNATURE_S_LIMIT) { + recovered = ecrecover( + keccak256(abi.encodePacked( + "\x19Ethereum Signed Message:\n32", + hash + )), + v, + r, + s + ); + } } else { // This should never happen. revert('SignatureValidator/ILLEGAL_CODE_PATH'); diff --git a/contracts/zero-ex/contracts/src/features/TransformERC20.sol b/contracts/zero-ex/contracts/src/features/TransformERC20.sol index e7bafaa355..a2fe5a48ff 100644 --- a/contracts/zero-ex/contracts/src/features/TransformERC20.sol +++ b/contracts/zero-ex/contracts/src/features/TransformERC20.sol @@ -360,9 +360,12 @@ contract TransformERC20 is // Call data. abi.encodeWithSelector( IERC20Transformer.transform.selector, - callDataHash, - taker, - transformation.data + IERC20Transformer.TransformContext({ + callDataHash: callDataHash, + sender: msg.sender, + taker: taker, + data: transformation.data + }) ) ); // Ensure the transformer returned the magic bytes. diff --git a/contracts/zero-ex/contracts/src/fixins/FixinReentrancyGuard.sol b/contracts/zero-ex/contracts/src/fixins/FixinReentrancyGuard.sol new file mode 100644 index 0000000000..f8081dc3d4 --- /dev/null +++ b/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); + } +} diff --git a/contracts/zero-ex/contracts/src/storage/LibReentrancyGuardStorage.sol b/contracts/zero-ex/contracts/src/storage/LibReentrancyGuardStorage.sol new file mode 100644 index 0000000000..58c52fb467 --- /dev/null +++ b/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 } + } +} diff --git a/contracts/zero-ex/contracts/src/storage/LibStorage.sol b/contracts/zero-ex/contracts/src/storage/LibStorage.sol index 1af79e919f..809977d4a8 100644 --- a/contracts/zero-ex/contracts/src/storage/LibStorage.sol +++ b/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 diff --git a/contracts/zero-ex/contracts/src/transformers/AffiliateFeeTransformer.sol b/contracts/zero-ex/contracts/src/transformers/AffiliateFeeTransformer.sol index 17025ef0b2..8ec1cd467e 100644 --- a/contracts/zero-ex/contracts/src/transformers/AffiliateFeeTransformer.sol +++ b/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( - bytes32, // callDataHash, - address payable, // taker, - bytes calldata data - ) + function transform(TransformContext calldata context) 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) { diff --git a/contracts/zero-ex/contracts/src/transformers/FillQuoteTransformer.sol b/contracts/zero-ex/contracts/src/transformers/FillQuoteTransformer.sol index 3adbb48036..656a86549d 100644 --- a/contracts/zero-ex/contracts/src/transformers/FillQuoteTransformer.sol +++ b/contracts/zero-ex/contracts/src/transformers/FillQuoteTransformer.sol @@ -74,6 +74,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()`. @@ -111,6 +117,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,32 +142,28 @@ 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( - bytes32, // callDataHash, - address payable, // taker, - bytes calldata data_ - ) + function transform(TransformContext calldata context) external override freesGasTokensFromCollector 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(); } @@ -249,6 +257,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; } diff --git a/contracts/zero-ex/contracts/src/transformers/IERC20Transformer.sol b/contracts/zero-ex/contracts/src/transformers/IERC20Transformer.sol index 40fd9387c0..5b079ed686 100644 --- a/contracts/zero-ex/contracts/src/transformers/IERC20Transformer.sol +++ b/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 taker The taker address (caller of `TransformERC20.transformERC20()`). - /// @param data Arbitrary data to pass to the transformer. + /// @param context Context information. /// @return success The success bytes (`LibERC20Transformer.TRANSFORMER_SUCCESS`). - function transform( - bytes32 callDataHash, - address payable taker, - bytes calldata data - ) + function transform(TransformContext calldata context) external returns (bytes4 success); } diff --git a/contracts/zero-ex/contracts/src/transformers/PayTakerTransformer.sol b/contracts/zero-ex/contracts/src/transformers/PayTakerTransformer.sol index 4b03759ab1..37925301a0 100644 --- a/contracts/zero-ex/contracts/src/transformers/PayTakerTransformer.sol +++ b/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 data_ ABI-encoded `TransformData`, indicating which tokens to transfer. + /// @param context Context information. /// @return success The success bytes (`LibERC20Transformer.TRANSFORMER_SUCCESS`). - function transform( - bytes32, // callDataHash, - address payable taker, - bytes calldata data_ - ) + function transform(TransformContext calldata context) 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; diff --git a/contracts/zero-ex/contracts/src/transformers/WethTransformer.sol b/contracts/zero-ex/contracts/src/transformers/WethTransformer.sol index 503712cfd9..3fa744f504 100644 --- a/contracts/zero-ex/contracts/src/transformers/WethTransformer.sol +++ b/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( - bytes32, // callDataHash, - address payable, // taker, - bytes calldata data_ - ) + function transform(TransformContext calldata context) 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(); } diff --git a/contracts/zero-ex/contracts/test/TestFillQuoteTransformerHost.sol b/contracts/zero-ex/contracts/test/TestFillQuoteTransformerHost.sol index 6b85acd2f5..250a8dd9da 100644 --- a/contracts/zero-ex/contracts/test/TestFillQuoteTransformerHost.sol +++ b/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 + }) + ); } } diff --git a/contracts/zero-ex/contracts/test/TestMetaTransactionsTransformERC20Feature.sol b/contracts/zero-ex/contracts/test/TestMetaTransactionsTransformERC20Feature.sol index 913fa414d7..5c7d8f1596 100644 --- a/contracts/zero-ex/contracts/test/TestMetaTransactionsTransformERC20Feature.sol +++ b/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, diff --git a/contracts/zero-ex/contracts/test/TestMintTokenERC20Transformer.sol b/contracts/zero-ex/contracts/test/TestMintTokenERC20Transformer.sol index 2bd2463b64..bb5fccd464 100644 --- a/contracts/zero-ex/contracts/test/TestMintTokenERC20Transformer.sol +++ b/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( - bytes32 callDataHash, - address payable taker, - bytes calldata data_ - ) + function transform(TransformContext calldata context) 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, - taker, - data_, + context.callDataHash, + context.sender, + 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; } diff --git a/contracts/zero-ex/contracts/test/TestTransformerBase.sol b/contracts/zero-ex/contracts/test/TestTransformerBase.sol index d08151d3b4..ba6415310b 100644 --- a/contracts/zero-ex/contracts/test/TestTransformerBase.sol +++ b/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( - bytes32, - address payable, - bytes calldata - ) + function transform(TransformContext calldata context) external override returns (bytes4 success) diff --git a/contracts/zero-ex/contracts/test/TestTransformerHost.sol b/contracts/zero-ex/contracts/test/TestTransformerHost.sol index 587b837de9..458a12b824 100644 --- a/contracts/zero-ex/contracts/test/TestTransformerHost.sol +++ b/contracts/zero-ex/contracts/test/TestTransformerHost.sol @@ -32,18 +32,14 @@ contract TestTransformerHost { function rawExecuteTransform( IERC20Transformer transformer, - bytes32 callDataHash, - address taker, - bytes calldata data + IERC20Transformer.TransformContext calldata context ) external { (bool _success, bytes memory resultData) = address(transformer).delegatecall(abi.encodeWithSelector( transformer.transform.selector, - callDataHash, - taker, - data + context )); if (!_success) { resultData.rrevert(); diff --git a/contracts/zero-ex/contracts/test/TestWethTransformerHost.sol b/contracts/zero-ex/contracts/test/TestWethTransformerHost.sol index 3c0fd83999..6e217c3257 100644 --- a/contracts/zero-ex/contracts/test/TestWethTransformerHost.sol +++ b/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 + }) + ); } } diff --git a/contracts/zero-ex/package.json b/contracts/zero-ex/package.json index fed17c3770..c78bbba942 100644 --- a/contracts/zero-ex/package.json +++ b/contracts/zero-ex/package.json @@ -41,7 +41,7 @@ "config": { "publicInterfaceContracts": "IZeroEx,ZeroEx,FullMigration,InitialMigration,IFlashWallet,IAllowanceTarget,IERC20Transformer,IOwnable,ISimpleFunctionRegistry,ITokenSpender,ITransformERC20,FillQuoteTransformer,PayTakerTransformer,WethTransformer,Ownable,SimpleFunctionRegistry,TransformERC20,TokenSpender,AffiliateFeeTransformer,SignatureValidator,MetaTransactions", "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually.", - "abis": "./test/generated-artifacts/@(AffiliateFeeTransformer|AllowanceTarget|Bootstrap|FillQuoteTransformer|FixinCommon|FixinEIP712|FixinGasToken|FlashWallet|FullMigration|IAllowanceTarget|IBootstrap|IERC20Bridge|IERC20Transformer|IExchange|IFeature|IFlashWallet|IGasToken|IMetaTransactions|IOwnable|ISignatureValidator|ISimpleFunctionRegistry|ITestSimpleFunctionRegistryFeature|ITokenSpender|ITransformERC20|IZeroEx|InitialMigration|LibBootstrap|LibCommonRichErrors|LibERC20Transformer|LibMetaTransactionsRichErrors|LibMetaTransactionsStorage|LibMigrate|LibOwnableRichErrors|LibOwnableStorage|LibProxyRichErrors|LibProxyStorage|LibSignatureRichErrors|LibSignedCallData|LibSimpleFunctionRegistryRichErrors|LibSimpleFunctionRegistryStorage|LibSpenderRichErrors|LibStorage|LibTokenSpenderStorage|LibTransformERC20RichErrors|LibTransformERC20Storage|LibWalletRichErrors|MetaTransactions|Ownable|PayTakerTransformer|SignatureValidator|SimpleFunctionRegistry|TestCallTarget|TestDelegateCaller|TestFillQuoteTransformerBridge|TestFillQuoteTransformerExchange|TestFillQuoteTransformerHost|TestFullMigration|TestInitialMigration|TestMetaTransactionsTransformERC20Feature|TestMigrator|TestMintTokenERC20Transformer|TestMintableERC20Token|TestSimpleFunctionRegistryFeatureImpl1|TestSimpleFunctionRegistryFeatureImpl2|TestTokenSpender|TestTokenSpenderERC20Token|TestTransformERC20|TestTransformerBase|TestTransformerDeployerTransformer|TestTransformerHost|TestWeth|TestWethTransformerHost|TestZeroExFeature|TokenSpender|TransformERC20|Transformer|TransformerDeployer|WethTransformer|ZeroEx).json" + "abis": "./test/generated-artifacts/@(AffiliateFeeTransformer|AllowanceTarget|Bootstrap|FillQuoteTransformer|FixinCommon|FixinEIP712|FixinGasToken|FixinReentrancyGuard|FlashWallet|FullMigration|IAllowanceTarget|IBootstrap|IERC20Bridge|IERC20Transformer|IExchange|IFeature|IFlashWallet|IGasToken|IMetaTransactions|IOwnable|ISignatureValidator|ISimpleFunctionRegistry|ITestSimpleFunctionRegistryFeature|ITokenSpender|ITransformERC20|IZeroEx|InitialMigration|LibBootstrap|LibCommonRichErrors|LibERC20Transformer|LibMetaTransactionsRichErrors|LibMetaTransactionsStorage|LibMigrate|LibOwnableRichErrors|LibOwnableStorage|LibProxyRichErrors|LibProxyStorage|LibReentrancyGuardStorage|LibSignatureRichErrors|LibSignedCallData|LibSimpleFunctionRegistryRichErrors|LibSimpleFunctionRegistryStorage|LibSpenderRichErrors|LibStorage|LibTokenSpenderStorage|LibTransformERC20RichErrors|LibTransformERC20Storage|LibWalletRichErrors|MetaTransactions|Ownable|PayTakerTransformer|SignatureValidator|SimpleFunctionRegistry|TestCallTarget|TestDelegateCaller|TestFillQuoteTransformerBridge|TestFillQuoteTransformerExchange|TestFillQuoteTransformerHost|TestFullMigration|TestInitialMigration|TestMetaTransactionsTransformERC20Feature|TestMigrator|TestMintTokenERC20Transformer|TestMintableERC20Token|TestSimpleFunctionRegistryFeatureImpl1|TestSimpleFunctionRegistryFeatureImpl2|TestTokenSpender|TestTokenSpenderERC20Token|TestTransformERC20|TestTransformerBase|TestTransformerDeployerTransformer|TestTransformerHost|TestWeth|TestWethTransformerHost|TestZeroExFeature|TokenSpender|TransformERC20|Transformer|TransformerDeployer|WethTransformer|ZeroEx).json" }, "repository": { "type": "git", diff --git a/contracts/zero-ex/test/artifacts.ts b/contracts/zero-ex/test/artifacts.ts index c8db1d2135..93bec8a119 100644 --- a/contracts/zero-ex/test/artifacts.ts +++ b/contracts/zero-ex/test/artifacts.ts @@ -12,6 +12,7 @@ import * as FillQuoteTransformer from '../test/generated-artifacts/FillQuoteTran import * as FixinCommon from '../test/generated-artifacts/FixinCommon.json'; import * as FixinEIP712 from '../test/generated-artifacts/FixinEIP712.json'; import * as FixinGasToken from '../test/generated-artifacts/FixinGasToken.json'; +import * as FixinReentrancyGuard from '../test/generated-artifacts/FixinReentrancyGuard.json'; import * as FlashWallet from '../test/generated-artifacts/FlashWallet.json'; import * as FullMigration from '../test/generated-artifacts/FullMigration.json'; import * as IAllowanceTarget from '../test/generated-artifacts/IAllowanceTarget.json'; @@ -41,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'; @@ -120,6 +122,7 @@ export const artifacts = { FixinCommon: FixinCommon as ContractArtifact, FixinEIP712: FixinEIP712 as ContractArtifact, FixinGasToken: FixinGasToken as ContractArtifact, + FixinReentrancyGuard: FixinReentrancyGuard as ContractArtifact, FullMigration: FullMigration as ContractArtifact, InitialMigration: InitialMigration as ContractArtifact, LibBootstrap: LibBootstrap as ContractArtifact, @@ -127,6 +130,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, diff --git a/contracts/zero-ex/test/features/meta_transactions_test.ts b/contracts/zero-ex/test/features/meta_transactions_test.ts index f5f30fda7f..c19f4a038c 100644 --- a/contracts/zero-ex/test/features/meta_transactions_test.ts +++ b/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()', () => { diff --git a/contracts/zero-ex/test/features/transform_erc20_test.ts b/contracts/zero-ex/test/features/transform_erc20_test.ts index 4625325a9c..a0a28e399b 100644 --- a/contracts/zero-ex/test/features/transform_erc20_test.ts +++ b/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, diff --git a/contracts/zero-ex/test/transformers/affiliate_fee_transformer_test.ts b/contracts/zero-ex/test/transformers/affiliate_fee_transformer_test.ts index 1fac52fe99..6cd2b94cbe 100644 --- a/contracts/zero-ex/test/transformers/affiliate_fee_transformer_test.ts +++ b/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), diff --git a/contracts/zero-ex/test/transformers/fill_quote_transformer_test.ts b/contracts/zero-ex/test/transformers/fill_quote_transformer_test.ts index 92bd69d2ef..dc987a10ce 100644 --- a/contracts/zero-ex/test/transformers/fill_quote_transformer_test.ts +++ b/contracts/zero-ex/test/transformers/fill_quote_transformer_test.ts @@ -31,6 +31,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; @@ -43,7 +45,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, @@ -69,7 +71,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( @@ -246,6 +248,7 @@ blockchainTests.resets('FillQuoteTransformer', env => { signatures: [], maxOrderFillAmounts: [], fillAmount: MAX_UINT256, + refundReceiver: NULL_ADDRESS, ...fields, }); } @@ -288,6 +291,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -309,6 +314,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -333,6 +340,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -355,6 +364,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -379,6 +390,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -405,6 +418,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -437,6 +452,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -461,6 +478,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -486,6 +505,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, takerTokenBalance, + sender, + taker, encodeTransformData({ orders, signatures, @@ -510,6 +531,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, takerTokenBalance, + sender, + taker, encodeTransformData({ orders, signatures, @@ -539,6 +562,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -561,6 +586,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -583,6 +610,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -602,6 +631,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -615,6 +646,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', () => { @@ -627,6 +732,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -650,6 +757,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -676,6 +785,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -700,6 +811,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -726,6 +839,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -749,6 +864,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -779,6 +896,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -803,6 +922,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -827,6 +948,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -848,6 +971,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -875,6 +1000,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -901,6 +1028,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -927,6 +1056,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, @@ -955,6 +1086,8 @@ blockchainTests.resets('FillQuoteTransformer', env => { transformer.address, takerToken.address, qfr.takerAssetSpent, + sender, + taker, encodeTransformData({ orders, signatures, diff --git a/contracts/zero-ex/test/transformers/pay_taker_transformer_test.ts b/contracts/zero-ex/test/transformers/pay_taker_transformer_test.ts index f7570e7c62..4b1add082c 100644 --- a/contracts/zero-ex/test/transformers/pay_taker_transformer_test.ts +++ b/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)), diff --git a/contracts/zero-ex/test/wrappers.ts b/contracts/zero-ex/test/wrappers.ts index 5e74584ff7..44ac411a25 100644 --- a/contracts/zero-ex/test/wrappers.ts +++ b/contracts/zero-ex/test/wrappers.ts @@ -10,6 +10,7 @@ export * from '../test/generated-wrappers/fill_quote_transformer'; export * from '../test/generated-wrappers/fixin_common'; export * from '../test/generated-wrappers/fixin_e_i_p712'; export * from '../test/generated-wrappers/fixin_gas_token'; +export * from '../test/generated-wrappers/fixin_reentrancy_guard'; export * from '../test/generated-wrappers/flash_wallet'; export * from '../test/generated-wrappers/full_migration'; export * from '../test/generated-wrappers/i_allowance_target'; @@ -39,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'; diff --git a/contracts/zero-ex/tsconfig.json b/contracts/zero-ex/tsconfig.json index e92154d08e..cccbec79f6 100644 --- a/contracts/zero-ex/tsconfig.json +++ b/contracts/zero-ex/tsconfig.json @@ -31,6 +31,7 @@ "test/generated-artifacts/FixinCommon.json", "test/generated-artifacts/FixinEIP712.json", "test/generated-artifacts/FixinGasToken.json", + "test/generated-artifacts/FixinReentrancyGuard.json", "test/generated-artifacts/FlashWallet.json", "test/generated-artifacts/FullMigration.json", "test/generated-artifacts/IAllowanceTarget.json", @@ -60,6 +61,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", diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index 72ebb1b118..3943c1693f 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -33,6 +33,10 @@ { "note": "Add support for buy token affiliate fees", "pr": 2658 + }, + { + "note": "Add `refundReceiver` to `ExchangeProxySwapQuoteConsumer` options.", + "pr": 2657 } ] }, diff --git a/packages/asset-swapper/src/index.ts b/packages/asset-swapper/src/index.ts index 1f75620bd2..65505e73e0 100644 --- a/packages/asset-swapper/src/index.ts +++ b/packages/asset-swapper/src/index.ts @@ -43,6 +43,7 @@ export { AffiliateFee, CalldataInfo, ExchangeProxyContractOpts, + ExchangeProxyRefundReceiver, ExtensionContractType, ForwarderExtensionContractOpts, GetExtensionContractTypeOpts, diff --git a/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts b/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts index 5fb2c44f63..921b18b11f 100644 --- a/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts +++ b/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts @@ -86,7 +86,7 @@ export class ExchangeProxySwapQuoteConsumer implements SwapQuoteConsumerBase { ): Promise { 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; @@ -114,6 +114,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: [], diff --git a/packages/asset-swapper/src/types.ts b/packages/asset-swapper/src/types.ts index bd193ef406..35d9ddce60 100644 --- a/packages/asset-swapper/src/types.ts +++ b/packages/asset-swapper/src/types.ts @@ -130,15 +130,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 type SwapQuote = MarketBuySwapQuote | MarketSellSwapQuote; diff --git a/packages/order-utils/CHANGELOG.json b/packages/order-utils/CHANGELOG.json index be2bacc702..290563e85a 100644 --- a/packages/order-utils/CHANGELOG.json +++ b/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 } ] }, diff --git a/packages/order-utils/src/transformer_data_encoders.ts b/packages/order-utils/src/transformer_data_encoders.ts index cb3bb17d11..8f747bdbe7 100644 --- a/packages/order-utils/src/transformer_data_encoders.ts +++ b/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; } /** diff --git a/packages/utils/CHANGELOG.json b/packages/utils/CHANGELOG.json index d7e95fd53c..37530885a8 100644 --- a/packages/utils/CHANGELOG.json +++ b/packages/utils/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "5.6.0", + "changes": [ + { + "note": "Add EP flavor of `IllegalReentrancyError`.", + "pr": 2657 + } + ] + }, { "timestamp": 1594788383, "version": "5.5.1", diff --git a/packages/utils/src/revert_errors/zero-ex/common_revert_errors.ts b/packages/utils/src/revert_errors/zero-ex/common_revert_errors.ts index c3f4091ee8..67df68fff0 100644 --- a/packages/utils/src/revert_errors/zero-ex/common_revert_errors.ts +++ b/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 { -// constructor() { -// super('IllegalReentrancyError', 'IllegalReentrancyError()', {}); -// } -// } +export class IllegalReentrancyError extends RevertError { + constructor(selector?: string, reentrancyFlags?: Numberish) { + super('IllegalReentrancyError', 'IllegalReentrancyError(bytes4 selector, uint256 reentrancyFlags)', { + selector, + reentrancyFlags, + }); + } +} -const types = [OnlyCallableBySelfError]; +const types = [OnlyCallableBySelfError, IllegalReentrancyError]; // Register the types we've defined. for (const type of types) {