use call - not transfer - to enable Gnosis Safes (#72)

* use call, not transfer, to include Gnosis Safes

* use Address library (with uncapped gas)

* add explicit reentrancy protection everywhere

* snapshot

Author: anna-carroll

.gas-snapshot

@@ -1,39 +1,40 @@
-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: 82688)
-OrdersTest:test_initiate_ETH() (gas: 45150)
-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: 60713)
-OrdersTest:test_sweepETH() (gas: 82348)
-OrdersTest:test_underflowETH() (gas: 63690)
-PassagePermit2Test:test_disallowedEnterPermit2() (gas: 696817)
-PassagePermit2Test:test_enterTokenPermit2() (gas: 145435)
-PassageTest:test_configureEnter() (gas: 128989)
-PassageTest:test_disallowedEnter() (gas: 57692)
+GnosisSafeTest:test_gnosis_receive() (gas: 15927)
+OrderOriginPermit2Test:test_fillPermit2() (gas: 225741)
+OrderOriginPermit2Test:test_fillPermit2_multi() (gas: 1019564)
+OrderOriginPermit2Test:test_initiatePermit2() (gas: 236222)
+OrderOriginPermit2Test:test_initiatePermit2_multi() (gas: 992548)
+OrdersTest:test_fill_ERC20() (gas: 72081)
+OrdersTest:test_fill_ETH() (gas: 69103)
+OrdersTest:test_fill_both() (gas: 168430)
+OrdersTest:test_fill_multiETH() (gas: 132837)
+OrdersTest:test_fill_underflowETH() (gas: 115826)
+OrdersTest:test_initiate_ERC20() (gas: 83154)
+OrdersTest:test_initiate_ETH() (gas: 45616)
+OrdersTest:test_initiate_both() (gas: 120429)
+OrdersTest:test_initiate_multiERC20() (gas: 725208)
+OrdersTest:test_initiate_multiETH() (gas: 76004)
+OrdersTest:test_orderExpired() (gas: 28394)
+OrdersTest:test_sweepERC20() (gas: 61179)
+OrdersTest:test_sweepETH() (gas: 83384)
+OrdersTest:test_underflowETH() (gas: 63978)
+PassagePermit2Test:test_disallowedEnterPermit2() (gas: 699905)
+PassagePermit2Test:test_enterTokenPermit2() (gas: 145901)
+PassageTest:test_configureEnter() (gas: 130009)
+PassageTest:test_disallowedEnter() (gas: 57980)
 PassageTest:test_enter() (gas: 25519)
-PassageTest:test_enterToken() (gas: 65469)
-PassageTest:test_enterToken_defaultChain() (gas: 64051)
+PassageTest:test_enterToken() (gas: 65935)
+PassageTest:test_enterToken_defaultChain() (gas: 64517)
 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: 60183)
-RollupPassagePermit2Test:test_exitTokenPermit2() (gas: 129388)
+PassageTest:test_fallback() (gas: 22170)
+PassageTest:test_onlyTokenAdmin() (gas: 17169)
+PassageTest:test_receive() (gas: 21487)
+PassageTest:test_setUp() (gas: 17000)
+PassageTest:test_withdraw() (gas: 60649)
+RollupPassagePermit2Test:test_exitTokenPermit2() (gas: 129854)
 RollupPassageTest:test_exit() (gas: 22403)
-RollupPassageTest:test_exitToken() (gas: 51071)
-RollupPassageTest:test_fallback() (gas: 19949)
-RollupPassageTest:test_receive() (gas: 19844)
+RollupPassageTest:test_exitToken() (gas: 51444)
+RollupPassageTest:test_fallback() (gas: 20586)
+RollupPassageTest:test_receive() (gas: 19948)
 TransactTest:test_configureGas() (gas: 22828)
 TransactTest:test_enterTransact() (gas: 103973)
 TransactTest:test_onlyGasAdmin() (gas: 8810)

