Merge pull request #52 from EspressoSystems/sync-v2.1.3

Sync v2.1.1 to v2.1.3

Author: jjeangal

audit-ci.jsonc

@@ -65,6 +65,8 @@
     // Server-Side Request Forgery in axios
     "GHSA-8hc4-vh64-cxmj",
     // Regular Expression Denial of Service (ReDoS) in micromatch
-    "GHSA-952p-6rrq-rcjv"
+    "GHSA-952p-6rrq-rcjv",
+    // cookie accepts cookie name, path, and domain with out of bounds characters
+    "GHSA-pxg6-pf52-xh8x"
   ]
 }

deploy/ExpressLaneAuction.js

@@ -0,0 +1,41 @@
+module.exports = async hre => {
+  const { deployments, getNamedAccounts } = hre
+  const { deploy } = deployments
+  const { deployer } = await getNamedAccounts()
+
+  await deploy('ExpressLaneAuction', {
+    from: deployer,
+    args: [],
+    proxy: {
+      proxyContract: 'TransparentUpgradeableProxy',
+      execute: {
+        init: {
+          methodName: 'initialize',
+          args: [{
+            _auctioneer: "0xeee584DA928A94950E177235EcB9A99bb655c7A0",
+            _biddingToken: "0x980B62Da83eFf3D4576C647993b0c1D7faf17c73", // WETH
+            _beneficiary: "0xeee584DA928A94950E177235EcB9A99bb655c7A0",
+            _roundTimingInfo: {
+              offsetTimestamp: 1727870000,
+              roundDurationSeconds: 60,
+              auctionClosingSeconds: 15,
+              reserveSubmissionSeconds: 15
+            },
+            _minReservePrice: ethers.utils.parseEther("0.00001"),
+            _auctioneerAdmin: "0xeee584DA928A94950E177235EcB9A99bb655c7A0",
+            _minReservePriceSetter: "0xeee584DA928A94950E177235EcB9A99bb655c7A0",
+            _reservePriceSetter: "0xeee584DA928A94950E177235EcB9A99bb655c7A0", 
+            _reservePriceSetterAdmin: "0xeee584DA928A94950E177235EcB9A99bb655c7A0",
+            _beneficiarySetter: "0xeee584DA928A94950E177235EcB9A99bb655c7A0",
+            _roundTimingSetter: "0xeee584DA928A94950E177235EcB9A99bb655c7A0",
+            _masterAdmin: "0xeee584DA928A94950E177235EcB9A99bb655c7A0"
+          }],
+        },
+      },
+      owner: deployer,
+    },
+  })
+}
+
+module.exports.tags = ['ExpressLaneAuction']
+module.exports.dependencies = []

foundry.toml

@@ -3,27 +3,30 @@ src = 'src/'
 out = 'out'
 libs = ['node_modules', 'lib']
 test = 'test/foundry'
-cache_path  = 'forge-cache/sol'
+cache_path = 'forge-cache/sol'
 optimizer = true
 optimizer_runs = 1
+allow_internal_expect_revert = true
 via_ir = true
 solc_version = '0.8.25'
 evm_version = 'cancun'
-fs_permissions = [{ access = "read", path = "./"}]
-remappings = ['ds-test/=lib/forge-std/lib/ds-test/src/',
-              'forge-std/=lib/forge-std/src/',
-              '@openzeppelin/contracts/=node_modules/@openzeppelin/contracts/',
-              '@openzeppelin/contracts-upgradeable/=node_modules/@openzeppelin/contracts-upgradeable/']
+fs_permissions = [{ access = "read", path = "./" }]
+remappings = [
+    'ds-test/=lib/forge-std/lib/ds-test/src/',
+    'forge-std/=lib/forge-std/src/',
+    '@openzeppelin/contracts/=node_modules/@openzeppelin/contracts/',
+    '@openzeppelin/contracts-upgradeable/=node_modules/@openzeppelin/contracts-upgradeable/',
+]
 
 [profile.yul]
 src = 'yul'
 out = 'out/yul'
 libs = ['node_modules', 'lib']
-cache_path  = 'forge-cache/yul'
+cache_path = 'forge-cache/yul'
 remappings = []
 auto_detect_remappings = false
 
 [fmt]
 number_underscore = 'thousands'
 line_length = 100
-# See more config options https://github.com/foundry-rs/foundry/tree/master/config
+# See more config options https://github.com/foundry-rs/foundry/tree/master/config

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@espressosystems/nitro-contracts",
-  "version": "2.1.1-beta.0",
+  "version": "2.1.3",
   "description": "Layer 2 precompiles and rollup for Arbitrum Nitro",
   "author": "Offchain Labs, Inc.",
   "license": "BUSL-1.1",
@@ -99,4 +99,4 @@
     "typescript": "^4.5.4",
     "yarn-audit-fix": "^10.0.7"
   }
-}
+}

slither.db.json

@@ -11,14 +11,15 @@ import {
     InsufficientSubmissionCost,
     L1Forked,
     NotAllowedOrigin,
-    NotOrigin,
+    NotCodelessOrigin,
     NotRollupOrOwner,
     RetryableData
 } from "../libraries/Error.sol";
 import "./IInboxBase.sol";
 import "./ISequencerInbox.sol";
 import "./IBridge.sol";
 import "../libraries/AddressAliasHelper.sol";
