use call, not transfer, to include Gnosis Safes

Author: anna-carroll

.gas-snapshot

@@ -1,21 +1,22 @@
-OrderOriginPermit2Test:test_fillPermit2() (gas: 225289)
-OrderOriginPermit2Test:test_fillPermit2_multi() (gas: 1019134)
-OrderOriginPermit2Test:test_initiatePermit2() (gas: 235752)
-OrderOriginPermit2Test:test_initiatePermit2_multi() (gas: 989274)
-OrdersTest:test_fill_ERC20() (gas: 70537)
-OrdersTest:test_fill_ETH() (gas: 68498)
-OrdersTest:test_fill_both() (gas: 166773)
-OrdersTest:test_fill_multiETH() (gas: 132119)
-OrdersTest:test_fill_underflowETH() (gas: 115403)
-OrdersTest:test_initiate_ERC20() (gas: 81636)
-OrdersTest:test_initiate_ETH() (gas: 45150)
-OrdersTest:test_initiate_both() (gas: 118911)
-OrdersTest:test_initiate_multiERC20() (gas: 722642)
-OrdersTest:test_initiate_multiETH() (gas: 75538)
-OrdersTest:test_orderExpired() (gas: 28106)
-OrdersTest:test_sweepERC20() (gas: 60491)
-OrdersTest:test_sweepETH() (gas: 82186)
-OrdersTest:test_underflowETH() (gas: 63690)
+GnosisSafeTest:test_gnosis_receive() (gas: 15927)
+OrderOriginPermit2Test:test_fillPermit2() (gas: 225755)
+OrderOriginPermit2Test:test_fillPermit2_multi() (gas: 1016800)
+OrderOriginPermit2Test:test_initiatePermit2() (gas: 236218)
+OrderOriginPermit2Test:test_initiatePermit2_multi() (gas: 989740)
+OrdersTest:test_fill_ERC20() (gas: 71003)
+OrdersTest:test_fill_ETH() (gas: 68991)
+OrdersTest:test_fill_both() (gas: 167266)
+OrdersTest:test_fill_multiETH() (gas: 132639)
+OrdersTest:test_fill_underflowETH() (gas: 115718)
+OrdersTest:test_initiate_ERC20() (gas: 82102)
+OrdersTest:test_initiate_ETH() (gas: 45616)
+OrdersTest:test_initiate_both() (gas: 119377)
+OrdersTest:test_initiate_multiERC20() (gas: 723108)
+OrdersTest:test_initiate_multiETH() (gas: 76004)
+OrdersTest:test_orderExpired() (gas: 28394)
+OrdersTest:test_sweepERC20() (gas: 60957)
+OrdersTest:test_sweepETH() (gas: 83142)
+OrdersTest:test_underflowETH() (gas: 63978)
 PassagePermit2Test:test_disallowedEnterPermit2() (gas: 699630)
 PassagePermit2Test:test_enterTokenPermit2() (gas: 145449)
 PassageTest:test_configureEnter() (gas: 125771)

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

@@ -4,9 +4,10 @@ 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 {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 {
     /// @notice Emitted when Order Outputs are sent to their recipients.
     /// @dev NOTE that here, Output.chainId denotes the *origin* chainId.
     event Filled(Output[] outputs);
@@ -16,7 +17,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);
 
@@ -34,7 +35,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
@@ -51,7 +52,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).transferFrom(msg.sender, outputs[i].recipient, outputs[i].amount);
             }

src/orders/OrderOrigin.sol

@@ -4,9 +4,10 @@ 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 {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 {
     /// @notice Thrown when an Order is submitted with a deadline that has passed.
     error OrderExpired();
 
@@ -33,7 +34,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();
 
@@ -56,7 +57,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
@@ -74,12 +75,13 @@ 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) external {
+    function sweep(address recipient, address token) external nonReentrant {
         // send ETH or tokens
         uint256 balance;
         if (token == address(0)) {
             balance = address(this).balance;
-            payable(recipient).transfer(balance);
+            (bool success,) = payable(recipient).call{value: balance}("");
+            if (!success) revert EthTransferFailed();
         } else {
             balance = IERC20(token).balanceOf(address(this));
             IERC20(token).transfer(recipient, balance);

src/passage/Passage.sol

@@ -22,6 +22,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.
@@ -125,7 +128,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).transfer(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}("");
+    }
+}