diff --git a/.forge-snapshots/allowanceHolder_rfq_DAI-WETH.snap b/.forge-snapshots/allowanceHolder_rfq_DAI-WETH.snap index e2644b97b..4ad65978b 100644 --- a/.forge-snapshots/allowanceHolder_rfq_DAI-WETH.snap +++ b/.forge-snapshots/allowanceHolder_rfq_DAI-WETH.snap @@ -1 +1 @@ -87101 \ No newline at end of file +87087 \ No newline at end of file diff --git a/.forge-snapshots/allowanceHolder_rfq_USDC-USDT.snap b/.forge-snapshots/allowanceHolder_rfq_USDC-USDT.snap index cc669e464..49a0efd70 100644 --- a/.forge-snapshots/allowanceHolder_rfq_USDC-USDT.snap +++ b/.forge-snapshots/allowanceHolder_rfq_USDC-USDT.snap @@ -1 +1 @@ -140080 \ No newline at end of file +140066 \ No newline at end of file diff --git a/.forge-snapshots/allowanceHolder_rfq_USDC-WETH.snap b/.forge-snapshots/allowanceHolder_rfq_USDC-WETH.snap index e4d8f6e98..7b1cd7b78 100644 --- a/.forge-snapshots/allowanceHolder_rfq_USDC-WETH.snap +++ b/.forge-snapshots/allowanceHolder_rfq_USDC-WETH.snap @@ -1 +1 @@ -106575 \ No newline at end of file +106561 \ No newline at end of file diff --git a/.forge-snapshots/allowanceHolder_rfq_USDT-WETH.snap b/.forge-snapshots/allowanceHolder_rfq_USDT-WETH.snap index 087ccbf8e..4428b8f9e 100644 --- a/.forge-snapshots/allowanceHolder_rfq_USDT-WETH.snap +++ b/.forge-snapshots/allowanceHolder_rfq_USDT-WETH.snap @@ -1 +1 @@ -98213 \ No newline at end of file +98199 \ No newline at end of file diff --git a/.forge-snapshots/allowanceHolder_rfq_fixedFee_sellToken_DAI-WETH.snap b/.forge-snapshots/allowanceHolder_rfq_fixedFee_sellToken_DAI-WETH.snap index 4684ba6d7..3b1ee0d9c 100644 --- a/.forge-snapshots/allowanceHolder_rfq_fixedFee_sellToken_DAI-WETH.snap +++ b/.forge-snapshots/allowanceHolder_rfq_fixedFee_sellToken_DAI-WETH.snap @@ -1 +1 @@ -99176 \ No newline at end of file +99055 \ No newline at end of file diff --git a/.forge-snapshots/allowanceHolder_rfq_fixedFee_sellToken_USDC-WETH.snap b/.forge-snapshots/allowanceHolder_rfq_fixedFee_sellToken_USDC-WETH.snap index 3122c0dce..2d8e22d3c 100644 --- a/.forge-snapshots/allowanceHolder_rfq_fixedFee_sellToken_USDC-WETH.snap +++ b/.forge-snapshots/allowanceHolder_rfq_fixedFee_sellToken_USDC-WETH.snap @@ -1 +1 @@ -122824 \ No newline at end of file +122703 \ No newline at end of file diff --git a/.forge-snapshots/allowanceHolder_rfq_fixedFee_sellToken_USDT-WETH.snap b/.forge-snapshots/allowanceHolder_rfq_fixedFee_sellToken_USDT-WETH.snap index 2159c529d..f900925c3 100644 --- a/.forge-snapshots/allowanceHolder_rfq_fixedFee_sellToken_USDT-WETH.snap +++ b/.forge-snapshots/allowanceHolder_rfq_fixedFee_sellToken_USDT-WETH.snap @@ -1 +1 @@ -111400 \ No newline at end of file +111279 \ No newline at end of file diff --git a/.forge-snapshots/settler_curveTricrypto_USDC-WETH.snap b/.forge-snapshots/settler_curveTricrypto_USDC-WETH.snap index bca353886..ed2d83703 100644 --- a/.forge-snapshots/settler_curveTricrypto_USDC-WETH.snap +++ b/.forge-snapshots/settler_curveTricrypto_USDC-WETH.snap @@ -1 +1 @@ -231504 \ No newline at end of file +231583 \ No newline at end of file diff --git a/.forge-snapshots/settler_curveTricrypto_USDT-WETH.snap b/.forge-snapshots/settler_curveTricrypto_USDT-WETH.snap index b90ca1d84..f19759120 100644 --- a/.forge-snapshots/settler_curveTricrypto_USDT-WETH.snap +++ b/.forge-snapshots/settler_curveTricrypto_USDT-WETH.snap @@ -1 +1 @@ -243871 \ No newline at end of file +243950 \ No newline at end of file diff --git a/.forge-snapshots/settler_rfq_DAI-USDC.snap b/.forge-snapshots/settler_rfq_DAI-USDC.snap index 223a45b97..1083b7787 100644 --- a/.forge-snapshots/settler_rfq_DAI-USDC.snap +++ b/.forge-snapshots/settler_rfq_DAI-USDC.snap @@ -1 +1 @@ -171011 \ No newline at end of file +171005 \ No newline at end of file diff --git a/.forge-snapshots/settler_rfq_DAI-WETH.snap b/.forge-snapshots/settler_rfq_DAI-WETH.snap index b0131d2e3..2d5f5d1a2 100644 --- a/.forge-snapshots/settler_rfq_DAI-WETH.snap +++ b/.forge-snapshots/settler_rfq_DAI-WETH.snap @@ -1 +1 @@ -94944 \ No newline at end of file +94938 \ No newline at end of file diff --git a/.forge-snapshots/settler_rfq_USDC-DAI.snap b/.forge-snapshots/settler_rfq_USDC-DAI.snap index 223a45b97..1083b7787 100644 --- a/.forge-snapshots/settler_rfq_USDC-DAI.snap +++ b/.forge-snapshots/settler_rfq_USDC-DAI.snap @@ -1 +1 @@ -171011 \ No newline at end of file +171005 \ No newline at end of file diff --git a/.forge-snapshots/settler_rfq_USDC-USDT.snap b/.forge-snapshots/settler_rfq_USDC-USDT.snap index 1f33adf38..ddc33a222 100644 --- a/.forge-snapshots/settler_rfq_USDC-USDT.snap +++ b/.forge-snapshots/settler_rfq_USDC-USDT.snap @@ -1 +1 @@ -147923 \ No newline at end of file +147917 \ No newline at end of file diff --git a/.forge-snapshots/settler_rfq_USDC-WETH.snap b/.forge-snapshots/settler_rfq_USDC-WETH.snap index b713024ba..52041caac 100644 --- a/.forge-snapshots/settler_rfq_USDC-WETH.snap +++ b/.forge-snapshots/settler_rfq_USDC-WETH.snap @@ -1 +1 @@ -114418 \ No newline at end of file +114412 \ No newline at end of file diff --git a/.forge-snapshots/settler_rfq_USDT-WETH.snap b/.forge-snapshots/settler_rfq_USDT-WETH.snap index 547891d33..8f96ba576 100644 --- a/.forge-snapshots/settler_rfq_USDT-WETH.snap +++ b/.forge-snapshots/settler_rfq_USDT-WETH.snap @@ -1 +1 @@ -106056 \ No newline at end of file +106050 \ No newline at end of file diff --git a/README.md b/README.md index 4980fb711..d7fb23e47 100644 --- a/README.md +++ b/README.md @@ -723,7 +723,7 @@ comparison. | Curve | DEX | Pair | Gas | % | | ----------------- | --------------------- | --------- | ------ | ------- | -| Settler | CurveV2 Tricrypto VIP | USDC/WETH | 231504 | NaN% | +| Settler | CurveV2 Tricrypto VIP | USDC/WETH | 231583 | NaN% | | | | | | | | | | | | | | 0x V4 | Curve | USDT/WETH | 400419 | 0.00% | diff --git a/src/allowanceholder/AllowanceHolder.sol b/src/allowanceholder/AllowanceHolder.sol index a0c6c0ba3..a125b7b63 100644 --- a/src/allowanceholder/AllowanceHolder.sol +++ b/src/allowanceholder/AllowanceHolder.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity =0.8.25; +pragma solidity =0.8.28; import {AllowanceHolderBase} from "./AllowanceHolderBase.sol"; import {TransientStorage} from "./TransientStorage.sol"; @@ -7,20 +7,9 @@ import {TransientStorage} from "./TransientStorage.sol"; /// @custom:security-contact security@0x.org contract AllowanceHolder is TransientStorage, AllowanceHolderBase { constructor() { - require(address(this) == 0x0000000000001fF3684f28c67538d4D072C22734 || block.chainid == 31337); - } - - /// @inheritdoc AllowanceHolderBase - function exec(address operator, address token, uint256 amount, address payable target, bytes calldata data) - internal - override - returns (bytes memory) - { - (bytes memory result, address sender, TSlot allowance) = _exec(operator, token, amount, target, data); - // EIP-3074 seems unlikely - if (sender != tx.origin) { - _set(allowance, 0); + // Check that we're on a chain with transient storage support + assembly ("memory-safe") { + tstore(0x00, 0x00) } - return result; } } diff --git a/src/allowanceholder/AllowanceHolderBase.sol b/src/allowanceholder/AllowanceHolderBase.sol index b90fe7584..0c42dc629 100644 --- a/src/allowanceholder/AllowanceHolderBase.sol +++ b/src/allowanceholder/AllowanceHolderBase.sol @@ -1,11 +1,13 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.25; +pragma solidity =0.8.28; import {IAllowanceHolder} from "./IAllowanceHolder.sol"; import {IERC20} from "@forge-std/interfaces/IERC20.sol"; -import {SafeTransferLib} from "../vendor/SafeTransferLib_Solmate.sol"; +import {SafeTransferLib} from "../vendor/SafeTransferLib.sol"; import {CheckCall} from "../utils/CheckCall.sol"; import {FreeMemory} from "../utils/FreeMemory.sol"; +import {Panic} from "../utils/Panic.sol"; +import {Ternary} from "../utils/Ternary.sol"; import {TransientStorageLayout} from "./TransientStorageLayout.sol"; /// @notice Thrown when validating the target, avoiding executing against an ERC20 directly @@ -14,6 +16,16 @@ error ConfusedDeputy(); abstract contract AllowanceHolderBase is TransientStorageLayout, FreeMemory { using SafeTransferLib for IERC20; using CheckCall for address payable; + using Ternary for bool; + + address internal constant _MULTICALL = 0x00000000000000CF9E3c5A26621af382fA17f24f; + + constructor() { + assert( + (msg.sender == 0x4e59b44847b379578588920cA78FbF26c0B4956C && uint160(address(this)) >> 104 == 0) + || block.chainid == 31337 + ); + } function _rejectIfERC20(address payable maybeERC20, bytes calldata data) private view DANGEROUS_freeMemory { // We could just choose a random address for this check, but to make @@ -21,49 +33,64 @@ abstract contract AllowanceHolderBase is TransientStorageLayout, FreeMemory { // (e.g. tokens with blacklists), we choose to copy the first argument // out of `data` and mask it as an address. If there isn't enough // `data`, we use 0xdead instead. - address target; - if (data.length > 0x10) { - target = address(uint160(bytes20(data[0x10:]))); + + address target; // = address(uint160(bytes20(data[0x10:]))); + assembly ("memory-safe") { + target := calldataload(add(0x04, data.offset)) + // `shl(0x08, data.length)` can't overflow because we're going to + // `calldatacopy(..., data.length)` later. It would OOG. We check + // for underflow in `sub(data.length, 0x04)` later. + let mask := shr(shl(0x08, sub(data.length, 0x04)), not(0x00)) + // Zero the low bits of `target` if `data` is short. Dirty low bits + // are only ever possible with nonstandard encodings, like ERC-2771. + target := and(not(mask), target) + // Zero `target` if `sub(data.length, 0x04)` underflowed. + target := mul(lt(0x03, data.length), target) } - // EIP-1352 (not adopted) specifies 0xffff as the maximum precompile - if (target <= address(0xffff)) { - // 0xdead is a conventional burn address; we assume that it is not treated specially - target = address(0xdead); + + // EIP-1352 (not adopted) specifies 0xffff as the maximum precompile. + // 0xdead is a conventional burn address; we assume that it is not + // treated specially. + target = (target > address(0xffff)).ternary(target, address(0xdead)); + + bytes memory testData; // = abi.encodeCall(IERC20.balanceOf, target); + assembly ("memory-safe") { + testData := mload(0x40) + mstore(add(0x24, testData), target) + mstore(add(0x10, testData), 0x70a08231000000000000000000000000) // `IERC20.balanceOf.selector` with `target`'s padding + mstore(testData, 0x24) + mstore(0x40, add(0x60, testData)) } - bytes memory testData = abi.encodeCall(IERC20.balanceOf, target); - if (maybeERC20.checkCall(testData, 0x20)) revert ConfusedDeputy(); - } - function _msgSender() private view returns (address sender) { - if ((sender = msg.sender) == address(this)) { + if (maybeERC20.checkCall(testData, 0x20)) { assembly ("memory-safe") { - sender := shr(0x60, calldataload(sub(calldatasize(), 0x14))) + mstore(0x00, 0xe758b8d5) // Selector for `ConfusedDeputy()` + revert(0x1c, 0x04) } } } - /// @dev This virtual function provides the implementation for the function - /// of the same name in `IAllowanceHolder`. It is unimplemented in this - /// base contract to accommodate the customization required to support - /// both chains that have EIP-1153 (transient storage) and those that - /// don't. + function _msgSender() private view returns (address sender) { + assembly ("memory-safe") { + let isSelfForwarded := eq(caller(), address()) + let isMultiCallForwarded := and(lt(0x03, calldatasize()), eq(_MULTICALL, caller())) + sender := + xor( + caller(), + mul( + xor(caller(), shr(0x60, calldataload(sub(calldatasize(), 0x14)))), + or(isMultiCallForwarded, isSelfForwarded) + ) + ) + } + } + + /// @dev This function provides the implementation for the function of the + /// same name in `IAllowanceHolder`. The arguments and return value + /// have the same meaning as documented there. function exec(address operator, address token, uint256 amount, address payable target, bytes calldata data) - internal - virtual - returns (bytes memory result); - - /// @dev This is the majority of the implementation of IAllowanceHolder.exec - /// . The arguments have the same meaning as documented there. - /// @return result - /// @return sender The (possibly forwarded) message sender that is - /// requesting the allowance be set. Provided to avoid - /// duplicated computation in customized `exec` - /// @return allowance The slot where the ephemeral allowance is - /// stored. Provided to avoid duplicated computation in - /// customized `exec` - function _exec(address operator, address token, uint256 amount, address payable target, bytes calldata data) - internal - returns (bytes memory result, address sender, TSlot allowance) + private + returns (bytes memory result) { // This contract has no special privileges, except for the allowances it // holds. In order to prevent abusing those allowances, we prohibit @@ -71,41 +98,64 @@ abstract contract AllowanceHolderBase is TransientStorageLayout, FreeMemory { // contract that might be an ERC20. _rejectIfERC20(target, data); - sender = _msgSender(); - allowance = _ephemeralAllowance(operator, sender, token); - _set(allowance, amount); + address sender = _msgSender(); + TSlot allowanceSlot = _ephemeralAllowance(operator, sender, token); + _set(allowanceSlot, amount); // For gas efficiency we're omitting a bunch of checks here. Notably, // we're omitting the check that `address(this)` has sufficient value to // send (we know it does), and we're omitting the check that `target` // contains code (we already checked in `_rejectIfERC20`). assembly ("memory-safe") { + // Copy the payload from calldata into memory result := mload(0x40) calldatacopy(result, data.offset, data.length) - // ERC-2771 style msgSender forwarding https://eips.ethereum.org/EIPS/eip-2771 + + // ERC-2771 style `msgSender` forwarding https://eips.ethereum.org/EIPS/eip-2771 + // We do not append the forwarded sender if the payload has no selector mstore(add(result, data.length), shl(0x60, sender)) - let success := call(gas(), target, callvalue(), result, add(data.length, 0x14), 0x00, 0x00) - let ptr := add(result, 0x20) + let length := add(mul(0x14, lt(0x03, data.length)), data.length) + + // Perform the call + let success := call(gas(), target, callvalue(), result, length, 0x00, 0x00) + + // Copy returndata into memory; if it is a revert, bubble + let ptr := add(0x20, result) returndatacopy(ptr, 0x00, returndatasize()) switch success case 0 { revert(ptr, returndatasize()) } default { + // Wrap the returndata in a level of ABIEncoding mstore(result, returndatasize()) - mstore(0x40, add(ptr, returndatasize())) + mstore(0x40, add(returndatasize(), ptr)) } } + + _set(allowanceSlot, 0); } /// @dev This provides the implementation of the function of the same name - /// in `IAllowanceHolder`. - function transferFrom(address token, address owner, address recipient, uint256 amount) internal { - // msg.sender is the assumed and later validated operator - TSlot allowance = _ephemeralAllowance(msg.sender, owner, token); - // validation of the ephemeral allowance for operator, owner, token via - // uint underflow - _set(allowance, _get(allowance) - amount); + /// in `IAllowanceHolder`. The arguments have the same meaning as + /// documented there. + function transferFrom(address token, address owner, address recipient, uint256 amount) private { + // `msg.sender` is the assumed and later validated `operator`. + TSlot allowanceSlot = _ephemeralAllowance(msg.sender, owner, token); + uint256 allowanceValue = _get(allowanceSlot); + + // We validate the ephemeral allowance for the 3-tuple of `operator` + // (`msg.sender`), `owner`, and `token` by reverting on unsigned integer + // underflow. + if (allowanceValue < amount) { + Panic.panic(Panic.ARITHMETIC_OVERFLOW); + } + + // Update the ephemeral allowance + unchecked { + _set(allowanceSlot, allowanceValue - amount); + } + // `safeTransferFrom` does not check that `token` actually contains - // code. It is the responsibility of integrating code to check for that + // code. It is the responsibility of integrating code to check for that, // if vacuous success is a security concern. IERC20(token).safeTransferFrom(owner, recipient, amount); } @@ -136,7 +186,7 @@ abstract contract AllowanceHolderBase is TransientStorageLayout, FreeMemory { transferFrom(token, owner, recipient, amount); - // return true; + // `return true;` assembly ("memory-safe") { mstore(0x00, 0x01) return(0x00, 0x20) @@ -171,20 +221,33 @@ abstract contract AllowanceHolderBase is TransientStorageLayout, FreeMemory { bytes memory result = exec(operator, token, amount, target, data); - // return result; + // `return result;` assembly ("memory-safe") { let returndata := sub(result, 0x20) + // This is technically not "memory-safe", but manual examination + // of the compiled bytecode shows that it's OK. mstore(returndata, 0x20) - return(returndata, add(0x40, mload(result))) + + // Pad `returndata` to a multiple of 32 bytes. + let len := mload(result) + let m := and(0x1f, len) + if m { + mstore(add(add(0x20, result), len), 0x00) + len := add(sub(0x20, m), len) + } + + // Return the ABIEncoding of `result`. + return(returndata, add(0x40, len)) } } else if (selector == uint256(uint32(IERC20.balanceOf.selector))) { - // balanceOf(address) reverts with a single byte of returndata, - // making it more gas efficient to pass the `_rejectERC20` check + // `balanceOf(address)` returns a single byte of returndata, making + // it more gas efficient to pass the `_rejectERC20` check during + // recursive/reentrant calls. assembly ("memory-safe") { - revert(0x00, 0x01) + return(0x00, 0x01) } } else { - // emulate standard Solidity behavior + // Emulate standard Solidity behavior. assembly ("memory-safe") { revert(0x00, 0x00) } diff --git a/src/allowanceholder/AllowanceHolderOld.sol b/src/allowanceholder/AllowanceHolderOld.sol index 9762f2fdd..7bc372be2 100644 --- a/src/allowanceholder/AllowanceHolderOld.sol +++ b/src/allowanceholder/AllowanceHolderOld.sol @@ -1,28 +1,16 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.25; +pragma solidity =0.8.28; import {AllowanceHolderBase} from "./AllowanceHolderBase.sol"; import {TransientStorageMock} from "./TransientStorageMock.sol"; /// @custom:security-contact security@0x.org contract AllowanceHolder is TransientStorageMock, AllowanceHolderBase { - /// @inheritdoc AllowanceHolderBase - function exec(address operator, address token, uint256 amount, address payable target, bytes calldata data) - internal - override - returns (bytes memory) - { - (bytes memory result,, TSlot allowance) = _exec(operator, token, amount, target, data); - _set(allowance, 0); - return result; - } - // This is here as a deploy-time check that AllowanceHolder doesn't have any // state. If it did, it would interfere with TransientStorageMock. bytes32 private _sentinel; constructor() { - require(address(this) == 0x0000000000005E88410CcDFaDe4a5EfaE4b49562 || block.chainid == 31337); uint256 _sentinelSlot; assembly ("memory-safe") { _sentinelSlot := _sentinel.slot diff --git a/src/allowanceholder/TransientStorageLayout.sol b/src/allowanceholder/TransientStorageLayout.sol index 5dacb94d6..6e3ab5d0c 100644 --- a/src/allowanceholder/TransientStorageLayout.sol +++ b/src/allowanceholder/TransientStorageLayout.sol @@ -7,14 +7,15 @@ abstract contract TransientStorageLayout is TransientStorageBase { /// @dev The key for this ephemeral allowance is keccak256(abi.encodePacked(operator, owner, token)). function _ephemeralAllowance(address operator, address owner, address token) internal pure returns (TSlot r) { assembly ("memory-safe") { - let ptr := mload(0x40) + // This dirties the upper 8 bytes of the free memory pointer. These bytes must always be + // zero, otherwise we would OOM. mstore(0x28, token) mstore(0x14, owner) mstore(0x00, operator) // allowance slot is keccak256(abi.encodePacked(operator, owner, token)) r := keccak256(0x0c, 0x3c) // restore dirtied free pointer - mstore(0x40, ptr) + mstore(0x28, 0x00) } } } diff --git a/src/utils/CheckCall.sol b/src/utils/CheckCall.sol index bda31190c..eb86e5489 100644 --- a/src/utils/CheckCall.sol +++ b/src/utils/CheckCall.sol @@ -21,7 +21,7 @@ library CheckCall { assembly ("memory-safe") { let beforeGas { - let offset := add(data, 0x20) + let offset := add(0x20, data) let length := mload(data) beforeGas := gas() success := staticcall(gas(), target, offset, length, 0x00, 0x00) @@ -48,11 +48,12 @@ library CheckCall { // Check whether the call reverted due to out-of-gas. // https://eips.ethereum.org/EIPS/eip-150 // https://ronan.eth.limo/blog/ethereum-gas-dangers/ - // We apply the "all but one 64th" rule twice because `target` could - // plausibly be a proxy. We apply it only twice because we assume only a - // single level of indirection. - let remainingGas := shr(6, beforeGas) - remainingGas := add(remainingGas, shr(6, sub(beforeGas, remainingGas))) + // We apply the "all but one 64th" rule three times because `target` could + // plausibly be a proxy. We apply it only three times because we assume only + // two levels of indirection. + let remainingGas := shr(0x06, beforeGas) + remainingGas := add(remainingGas, shr(0x06, sub(beforeGas, remainingGas))) + remainingGas := add(remainingGas, shr(0x06, sub(beforeGas, remainingGas))) if iszero(lt(remainingGas, afterGas)) { // The call failed due to not enough gas left. We deliberately consume // all remaining gas with `invalid` (instead of `revert`) to make this diff --git a/src/vendor/SafeTransferLib_Solmate.sol b/src/vendor/SafeTransferLib_Solmate.sol deleted file mode 100644 index 6aebad031..000000000 --- a/src/vendor/SafeTransferLib_Solmate.sol +++ /dev/null @@ -1,119 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0-only -pragma solidity >=0.8.25; - -import {IERC20} from "@forge-std/interfaces/IERC20.sol"; - -/// @notice Safe ETH and ERC20 transfer library that gracefully handles missing return values. -/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/utils/SafeTransferLib.sol) -/// @dev Use with caution! Some functions in this library knowingly create dirty bits at the destination of the free memory pointer. -/// @dev Note that none of the functions in this library check that a token has code at all! That responsibility is delegated to the caller. -library SafeTransferLib { - uint32 private constant _TRANSFER_FROM_FAILED_SELECTOR = 0x7939f424; // bytes4(keccak256("TransferFromFailed()")) - uint32 private constant _TRANSFER_FAILED_SELECTOR = 0x90b8ec18; // bytes4(keccak256("TransferFailed()")) - uint32 private constant _APPROVE_FAILED_SELECTOR = 0x3e3f8f73; // bytes4(keccak256("ApproveFailed()")) - - /*////////////////////////////////////////////////////////////// - ETH OPERATIONS - //////////////////////////////////////////////////////////////*/ - - function safeTransferETH(address payable to, uint256 amount) internal { - assembly ("memory-safe") { - // Transfer the ETH and store if it succeeded or not. - if iszero(call(gas(), to, amount, 0, 0, 0, 0)) { - let freeMemoryPointer := mload(0x40) - returndatacopy(freeMemoryPointer, 0, returndatasize()) - revert(freeMemoryPointer, returndatasize()) - } - } - } - - /*////////////////////////////////////////////////////////////// - ERC20 OPERATIONS - //////////////////////////////////////////////////////////////*/ - - function safeTransferFrom(IERC20 token, address from, address to, uint256 amount) internal { - assembly ("memory-safe") { - // Get a pointer to some free memory. - let freeMemoryPointer := mload(0x40) - - // Write the abi-encoded calldata into memory, beginning with the function selector. - mstore(freeMemoryPointer, 0x23b872dd00000000000000000000000000000000000000000000000000000000) - mstore(add(freeMemoryPointer, 4), and(from, 0xffffffffffffffffffffffffffffffffffffffff)) // Append and mask the "from" argument. - mstore(add(freeMemoryPointer, 36), and(to, 0xffffffffffffffffffffffffffffffffffffffff)) // Append and mask the "to" argument. - mstore(add(freeMemoryPointer, 68), amount) // Append the "amount" argument. Masking not required as it's a full 32 byte type. - - // We use 100 because the length of our calldata totals up like so: 4 + 32 * 3. - // We use 0 and 32 to copy up to 32 bytes of return data into the scratch space. - if iszero(call(gas(), token, 0, freeMemoryPointer, 100, 0, 32)) { - returndatacopy(freeMemoryPointer, 0, returndatasize()) - revert(freeMemoryPointer, returndatasize()) - } - // We check that the call either returned exactly 1 (can't just be non-zero data), or had no - // return data. - if iszero(or(and(eq(mload(0), 1), gt(returndatasize(), 31)), iszero(returndatasize()))) { - mstore(0, _TRANSFER_FROM_FAILED_SELECTOR) - revert(0x1c, 0x04) - } - } - } - - function safeTransfer(IERC20 token, address to, uint256 amount) internal { - assembly ("memory-safe") { - // Get a pointer to some free memory. - let freeMemoryPointer := mload(0x40) - - // Write the abi-encoded calldata into memory, beginning with the function selector. - mstore(freeMemoryPointer, 0xa9059cbb00000000000000000000000000000000000000000000000000000000) - mstore(add(freeMemoryPointer, 4), and(to, 0xffffffffffffffffffffffffffffffffffffffff)) // Append and mask the "to" argument. - mstore(add(freeMemoryPointer, 36), amount) // Append the "amount" argument. Masking not required as it's a full 32 byte type. - - // We use 68 because the length of our calldata totals up like so: 4 + 32 * 2. - // We use 0 and 32 to copy up to 32 bytes of return data into the scratch space. - if iszero(call(gas(), token, 0, freeMemoryPointer, 68, 0, 32)) { - returndatacopy(freeMemoryPointer, 0, returndatasize()) - revert(freeMemoryPointer, returndatasize()) - } - // We check that the call either returned exactly 1 (can't just be non-zero data), or had no - // return data. - if iszero(or(and(eq(mload(0), 1), gt(returndatasize(), 31)), iszero(returndatasize()))) { - mstore(0, _TRANSFER_FAILED_SELECTOR) - revert(0x1c, 0x04) - } - } - } - - function safeApprove(IERC20 token, address to, uint256 amount) internal { - assembly ("memory-safe") { - // Get a pointer to some free memory. - let freeMemoryPointer := mload(0x40) - - // Write the abi-encoded calldata into memory, beginning with the function selector. - mstore(freeMemoryPointer, 0x095ea7b300000000000000000000000000000000000000000000000000000000) - mstore(add(freeMemoryPointer, 4), and(to, 0xffffffffffffffffffffffffffffffffffffffff)) // Append and mask the "to" argument. - mstore(add(freeMemoryPointer, 36), amount) // Append the "amount" argument. Masking not required as it's a full 32 byte type. - - // We use 68 because the length of our calldata totals up like so: 4 + 32 * 2. - // We use 0 and 32 to copy up to 32 bytes of return data into the scratch space. - if iszero(call(gas(), token, 0, freeMemoryPointer, 68, 0, 32)) { - returndatacopy(freeMemoryPointer, 0, returndatasize()) - revert(freeMemoryPointer, returndatasize()) - } - // We check that the call either returned exactly 1 (can't just be non-zero data), or had no - // return data. - if iszero(or(and(eq(mload(0), 1), gt(returndatasize(), 31)), iszero(returndatasize()))) { - mstore(0, _APPROVE_FAILED_SELECTOR) - revert(0x1c, 0x04) - } - } - } - - function safeApproveIfBelow(IERC20 token, address spender, uint256 amount) internal { - uint256 allowance = token.allowance(address(this), spender); - if (allowance < amount) { - if (allowance != 0) { - safeApprove(token, spender, 0); - } - safeApprove(token, spender, type(uint256).max); - } - } -} diff --git a/test/integration/BasePairTest.t.sol b/test/integration/BasePairTest.t.sol index b6ab6f244..45b376240 100644 --- a/test/integration/BasePairTest.t.sol +++ b/test/integration/BasePairTest.t.sol @@ -8,13 +8,14 @@ import {IERC20} from "@forge-std/interfaces/IERC20.sol"; import {ISignatureTransfer} from "@permit2/interfaces/ISignatureTransfer.sol"; import {Permit2Signature} from "../utils/Permit2Signature.sol"; +import {DeployAllowanceHolder} from "./DeployAllowanceHolder.t.sol"; import {SafeTransferLib} from "../../src/vendor/SafeTransferLib.sol"; import {MainnetDefaultFork} from "./BaseForkTest.t.sol"; import {UNIVERSAL_ROUTER} from "src/vendor/IUniswapUniversalRouter.sol"; -abstract contract BasePairTest is Test, GasSnapshot, Permit2Signature, MainnetDefaultFork { +abstract contract BasePairTest is Test, GasSnapshot, Permit2Signature, DeployAllowanceHolder, MainnetDefaultFork { using SafeTransferLib for IERC20; uint256 internal constant FROM_PRIVATE_KEY = 0x1337; @@ -62,6 +63,8 @@ abstract contract BasePairTest is Test, GasSnapshot, Permit2Signature, MainnetDe vm.label(MAKER, "MAKER"); vm.label(BURN_ADDRESS, "BURN"); + _deployAllowanceHolder(); + // Initialize addresses with non-zero balances // https://github.com/0xProject/0x-settler#gas-comparisons if (address(fromToken()).code.length != 0) { diff --git a/test/integration/DeployAllowanceHolder.t.sol b/test/integration/DeployAllowanceHolder.t.sol new file mode 100644 index 000000000..b25cc1114 --- /dev/null +++ b/test/integration/DeployAllowanceHolder.t.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.25; + +import {IAllowanceHolder} from "src/allowanceholder/IAllowanceHolder.sol"; + +import {Vm} from "@forge-std/Vm.sol"; + +contract DeployAllowanceHolder { + Vm private constant vm = Vm(address(uint160(uint256(keccak256("hevm cheat code"))))); + + IAllowanceHolder internal allowanceHolder; + + function _deployAllowanceHolder() internal returns (IAllowanceHolder) { + allowanceHolder = IAllowanceHolder(0x0000000000001fF3684f28c67538d4D072C22734); + vm.etch(address(allowanceHolder), vm.getDeployedCode("AllowanceHolder.sol:AllowanceHolder")); + vm.label(address(allowanceHolder), "AllowanceHolder"); + return allowanceHolder; + } +} diff --git a/test/integration/DodoV2PairTest.t.sol b/test/integration/DodoV2PairTest.t.sol index 988351782..acaeb98d8 100644 --- a/test/integration/DodoV2PairTest.t.sol +++ b/test/integration/DodoV2PairTest.t.sol @@ -10,7 +10,6 @@ import {ActionDataBuilder} from "../utils/ActionDataBuilder.sol"; import {MainnetSettler as Settler} from "src/chains/Mainnet/TakerSubmitted.sol"; import {Shim} from "./SettlerBasePairTest.t.sol"; -import {AllowanceHolder} from "src/allowanceholder/AllowanceHolder.sol"; import {IAllowanceHolder} from "src/allowanceholder/IAllowanceHolder.sol"; contract DodoV2PairTest is BasePairTest { @@ -19,7 +18,6 @@ contract DodoV2PairTest is BasePairTest { } Settler internal settler; - IAllowanceHolder internal allowanceHolder; uint256 private _amount; function setUp() public override { @@ -31,12 +29,9 @@ contract DodoV2PairTest is BasePairTest { safeApproveIfBelow(fromToken(), FROM, address(PERMIT2), amount()); warmPermit2Nonce(FROM); - allowanceHolder = IAllowanceHolder(0x0000000000001fF3684f28c67538d4D072C22734); - uint256 forkChainId = (new Shim()).chainId(); vm.chainId(31337); settler = new Settler(bytes20(0)); - vm.etch(address(allowanceHolder), address(new AllowanceHolder()).code); vm.chainId(forkChainId); // USDT is obnoxious about throwing errors, so let's check here before diff --git a/test/integration/SettlerBasePairTest.t.sol b/test/integration/SettlerBasePairTest.t.sol index 7b480387c..f66aa60e4 100644 --- a/test/integration/SettlerBasePairTest.t.sol +++ b/test/integration/SettlerBasePairTest.t.sol @@ -11,7 +11,6 @@ import {IERC20} from "@forge-std/interfaces/IERC20.sol"; import {LibBytes} from "../utils/LibBytes.sol"; import {SafeTransferLib} from "src/vendor/SafeTransferLib.sol"; -import {AllowanceHolder} from "src/allowanceholder/AllowanceHolder.sol"; import {IAllowanceHolder} from "src/allowanceholder/IAllowanceHolder.sol"; import {MainnetSettler as Settler} from "src/chains/Mainnet/TakerSubmitted.sol"; @@ -34,7 +33,6 @@ abstract contract SettlerBasePairTest is BasePairTest { uint256 internal constant PERMIT2_MAKER_NONCE = 1; Settler internal settler; - IAllowanceHolder internal allowanceHolder; IZeroEx internal ZERO_EX = IZeroEx(0xDef1C0ded9bec7F1a1670819833240f027b25EfF); function settlerInitCode() internal virtual returns (bytes memory) { @@ -51,14 +49,21 @@ abstract contract SettlerBasePairTest is BasePairTest { function setUp() public virtual override { super.setUp(); - allowanceHolder = IAllowanceHolder(0x0000000000001fF3684f28c67538d4D072C22734); uint256 forkChainId = (new Shim()).chainId(); vm.chainId(31337); + // Some derived tests have an indirect dependency on the nonce of the test + // contract. Previously, we deployed a mock contract here to get the code for + // AllowanceHolder, but now this is taken care of by DeployAllowanceHolder. So we use a + // dummy `CREATE` to advance the nonce. + assembly ("memory-safe") { + if iszero(create(0x00, 0x00, 0x00)) { + revert(0x00, 0x00) + } + } + settler = _deploySettler(); vm.label(address(settler), "Settler"); - vm.etch(address(allowanceHolder), address(new AllowanceHolder()).code); - vm.label(address(allowanceHolder), "AllowanceHolder"); vm.chainId(forkChainId); } diff --git a/test/integration/SettlerMetaTxnPairTest.t.sol b/test/integration/SettlerMetaTxnPairTest.t.sol index be6de03aa..84a5072f9 100644 --- a/test/integration/SettlerMetaTxnPairTest.t.sol +++ b/test/integration/SettlerMetaTxnPairTest.t.sol @@ -14,7 +14,6 @@ import {ActionDataBuilder} from "../utils/ActionDataBuilder.sol"; import {SafeTransferLib} from "src/vendor/SafeTransferLib.sol"; -import {AllowanceHolder} from "src/allowanceholder/AllowanceHolder.sol"; import {MainnetSettlerMetaTxn as SettlerMetaTxn} from "src/chains/Mainnet/MetaTxn.sol"; import {Settler} from "src/Settler.sol"; import {ISettlerActions} from "src/ISettlerActions.sol"; diff --git a/test/integration/UniV3CallbackPoC.t.sol b/test/integration/UniV3CallbackPoC.t.sol index 66f52a4be..23b7e5ffc 100644 --- a/test/integration/UniV3CallbackPoC.t.sol +++ b/test/integration/UniV3CallbackPoC.t.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.25; -import {AllowanceHolder} from "src/allowanceholder/AllowanceHolderOld.sol"; import {IAllowanceHolder} from "src/allowanceholder/IAllowanceHolder.sol"; import {MainnetSettler as Settler} from "src/chains/Mainnet/TakerSubmitted.sol"; import {ISettlerActions} from "src/ISettlerActions.sol"; @@ -16,6 +15,7 @@ import {uniswapV3MainnetFactory} from "src/core/univ3forks/UniswapV3.sol"; import {Utils} from "../unit/Utils.sol"; import {Permit2Signature} from "../utils/Permit2Signature.sol"; import {ActionDataBuilder} from "../utils/ActionDataBuilder.sol"; +import {DeployAllowanceHolder} from "./DeployAllowanceHolder.t.sol"; import {Test, console} from "@forge-std/Test.sol"; import {MockERC20} from "@solmate/test/utils/mocks/MockERC20.sol"; @@ -56,11 +56,10 @@ contract Shim { } } -contract UniV3CallbackPoC is Utils, Permit2Signature, MainnetDefaultFork { +contract UniV3CallbackPoC is Utils, Permit2Signature, DeployAllowanceHolder, MainnetDefaultFork { ISignatureTransfer permit2 = ISignatureTransfer(0x000000000022D473030F116dDEE9F6B43aC78BA3); bytes32 internal permit2Domain; - IAllowanceHolder ah; Settler settler; address pool; @@ -91,13 +90,7 @@ contract UniV3CallbackPoC is Utils, Permit2Signature, MainnetDefaultFork { permit2Domain = permit2.DOMAIN_SEPARATOR(); // Deploy AllowanceHolder - ah = IAllowanceHolder(0x0000000000001fF3684f28c67538d4D072C22734); - { - uint256 forkChainId = (new Shim()).chainId(); - vm.chainId(31337); - vm.etch(address(ah), address(new AllowanceHolder()).code); - vm.chainId(forkChainId); - } + _deployAllowanceHolder(); // Deploy Settler. { @@ -143,7 +136,7 @@ contract UniV3CallbackPoC is Utils, Permit2Signature, MainnetDefaultFork { MockERC20(dai).mint(alice, 100e18); vm.prank(alice); - MockERC20(dai).approve(address(ah), type(uint256).max); + MockERC20(dai).approve(address(allowanceHolder), type(uint256).max); vm.prank(alice); MockERC20(dai).approve(address(permit2), type(uint256).max); diff --git a/test/integration/VelodromePairTest.t.sol b/test/integration/VelodromePairTest.t.sol index 5c40e3773..1125f1f21 100644 --- a/test/integration/VelodromePairTest.t.sol +++ b/test/integration/VelodromePairTest.t.sol @@ -10,7 +10,6 @@ import {ActionDataBuilder} from "../utils/ActionDataBuilder.sol"; import {BaseSettler as Settler} from "src/chains/Base/TakerSubmitted.sol"; import {Shim} from "./SettlerBasePairTest.t.sol"; -import {AllowanceHolder} from "src/allowanceholder/AllowanceHolder.sol"; import {IAllowanceHolder} from "src/allowanceholder/IAllowanceHolder.sol"; contract VelodromePairTest is BasePairTest { @@ -19,7 +18,6 @@ contract VelodromePairTest is BasePairTest { } Settler internal settler; - IAllowanceHolder internal allowanceHolder; uint256 private _amount; function setUp() public override { @@ -31,12 +29,9 @@ contract VelodromePairTest is BasePairTest { safeApproveIfBelow(fromToken(), FROM, address(PERMIT2), amount()); warmPermit2Nonce(FROM); - allowanceHolder = IAllowanceHolder(0x0000000000001fF3684f28c67538d4D072C22734); - uint256 forkChainId = (new Shim()).chainId(); vm.chainId(31337); settler = new Settler(bytes20(0)); - vm.etch(address(allowanceHolder), address(new AllowanceHolder()).code); vm.chainId(forkChainId); // USDT is obnoxious about throwing errors, so let's check here before diff --git a/test/integration/WethWrapTest.t.sol b/test/integration/WethWrapTest.t.sol index b8c683284..99372154d 100644 --- a/test/integration/WethWrapTest.t.sol +++ b/test/integration/WethWrapTest.t.sol @@ -5,6 +5,7 @@ import {IERC20} from "@forge-std/interfaces/IERC20.sol"; import {ISettlerBase} from "src/interfaces/ISettlerBase.sol"; import {Test} from "@forge-std/Test.sol"; + import {WETH as WETHERC20} from "@solmate/tokens/WETH.sol"; import {AllowanceHolder} from "src/allowanceholder/AllowanceHolder.sol"; import {MainnetSettler as Settler} from "src/chains/Mainnet/TakerSubmitted.sol"; diff --git a/test/unit/AllowanceHolderUnitTest.t.sol b/test/unit/AllowanceHolderUnitTest.t.sol index 018e547a1..ed0bb99ef 100644 --- a/test/unit/AllowanceHolderUnitTest.t.sol +++ b/test/unit/AllowanceHolderUnitTest.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.25; +pragma solidity ^0.8.28; import {AllowanceHolder} from "src/allowanceholder/AllowanceHolderOld.sol"; import {IAllowanceHolder} from "src/allowanceholder/IAllowanceHolder.sol";