From b307211a55779c88796c3f95259453f56e5b34a3 Mon Sep 17 00:00:00 2001 From: Anna Carroll Date: Fri, 2 Aug 2024 14:25:36 +0200 Subject: [PATCH] fix: interface compatibility with non-standard ERC20s (#71) --- .gas-snapshot | 42 ++++++++++++++++----------------- src/orders/OrderDestination.sol | 5 +++- src/orders/OrderOrigin.sol | 7 ++++-- src/passage/Passage.sol | 7 ++++-- src/passage/RollupPassage.sol | 5 +++- 5 files changed, 39 insertions(+), 27 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 92b3889..471ecd0 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,37 +1,37 @@ -OrderOriginPermit2Test:test_fillPermit2() (gas: 225289) -OrderOriginPermit2Test:test_fillPermit2_multi() (gas: 1019134) -OrderOriginPermit2Test:test_initiatePermit2() (gas: 235774) -OrderOriginPermit2Test:test_initiatePermit2_multi() (gas: 992096) -OrdersTest:test_fill_ERC20() (gas: 70559) -OrdersTest:test_fill_ETH() (gas: 68520) -OrdersTest:test_fill_both() (gas: 166795) -OrdersTest:test_fill_multiETH() (gas: 132141) +OrderOriginPermit2Test:test_fillPermit2() (gas: 225275) +OrderOriginPermit2Test:test_fillPermit2_multi() (gas: 1019098) +OrderOriginPermit2Test:test_initiatePermit2() (gas: 235756) +OrderOriginPermit2Test:test_initiatePermit2_multi() (gas: 992082) +OrdersTest:test_fill_ERC20() (gas: 71615) +OrdersTest:test_fill_ETH() (gas: 68524) +OrdersTest:test_fill_both() (gas: 167851) +OrdersTest:test_fill_multiETH() (gas: 132145) OrdersTest:test_fill_underflowETH() (gas: 115425) -OrdersTest:test_initiate_ERC20() (gas: 81636) +OrdersTest:test_initiate_ERC20() (gas: 82688) OrdersTest:test_initiate_ETH() (gas: 45150) -OrdersTest:test_initiate_both() (gas: 118911) -OrdersTest:test_initiate_multiERC20() (gas: 722642) +OrdersTest:test_initiate_both() (gas: 119963) +OrdersTest:test_initiate_multiERC20() (gas: 724742) OrdersTest:test_initiate_multiETH() (gas: 75538) OrdersTest:test_orderExpired() (gas: 28106) -OrdersTest:test_sweepERC20() (gas: 59721) +OrdersTest:test_sweepERC20() (gas: 60713) OrdersTest:test_sweepETH() (gas: 82348) OrdersTest:test_underflowETH() (gas: 63690) -PassagePermit2Test:test_disallowedEnterPermit2() (gas: 699630) -PassagePermit2Test:test_enterTokenPermit2() (gas: 145449) -PassageTest:test_configureEnter() (gas: 125771) -PassageTest:test_disallowedEnter() (gas: 56619) +PassagePermit2Test:test_disallowedEnterPermit2() (gas: 696817) +PassagePermit2Test:test_enterTokenPermit2() (gas: 145435) +PassageTest:test_configureEnter() (gas: 128989) +PassageTest:test_disallowedEnter() (gas: 57692) PassageTest:test_enter() (gas: 25519) -PassageTest:test_enterToken() (gas: 64397) -PassageTest:test_enterToken_defaultChain() (gas: 62979) +PassageTest:test_enterToken() (gas: 65469) +PassageTest:test_enterToken_defaultChain() (gas: 64051) PassageTest:test_enter_defaultChain() (gas: 24055) PassageTest:test_fallback() (gas: 21533) PassageTest:test_onlyTokenAdmin() (gas: 16881) PassageTest:test_receive() (gas: 21383) PassageTest:test_setUp() (gas: 17011) -PassageTest:test_withdraw() (gas: 59188) -RollupPassagePermit2Test:test_exitTokenPermit2() (gas: 129402) +PassageTest:test_withdraw() (gas: 60183) +RollupPassagePermit2Test:test_exitTokenPermit2() (gas: 129388) RollupPassageTest:test_exit() (gas: 22403) -RollupPassageTest:test_exitToken() (gas: 50232) +RollupPassageTest:test_exitToken() (gas: 51071) RollupPassageTest:test_fallback() (gas: 19949) RollupPassageTest:test_receive() (gas: 19844) TransactTest:test_configureGas() (gas: 22828) diff --git a/src/orders/OrderDestination.sol b/src/orders/OrderDestination.sol index 556ef31..14de9a6 100644 --- a/src/orders/OrderDestination.sol +++ b/src/orders/OrderDestination.sol @@ -4,9 +4,12 @@ pragma solidity ^0.8.24; import {OrdersPermit2} from "./OrdersPermit2.sol"; import {IOrders} from "./IOrders.sol"; import {IERC20} from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol"; /// @notice Contract capable of processing fulfillment of intent-based Orders. abstract contract OrderDestination is IOrders, OrdersPermit2 { + using SafeERC20 for IERC20; + /// @notice Emitted when Order Outputs are sent to their recipients. /// @dev NOTE that here, Output.chainId denotes the *origin* chainId. event Filled(Output[] outputs); @@ -53,7 +56,7 @@ abstract contract OrderDestination is IOrders, OrdersPermit2 { value -= outputs[i].amount; payable(outputs[i].recipient).transfer(outputs[i].amount); } else { - IERC20(outputs[i].token).transferFrom(msg.sender, outputs[i].recipient, outputs[i].amount); + IERC20(outputs[i].token).safeTransferFrom(msg.sender, outputs[i].recipient, outputs[i].amount); } } } diff --git a/src/orders/OrderOrigin.sol b/src/orders/OrderOrigin.sol index c805a68..9e3c7a2 100644 --- a/src/orders/OrderOrigin.sol +++ b/src/orders/OrderOrigin.sol @@ -4,9 +4,12 @@ pragma solidity ^0.8.24; import {OrdersPermit2} from "./OrdersPermit2.sol"; import {IOrders} from "./IOrders.sol"; import {IERC20} from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol"; /// @notice Contract capable of registering initiation of intent-based Orders. abstract contract OrderOrigin is IOrders, OrdersPermit2 { + using SafeERC20 for IERC20; + /// @notice Thrown when an Order is submitted with a deadline that has passed. error OrderExpired(); @@ -79,7 +82,7 @@ abstract contract OrderOrigin is IOrders, OrdersPermit2 { if (token == address(0)) { payable(recipient).transfer(amount); } else { - IERC20(token).transfer(recipient, amount); + IERC20(token).safeTransfer(recipient, amount); } emit Sweep(recipient, token, amount); } @@ -92,7 +95,7 @@ abstract contract OrderOrigin is IOrders, OrdersPermit2 { // this line should underflow if there's an attempt to spend more ETH than is attached to the transaction value -= inputs[i].amount; } else { - IERC20(inputs[i].token).transferFrom(msg.sender, address(this), inputs[i].amount); + IERC20(inputs[i].token).safeTransferFrom(msg.sender, address(this), inputs[i].amount); } } } diff --git a/src/passage/Passage.sol b/src/passage/Passage.sol index c39f803..b8a67da 100644 --- a/src/passage/Passage.sol +++ b/src/passage/Passage.sol @@ -4,9 +4,12 @@ pragma solidity ^0.8.24; import {PassagePermit2} from "./PassagePermit2.sol"; import {UsesPermit2} from "../UsesPermit2.sol"; import {IERC20} from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol"; /// @notice A contract deployed to Host chain that allows tokens to enter the rollup. contract Passage is PassagePermit2 { + using SafeERC20 for IERC20; + /// @notice The chainId of rollup that Ether will be sent to by default when entering the rollup via fallback() or receive(). uint256 public immutable defaultRollupChainId; @@ -90,7 +93,7 @@ contract Passage is PassagePermit2 { /// @param amount - The amount of tokens entering the rollup. function enterToken(uint256 rollupChainId, address rollupRecipient, address token, uint256 amount) public { // transfer tokens to this contract - IERC20(token).transferFrom(msg.sender, address(this), amount); + IERC20(token).safeTransferFrom(msg.sender, address(this), amount); // check and emit _enterToken(rollupChainId, rollupRecipient, token, amount); } @@ -127,7 +130,7 @@ contract Passage is PassagePermit2 { if (token == address(0)) { payable(recipient).transfer(amount); } else { - IERC20(token).transfer(recipient, amount); + IERC20(token).safeTransfer(recipient, amount); } emit Withdrawal(token, recipient, amount); } diff --git a/src/passage/RollupPassage.sol b/src/passage/RollupPassage.sol index 18fd0cd..3b97767 100644 --- a/src/passage/RollupPassage.sol +++ b/src/passage/RollupPassage.sol @@ -4,10 +4,13 @@ pragma solidity ^0.8.24; import {PassagePermit2} from "./PassagePermit2.sol"; import {UsesPermit2} from "../UsesPermit2.sol"; import {IERC20} from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol"; import {ERC20Burnable} from "openzeppelin-contracts/contracts/token/ERC20/extensions/ERC20Burnable.sol"; /// @notice Enables tokens to Exit the rollup. contract RollupPassage is PassagePermit2 { + using SafeERC20 for IERC20; + /// @notice Emitted when native Ether exits the rollup. /// @param hostRecipient - The *requested* recipient of tokens on the host chain. /// @param amount - The amount of Ether exiting the rollup. @@ -46,7 +49,7 @@ contract RollupPassage is PassagePermit2 { /// @custom:emits ExitToken function exitToken(address hostRecipient, address token, uint256 amount) external { // transfer tokens to this contract - IERC20(token).transferFrom(msg.sender, address(this), amount); + IERC20(token).safeTransferFrom(msg.sender, address(this), amount); // burn and emit _exitToken(hostRecipient, token, amount); }