Merge pull request #3195 from ethereum-optimism/fix/use-safecall

contracts-bedrock: use `MemorySafeCall`

.changeset/shaggy-ladybugs-arrive.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Use safecall that doesn't copy calldata

packages/contracts-bedrock/.gas-snapshot

@@ -4,7 +4,7 @@ GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 3544
 GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 118592)
 GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 354436)
 GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 118567)
-GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 45432)
+GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 45250)
 GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 68671)
 GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 74964)
 GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 35777)
@@ -37,9 +37,9 @@ L1CrossDomainMessenger_Test:testCannot_L1MessengerPause() (gas: 24515)
 L1CrossDomainMessenger_Test:testCannot_L1MessengerUnpause() (gas: 24562)
 L1CrossDomainMessenger_Test:test_L1MessengerMessageVersion() (gas: 24716)
 L1CrossDomainMessenger_Test:test_L1MessengerPause() (gas: 48053)
-L1CrossDomainMessenger_Test:test_L1MessengerRelayMessageFirstStuckSecondSucceeds() (gas: 201827)
-L1CrossDomainMessenger_Test:test_L1MessengerRelayMessageRevertsOnReentrancy() (gas: 195126)
-L1CrossDomainMessenger_Test:test_L1MessengerRelayMessageSucceeds() (gas: 77865)
+L1CrossDomainMessenger_Test:test_L1MessengerRelayMessageFirstStuckSecondSucceeds() (gas: 201405)
+L1CrossDomainMessenger_Test:test_L1MessengerRelayMessageRevertsOnReentrancy() (gas: 194907)
+L1CrossDomainMessenger_Test:test_L1MessengerRelayMessageSucceeds() (gas: 77654)
 L1CrossDomainMessenger_Test:test_L1MessengerRelayMessageToSystemContract() (gas: 68005)
 L1CrossDomainMessenger_Test:test_L1MessengerRelayShouldRevertIfPaused() (gas: 60526)
 L1CrossDomainMessenger_Test:test_L1MessengerReplayMessageWithValue() (gas: 38193)
@@ -47,14 +47,14 @@ L1CrossDomainMessenger_Test:test_L1MessengerSendMessage() (gas: 301590)
 L1CrossDomainMessenger_Test:test_L1MessengerTwiceSendMessage() (gas: 1492584)
 L1CrossDomainMessenger_Test:test_L1MessengerUnpause() (gas: 40948)
 L1CrossDomainMessenger_Test:test_L1MessengerXDomainSenderReverts() (gas: 24305)
-L1CrossDomainMessenger_Test:test_L1MessengerxDomainMessageSenderResets() (gas: 86378)
+L1CrossDomainMessenger_Test:test_L1MessengerxDomainMessageSenderResets() (gas: 86170)
 L1StandardBridge_Test:test_depositERC20() (gas: 579126)
 L1StandardBridge_Test:test_depositERC20To() (gas: 581333)
 L1StandardBridge_Test:test_depositETH() (gas: 373955)
 L1StandardBridge_Test:test_depositETHTo() (gas: 331091)
 L1StandardBridge_Test:test_finalizeBridgeERC20FailSendBack() (gas: 680116)
 L1StandardBridge_Test:test_finalizeERC20Withdrawal() (gas: 489836)
-L1StandardBridge_Test:test_finalizeETHWithdrawal() (gas: 64273)
+L1StandardBridge_Test:test_finalizeETHWithdrawal() (gas: 64091)
 L1StandardBridge_Test:test_initialize() (gas: 26334)
 L1StandardBridge_Test:test_onlyEOADepositERC20() (gas: 22376)
 L1StandardBridge_Test:test_onlyEOADepositETH() (gas: 40859)
@@ -62,17 +62,17 @@ L1StandardBridge_Test:test_onlyL2BridgeFinalizeERC20Withdrawal() (gas: 36268)
 L1StandardBridge_Test:test_onlyPortalFinalizeERC20Withdrawal() (gas: 35573)
 L1StandardBridge_Test:test_receive() (gas: 520573)
 L2CrossDomainMessenger_Test:testCannot_L2MessengerPause() (gas: 10860)
