use call, not transfer, to include Gnosis Safes

Author: anna-carroll

.gas-snapshot

@@ -1,22 +1,23 @@
-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)
+GnosisSafeTest:test_gnosis_receive() (gas: 15927)
+OrderOriginPermit2Test:test_fillPermit2() (gas: 225741)
+OrderOriginPermit2Test:test_fillPermit2_multi() (gas: 1016764)
+OrderOriginPermit2Test:test_initiatePermit2() (gas: 236222)
+OrderOriginPermit2Test:test_initiatePermit2_multi() (gas: 992548)
+OrdersTest:test_fill_ERC20() (gas: 72081)
+OrdersTest:test_fill_ETH() (gas: 69017)
+OrdersTest:test_fill_both() (gas: 168344)
+OrdersTest:test_fill_multiETH() (gas: 132665)
+OrdersTest:test_fill_underflowETH() (gas: 115740)
+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: 83304)
+OrdersTest:test_underflowETH() (gas: 63978)
+PassagePermit2Test:test_disallowedEnterPermit2() (gas: 699617)
 PassagePermit2Test:test_enterTokenPermit2() (gas: 145435)
 PassageTest:test_configureEnter() (gas: 128989)
 PassageTest:test_disallowedEnter() (gas: 57692)

src/orders/IOrders.sol

@@ -2,6 +2,9 @@
 pragma solidity ^0.8.24;
 
 interface IOrders {
+    /// @notice Thrown when a transfer of Ether fails.
+    error EthTransferFailed();
+
     /// @notice Tokens sent by the swapper as inputs to the order
     /// @dev From ERC-7683
     struct Input {

src/orders/OrderDestination.sol

@@ -5,9 +5,10 @@ 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 {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;
 
     /// @notice Emitted when Order Outputs are sent to their recipients.
@@ -19,7 +20,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 +38,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 +55,8 @@ 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);
+                (bool success,) = payable(outputs[i].recipient).call{value: outputs[i].amount, gas: 5000}("");
+                if (!success) revert EthTransferFailed();
             } else {
                 IERC20(outputs[i].token).safeTransferFrom(msg.sender, outputs[i].recipient, outputs[i].amount);
             }

src/orders/OrderOrigin.sol

@@ -5,9 +5,10 @@ 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 {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;
 
     /// @notice Thrown when an Order is submitted with a deadline that has passed.
@@ -36,7 +37,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 +60,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 +78,11 @@ 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);
+            (bool success,) = payable(recipient).call{value: amount}("");
+            if (!success) revert EthTransferFailed();
         } else {
             IERC20(token).safeTransfer(recipient, amount);
         }

src/passage/Passage.sol

@@ -25,6 +25,9 @@ contract Passage is PassagePermit2 {
     /// @notice Thrown when attempting to enter the rollup with an ERC20 token that is not currently allowed.
     error DisallowedEnter(address token);
 
+    /// @notice Thrown when a transfer of Ether fails.
+    error EthTransferFailed();
+
     /// @notice Emitted when Ether enters the rollup.
     /// @param rollupChainId - The chainId of the destination rollup.
     /// @param rollupRecipient - The recipient of Ether on the rollup.
@@ -128,7 +131,8 @@ contract Passage is PassagePermit2 {
     function withdraw(address token, address recipient, uint256 amount) external {
         if (msg.sender != tokenAdmin) revert OnlyTokenAdmin();
         if (token == address(0)) {
-            payable(recipient).transfer(amount);
+            (bool success,) = payable(recipient).call{value: amount, gas: 5000}("");
+            if (!success) revert EthTransferFailed();
         } else {
             IERC20(token).safeTransfer(recipient, amount);
         }

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}("");
+    }
+}