src/orders/OrderDestination.sol

@@ -5,10 +5,13 @@ 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";
+import {Address} from "openzeppelin-contracts/contracts/utils/Address.sol";
+import {ReentrancyGuardTransient} from "openzeppelin-contracts/contracts/utils/ReentrancyGuardTransient.sol";
 
 /// @notice Contract capable of processing fulfillment of intent-based Orders.
-abstract contract OrderDestination is IOrders, OrdersPermit2 {
+abstract contract OrderDestination is IOrders, OrdersPermit2, ReentrancyGuardTransient {
     using SafeERC20 for IERC20;
+    using Address for address payable;
 
     /// @notice Emitted when Order Outputs are sent to their recipients.
     /// @dev NOTE that here, Output.chainId denotes the *origin* chainId.
@@ -19,7 +22,7 @@ abstract contract OrderDestination is IOrders, OrdersPermit2 {
     /// @dev NOTE that here, Output.chainId denotes the *origin* chainId.
     /// @param outputs - The Outputs to be transferred.
     /// @custom:emits Filled
-    function fill(Output[] memory outputs) external payable {
+    function fill(Output[] memory outputs) external payable nonReentrant {
         // transfer outputs
         _transferOutputs(outputs);
 
@@ -37,7 +40,7 @@ abstract contract OrderDestination is IOrders, OrdersPermit2 {
     /// @param outputs - The Outputs to be transferred. signed over via permit2 witness.
     /// @param permit2 - the permit2 details, signer, and signature.
     /// @custom:emits Filled
-    function fillPermit2(Output[] memory outputs, OrdersPermit2.Permit2Batch calldata permit2) external {
+    function fillPermit2(Output[] memory outputs, OrdersPermit2.Permit2Batch calldata permit2) external nonReentrant {
         // transfer all tokens to the Output recipients via permit2 (includes check on nonce & deadline)
         _permitWitnessTransferFrom(
             outputWitness(outputs), _fillTransferDetails(outputs, permit2.permit.permitted), permit2
@@ -54,7 +57,7 @@ abstract contract OrderDestination is IOrders, OrdersPermit2 {
             if (outputs[i].token == address(0)) {
                 // this line should underflow if there's an attempt to spend more ETH than is attached to the transaction
                 value -= outputs[i].amount;
-                payable(outputs[i].recipient).transfer(outputs[i].amount);
+                payable(outputs[i].recipient).sendValue(outputs[i].amount);
             } else {
                 IERC20(outputs[i].token).safeTransferFrom(msg.sender, outputs[i].recipient, outputs[i].amount);
             }

src/orders/OrderOrigin.sol

@@ -5,10 +5,13 @@ 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";
+import {Address} from "openzeppelin-contracts/contracts/utils/Address.sol";
+import {ReentrancyGuardTransient} from "openzeppelin-contracts/contracts/utils/ReentrancyGuardTransient.sol";
 
 /// @notice Contract capable of registering initiation of intent-based Orders.
-abstract contract OrderOrigin is IOrders, OrdersPermit2 {
+abstract contract OrderOrigin is IOrders, OrdersPermit2, ReentrancyGuardTransient {
     using SafeERC20 for IERC20;
+    using Address for address payable;
 
     /// @notice Thrown when an Order is submitted with a deadline that has passed.
     error OrderExpired();
@@ -36,7 +39,7 @@ abstract contract OrderOrigin is IOrders, OrdersPermit2 {
     /// @param outputs - The token amounts that must be received on their target chain(s) in order for the Order to be executed.
     /// @custom:reverts OrderExpired if the deadline has passed.
     /// @custom:emits Order if the transaction mines.
-    function initiate(uint256 deadline, Input[] memory inputs, Output[] memory outputs) external payable {
+    function initiate(uint256 deadline, Input[] memory inputs, Output[] memory outputs) external payable nonReentrant {
         // check that the deadline hasn't passed
         if (block.timestamp > deadline) revert OrderExpired();
 
@@ -59,7 +62,7 @@ abstract contract OrderOrigin is IOrders, OrdersPermit2 {
         address tokenRecipient,
         Output[] memory outputs,
         OrdersPermit2.Permit2Batch calldata permit2
-    ) external {
+    ) external nonReentrant {
         // transfer all tokens to the tokenRecipient via permit2 (includes check on nonce & deadline)
         _permitWitnessTransferFrom(
             outputWitness(outputs), _initiateTransferDetails(tokenRecipient, permit2.permit.permitted), permit2
@@ -77,10 +80,10 @@ abstract contract OrderOrigin is IOrders, OrdersPermit2 {
     /// @param token - The token to transfer.
     /// @custom:emits Sweep
     /// @custom:reverts OnlyBuilder if called by non-block builder
-    function sweep(address recipient, address token, uint256 amount) external {
+    function sweep(address recipient, address token, uint256 amount) external nonReentrant {
         // send ETH or tokens
         if (token == address(0)) {
-            payable(recipient).transfer(amount);
+            payable(recipient).sendValue(amount);
         } else {
             IERC20(token).safeTransfer(recipient, amount);
         }

src/passage/Passage.sol

@@ -5,10 +5,13 @@ 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 {Address} from "openzeppelin-contracts/contracts/utils/Address.sol";
+import {ReentrancyGuardTransient} from "openzeppelin-contracts/contracts/utils/ReentrancyGuardTransient.sol";
 
 /// @notice A contract deployed to Host chain that allows tokens to enter the rollup.
-contract Passage is PassagePermit2 {
+contract Passage is PassagePermit2, ReentrancyGuardTransient {
     using SafeERC20 for IERC20;
+    using Address for address payable;
 
     /// @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;
@@ -91,7 +94,10 @@ contract Passage is PassagePermit2 {
     /// @param rollupRecipient - The recipient of tokens on the rollup.
     /// @param token - The host chain address of the token entering the rollup.
     /// @param amount - The amount of tokens entering the rollup.
-    function enterToken(uint256 rollupChainId, address rollupRecipient, address token, uint256 amount) public {
+    function enterToken(uint256 rollupChainId, address rollupRecipient, address token, uint256 amount)
+        public
+        nonReentrant
+    {
         // transfer tokens to this contract
         IERC20(token).safeTransferFrom(msg.sender, address(this), amount);
         // check and emit
@@ -110,6 +116,7 @@ contract Passage is PassagePermit2 {
     /// @param permit2 - The Permit2 information, including token & amount.
     function enterTokenPermit2(uint256 rollupChainId, address rollupRecipient, PassagePermit2.Permit2 calldata permit2)
         external
+        nonReentrant
     {
         // transfer tokens to this contract via permit2
         _permitWitnessTransferFrom(enterWitness(rollupChainId, rollupRecipient), permit2);
@@ -125,10 +132,10 @@ contract Passage is PassagePermit2 {
 
     /// @notice Allows the admin to withdraw ETH or ERC20 tokens from the contract.
     /// @dev Only the admin can call this function.
-    function withdraw(address token, address recipient, uint256 amount) external {
+    function withdraw(address token, address recipient, uint256 amount) external nonReentrant {
         if (msg.sender != tokenAdmin) revert OnlyTokenAdmin();
         if (token == address(0)) {
-            payable(recipient).transfer(amount);
+            payable(recipient).sendValue(amount);
         } else {
             IERC20(token).safeTransfer(recipient, amount);
         }

src/passage/RollupPassage.sol

@@ -6,9 +6,10 @@ 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";
+import {ReentrancyGuardTransient} from "openzeppelin-contracts/contracts/utils/ReentrancyGuardTransient.sol";
 
 /// @notice Enables tokens to Exit the rollup.
-contract RollupPassage is PassagePermit2 {
+contract RollupPassage is PassagePermit2, ReentrancyGuardTransient {
     using SafeERC20 for IERC20;
 
     /// @notice Emitted when native Ether exits the rollup.
@@ -47,7 +48,7 @@ contract RollupPassage is PassagePermit2 {
     /// @param token - The rollup address of the token exiting the rollup.
     /// @param amount - The amount of tokens exiting the rollup.
     /// @custom:emits ExitToken
-    function exitToken(address hostRecipient, address token, uint256 amount) external {
+    function exitToken(address hostRecipient, address token, uint256 amount) external nonReentrant {
         // transfer tokens to this contract
         IERC20(token).safeTransferFrom(msg.sender, address(this), amount);
         // burn and emit
@@ -58,7 +59,7 @@ contract RollupPassage is PassagePermit2 {
     /// @param hostRecipient - The *requested* recipient of tokens on the host chain.
     /// @param permit2 - The Permit2 information, including token & amount.
     /// @custom:emits ExitToken
-    function exitTokenPermit2(address hostRecipient, PassagePermit2.Permit2 calldata permit2) external {
+    function exitTokenPermit2(address hostRecipient, PassagePermit2.Permit2 calldata permit2) external nonReentrant {
         // transfer tokens to this contract
         _permitWitnessTransferFrom(exitWitness(hostRecipient), permit2);
         // burn and emit

test/Passage.t.sol

@@ -8,9 +8,12 @@ import {RollupPassage} from "../src/passage/RollupPassage.sol";
 import {TestERC20} from "./Helpers.t.sol";
 import {ERC20} from "openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
 import {ERC20Burnable} from "openzeppelin-contracts/contracts/token/ERC20/extensions/ERC20Burnable.sol";
+import {Address} from "openzeppelin-contracts/contracts/utils/Address.sol";
 import {Test, console2} from "forge-std/Test.sol";
 
 contract PassageTest is Test {
+    using Address for address payable;
+
     Passage public target;
     address token;
     address newToken;
@@ -113,13 +116,13 @@ contract PassageTest is Test {
     function test_receive() public {
         vm.expectEmit();
         emit Enter(target.defaultRollupChainId(), address(this), amount);
-        address(target).call{value: amount}("");
+        payable(address(target)).sendValue(amount);
     }
 
     function test_fallback() public {
         vm.expectEmit();
         emit Enter(target.defaultRollupChainId(), address(this), amount);
-        address(target).call{value: amount}("0xabcd");
+        payable(address(target)).functionCallWithValue("0xabcd", amount);
     }
 
     function test_enter() public {
@@ -163,6 +166,8 @@ contract PassageTest is Test {
 }
 
 contract RollupPassageTest is Test {
+    using Address for address payable;
+
     RollupPassage public target;
     address token;
     address recipient = address(0x123);
@@ -185,13 +190,13 @@ contract RollupPassageTest is Test {
     function test_receive() public {
         vm.expectEmit();
         emit Exit(address(this), amount);
-        address(target).call{value: amount}("");
+        payable(address(target)).sendValue(amount);
     }
 
     function test_fallback() public {
         vm.expectEmit();
         emit Exit(address(this), amount);
-        address(target).call{value: amount}("0xabcd");
+        payable(address(target)).functionCallWithValue("0xabcd", amount);
     }
 
     function test_exit() public {

test/Safe.t.sol

@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.24;
+
+// utils
+import {Test, console2} from "forge-std/Test.sol";
+
+contract GnosisSafeTest is Test {
+    function setUp() public {
+        vm.createSelectFork("https://ethereum-rpc.publicnode.com");
+    }
+
+    // NOTE: this test fails if 4000 gas is provided. seems 4100 is approx the minimum.
+    function test_gnosis_receive() public {
+        payable(address(0x7c68c42De679ffB0f16216154C996C354cF1161B)).call{value: 1 ether, gas: 4100}("");
+    }
+}