-L2CrossDomainMessenger_Test:test_L1MessengerRelayMessageRevertsOnReentrancy() (gas: 171954)
+L2CrossDomainMessenger_Test:test_L1MessengerRelayMessageRevertsOnReentrancy() (gas: 171735)
 L2CrossDomainMessenger_Test:test_L2MessengerMessageVersion() (gas: 8411)
 L2CrossDomainMessenger_Test:test_L2MessengerPause() (gas: 31797)
-L2CrossDomainMessenger_Test:test_L2MessengerRelayMessageFirstStuckSecondSucceeds() (gas: 172968)
-L2CrossDomainMessenger_Test:test_L2MessengerRelayMessageSucceeds() (gas: 57396)
+L2CrossDomainMessenger_Test:test_L2MessengerRelayMessageFirstStuckSecondSucceeds() (gas: 172546)
+L2CrossDomainMessenger_Test:test_L2MessengerRelayMessageSucceeds() (gas: 57185)
 L2CrossDomainMessenger_Test:test_L2MessengerRelayMessageToSystemContract() (gas: 36217)
 L2CrossDomainMessenger_Test:test_L2MessengerRelayShouldRevertIfPaused() (gas: 41682)
 L2CrossDomainMessenger_Test:test_L2MessengerSendMessage() (gas: 122833)
 L2CrossDomainMessenger_Test:test_L2MessengerTwiceSendMessage() (gas: 138556)
 L2CrossDomainMessenger_Test:test_L2MessengerXDomainSenderReverts() (gas: 10598)
-L2CrossDomainMessenger_Test:test_L2MessengerxDomainMessageSenderResets() (gas: 54889)
+L2CrossDomainMessenger_Test:test_L2MessengerxDomainMessageSenderResets() (gas: 54681)
 L2OutputOracleTest:testCannot_ProposeWithUnmatchedBlockhash() (gas: 26829)
 L2OutputOracleTest:testCannot_deleteL2Output_ifNotOwner() (gas: 25123)
 L2OutputOracleTest:testCannot_deleteL2Output_withWrongRoot() (gas: 91445)
@@ -138,12 +138,12 @@ OptimismPortalUpgradeable_Test:test_upgrading() (gas: 180632)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnInsufficientGas() (gas: 160788)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnInvalidOutputRootProof() (gas: 81241)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnRecentWithdrawal() (gas: 53001)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnReentrancy() (gas: 205566)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnReplay() (gas: 278941)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnReentrancy() (gas: 205354)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnReplay() (gas: 278729)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnSelfCall() (gas: 50539)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOninvalidWithdrawalProof() (gas: 148562)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_succeeds() (gas: 187013)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_targetFails() (gas: 289662)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_succeeds() (gas: 186801)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_targetFails() (gas: 289450)
 OptimismPortal_Test:test_OptimismPortalConstructor() (gas: 17341)
 OptimismPortal_Test:test_OptimismPortalContractCreationReverts() (gas: 14199)
 OptimismPortal_Test:test_OptimismPortalReceiveEth() (gas: 127534)

packages/contracts-bedrock/contracts/L1/OptimismPortal.sol

@@ -2,7 +2,7 @@
 pragma solidity 0.8.15;
 
 import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
-import { ExcessivelySafeCall } from "excessively-safe-call/src/ExcessivelySafeCall.sol";
+import { SafeCall } from "../libraries/SafeCall.sol";
 import { L2OutputOracle } from "./L2OutputOracle.sol";
 import { Types } from "../libraries/Types.sol";
 import { Hashing } from "../libraries/Hashing.sol";
@@ -195,16 +195,10 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
         // Set the l2Sender so contracts know who triggered this withdrawal on L2.
         l2Sender = _tx.sender;
 