+import "../libraries/CallerChecker.sol";
 import "../libraries/DelegateCallAware.sol";
 import {
     L1MessageType_submitRetryableTx,
@@ -138,8 +139,7 @@ abstract contract AbsInbox is DelegateCallAware, PausableUpgradeable, IInboxBase
         returns (uint256)
     {
         if (_chainIdChanged()) revert L1Forked();
-        // solhint-disable-next-line avoid-tx-origin
-        if (msg.sender != tx.origin) revert NotOrigin();
+        if (!CallerChecker.isCallerCodelessOrigin()) revert NotCodelessOrigin();
         if (messageData.length > maxDataSize) revert DataTooLarge(messageData.length, maxDataSize);
         uint256 msgNum = _deliverToBridge(L2_MSG, msg.sender, keccak256(messageData), 0);
         emit InboxMessageDeliveredFromOrigin(msgNum);

src/bridge/AbsInbox.sol

@@ -62,6 +62,15 @@ contract ERC20Bridge is AbsBridge, IERC20Bridge {
         }
     }
 
+    /// @notice When upgrading a custom fee chain from v1.x.x to v2.1.2, nativeTokenDecimals must be set to 18.
+    ///         This is because v1.x.x contracts assume 18 decimals, but the ERC20Bridge does not have the decimals set in storage.
+    function postUpgradeInit() external onlyDelegated onlyProxyOwner {
+        // this zero check might save you from accidentally upgrading from v2.x.x to v2.1.2
+        // it will not save you if your native token is supposed to have 0 decimals
+        require(nativeTokenDecimals == 0, "NONZERO_NATIVE_TOKEN_DECIMALS");
+        nativeTokenDecimals = 18;
+    }
+
     /// @inheritdoc IERC20Bridge
     function enqueueDelayedMessage(
         uint8 kind,

src/bridge/ERC20Bridge.sol

@@ -118,6 +118,7 @@ contract Inbox is AbsInbox, IInbox {
         if (!_chainIdChanged()) revert NotForked();
         // solhint-disable-next-line avoid-tx-origin
         if (msg.sender != tx.origin) revert NotOrigin();
+        // no code size check required because we only want to know if msg.sender is an EOA to undo alias
         // arbos will discard unsigned tx with gas limit too large
         if (gasLimit > type(uint64).max) {
             revert GasLimitTooLarge();
@@ -152,6 +153,7 @@ contract Inbox is AbsInbox, IInbox {
         if (!_chainIdChanged()) revert NotForked();
         // solhint-disable-next-line avoid-tx-origin
         if (msg.sender != tx.origin) revert NotOrigin();
+        // no code size check required because we only want to know if msg.sender is an EOA to undo alias
         // arbos will discard unsigned tx with gas limit too large
         if (gasLimit > type(uint64).max) {
             revert GasLimitTooLarge();
@@ -185,6 +187,7 @@ contract Inbox is AbsInbox, IInbox {
         if (!_chainIdChanged()) revert NotForked();
         // solhint-disable-next-line avoid-tx-origin
         if (msg.sender != tx.origin) revert NotOrigin();
+        // no code size check required because we only want to know if msg.sender is an EOA to undo alias
         // arbos will discard unsigned tx with gas limit too large
         if (gasLimit > type(uint64).max) {
             revert GasLimitTooLarge();

src/bridge/Inbox.sol

@@ -21,6 +21,7 @@ import {
     NoSuchKeyset,
     NotForked,
     NotBatchPosterManager,
+    NotCodelessOrigin,
     RollupNotChanged,
     DataBlobsNotSupported,
     InitParamZero,
@@ -38,6 +39,7 @@ import "../rollup/IRollupLogic.sol";
 import "./Messages.sol";
 import "../precompiles/ArbGasInfo.sol";
 import "../precompiles/ArbSys.sol";
+import "../libraries/CallerChecker.sol";
 import "../libraries/IReader4844.sol";
 
 import {L1MessageType_batchPostingReport} from "../libraries/MessageTypes.sol";
@@ -370,8 +372,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
         uint256 newMessageCount,
         bytes memory quote
     ) external refundsGas(gasRefunder, IReader4844(address(0))) {
-        // solhint-disable-next-line avoid-tx-origin
-        if (msg.sender != tx.origin) revert NotOrigin();
+        if (!CallerChecker.isCallerCodelessOrigin()) revert NotCodelessOrigin();
         if (!isBatchPoster[msg.sender]) revert NotBatchPoster();
 
         // take keccak2256 hash of all the function arguments except the quote
@@ -483,10 +484,9 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
         if (hostChainIsArbitrum) revert DataBlobsNotSupported();
 
         // submit a batch spending report to refund the entity that produced the blob batch data
-        // same as using calldata, we only submit spending report if the caller is the origin of the tx
+        // same as using calldata, we only submit spending report if the caller is the origin and is codeless
         // such that one cannot "double-claim" batch posting refund in the same tx
-        // solhint-disable-next-line avoid-tx-origin
-        if (msg.sender == tx.origin && !isUsingFeeToken) {
+        if (CallerChecker.isCallerCodelessOrigin() && !isUsingFeeToken) {
             submitBatchSpendingReport(dataHash, seqMessageIndex, block.basefee, blobGas);
         }
     }

src/bridge/SequencerInbox.sol

@@ -0,0 +1,18 @@
+// Copyright 2021-2024, Offchain Labs, Inc.
+// For license information, see https://github.com/OffchainLabs/nitro-contracts/blob/main/LICENSE
+// SPDX-License-Identifier: BUSL-1.1
+
+pragma solidity ^0.8.0;
+
+library CallerChecker {
+    /**
+     * @notice A EIP-7702 safe check to ensure the caller is the origin and is codeless
+     * @return bool true if the caller is the origin and is codeless, false otherwise
+     * @dev    If the caller is the origin and is codeless, then msg.data is guaranteed to be same as tx.data
+     *         It also mean the caller would not be able to call a contract multiple times with the same transaction
+     */
+    function isCallerCodelessOrigin() internal view returns (bool) {
+        // solhint-disable-next-line avoid-tx-origin
+        return msg.sender == tx.origin && msg.sender.code.length == 0;
+    }
+}