-        // Trigger the call to the target contract. We use excessivelySafeCall because we don't
+        // Trigger the call to the target contract. We use SafeCall because we don't
         // care about the returndata and we don't want target contracts to be able to force this
-        // call to run out of gas.
-        (bool success, ) = ExcessivelySafeCall.excessivelySafeCall(
-            _tx.target,
-            _tx.gasLimit,
-            _tx.value,
-            0,
-            _tx.data
-        );
+        // call to run out of gas via a returndata bomb.
+        bool success = SafeCall.call(_tx.target, _tx.gasLimit, _tx.value, _tx.data);
 
         // Reset the l2Sender back to the default value.
         l2Sender = DEFAULT_L2_SENDER;

packages/contracts-bedrock/contracts/libraries/SafeCall.sol

+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.15;
+
+/**
+ * @title SafeCall
+ * @notice Perform low level safe calls
+ */
+library SafeCall {
+    /**
+     * @notice Perform a low level call without copying any returndata
+     *
+     * @param _target   Address to call
+     * @param _gas      Amount of gas to pass to the call
+     * @param _value    Amount of value to pass to the call
+     * @param _calldata Calldata to pass to the call
+     */
+    function call(
+        address _target,
+        uint256 _gas,
+        uint256 _value,
+        bytes memory _calldata
+    ) internal returns (bool) {
+        bool _success;
+        assembly {
+            _success := call(
+                _gas, // gas
+                _target, // recipient
+                _value, // ether value
+                add(_calldata, 0x20), // inloc
+                mload(_calldata), // inlen
+                0, // outloc
+                0 // outlen
+            )
+        }
+        return _success;
+    }
+}

packages/contracts-bedrock/contracts/test/SafeCall.t.sol

+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.15;
+
+import { CommonTest } from "./CommonTest.t.sol";
+import { SafeCall } from "../libraries/SafeCall.sol";
+
+contract SafeCall_Test is CommonTest {
+    function test_safeCall(
+        address from,
+        address to,
+        uint256 gas,
+        uint64 value,
+        bytes memory data
+    ) external {
+        vm.assume(from.balance == 0);
+        vm.assume(to.balance == 0);
+        // no precompiles
+        vm.assume(uint160(to) > 10);
+        // don't call the vm
+        vm.assume(to != address(vm));
+        vm.assume(from != address(vm));
+        // don't call the console
+        vm.assume(
+            to != address(0x000000000000000000636F6e736F6c652e6c6f67)
+        );
+        // don't call the create2 deployer
+        vm.assume(
+            to != address(0x4e59b44847b379578588920cA78FbF26c0B4956C)
+        );
+        // don't send funds to self
+        vm.assume(from != to);
+
+        assertEq(from.balance, 0, "from balance is 0");
+        vm.deal(from, value);
+        assertEq(from.balance, value, "from balance not dealt");
+
+        vm.expectCall(
+            to,
+            value,
+            data
+        );
+
+        vm.prank(from);
+        bool success = SafeCall.call(
+            to,
+            gas,
+            value,
+            data
+        );
+
+        assertEq(success, true, "call not successful");
+        assertEq(to.balance, value, "to balance received");
+        assertEq(from.balance, 0, "from balance not drained");
+    }
+}

packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol

@@ -10,7 +10,7 @@ import {
 import {
     ReentrancyGuardUpgradeable
 } from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
-import { ExcessivelySafeCall } from "excessively-safe-call/src/ExcessivelySafeCall.sol";
+import { SafeCall } from "../libraries/SafeCall.sol";
 import { Hashing } from "../libraries/Hashing.sol";
 import { Encoding } from "../libraries/Encoding.sol";
 
@@ -269,13 +269,7 @@ abstract contract CrossDomainMessenger is
         );
 
         xDomainMsgSender = _sender;
-        (bool success, ) = ExcessivelySafeCall.excessivelySafeCall(
-            _target,
-            gasleft() - RELAY_GAS_BUFFER,
-            _value,
-            0,
-            _message
-        );
+        bool success = SafeCall.call(_target, gasleft() - RELAY_GAS_BUFFER, _value, _message);
         xDomainMsgSender = DEFAULT_XDOMAIN_SENDER;
 
         if (success == true) {

packages/contracts-bedrock/contracts/universal/StandardBridge.sol

@@ -5,6 +5,7 @@ import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
 import { Address } from "@openzeppelin/contracts/utils/Address.sol";
 import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+import { SafeCall } from "../libraries/SafeCall.sol";
 import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import { IRemoteToken, IL1Token } from "./SupportedInterfaces.sol";
 import { CrossDomainMessenger } from "./CrossDomainMessenger.sol";
@@ -284,7 +285,7 @@ abstract contract StandardBridge is Initializable {
         require(_to != address(this), "StandardBridge: cannot send to self");
 
         emit ETHBridgeFinalized(_from, _to, _amount, _extraData);
-        (bool success, ) = _to.call{ value: _amount }(new bytes(0));
+        bool success = SafeCall.call(_to, gasleft(), _amount, hex"");
         require(success, "StandardBridge: ETH transfer failed");
     }
 

packages/contracts-bedrock/foundry.toml

@@ -8,7 +8,6 @@ remappings = [
   '@openzeppelin/contracts-upgradeable/=node_modules/@openzeppelin/contracts-upgradeable/',
   '@openzeppelin/contracts/=node_modules/@openzeppelin/contracts/',
   '@eth-optimism/contracts-periphery/=node_modules/@eth-optimism/contracts-periphery/contracts',
-  'excessively-safe-call/=node_modules/excessively-safe-call/',
   '@rari-capital/solmate/=node_modules/@rari-capital/solmate',
   'forge-std/=node_modules/forge-std/src',
   'ds-test/=node_modules/ds-test/src'

packages/contracts-bedrock/package.json

@@ -40,9 +40,7 @@
     "@eth-optimism/core-utils": "^0.9.3",
     "@openzeppelin/contracts": "4.7.3",
     "@openzeppelin/contracts-upgradeable": "4.7.3",
-    "@rari-capital/solmate": "https://github.com/rari-capital/solmate.git#8f9b23f8838670afda0fd8983f2c41e8037ae6bc",
     "ethers": "^5.6.8",
-    "excessively-safe-call": "https://github.com/nomad-xyz/ExcessivelySafeCall.git#81cd99ce3e69117d665d7601c330ea03b97acce0",
     "hardhat": "^2.9.6"
   },
   "devDependencies": {
@@ -55,6 +53,7 @@
     "@foundry-rs/easy-foundryup": "^0.1.3",
     "@nomiclabs/hardhat-ethers": "^2.0.0",
     "@nomiclabs/hardhat-waffle": "^2.0.0",
+    "@rari-capital/solmate": "https://github.com/rari-capital/solmate.git#8f9b23f8838670afda0fd8983f2c41e8037ae6bc",
     "@typechain/ethers-v5": "^10.1.0",
     "@typescript-eslint/eslint-plugin": "^5.26.0",
     "@typescript-eslint/parser": "^4.29.1",

yarn.lock

@@ -8608,10 +8608,6 @@ evp_bytestokey@^1.0.0, evp_bytestokey@^1.0.3:
     md5.js "^1.3.4"
     safe-buffer "^5.1.1"
 
-"excessively-safe-call@https://github.com/nomad-xyz/ExcessivelySafeCall.git#81cd99ce3e69117d665d7601c330ea03b97acce0":
-  version "0.0.1-rc.1"
-  resolved "https://github.com/nomad-xyz/ExcessivelySafeCall.git#81cd99ce3e69117d665d7601c330ea03b97acce0"
-
 execa@^0.7.0:
   version "0.7.0"
   resolved "https://registry.yarnpkg.com/execa/-/execa-0.7.0.tgz#944becd34cc41ee32a63a9faf27ad5a65fc59777"