Use transient storage in Outbox and Bridge

hardhat.config.ts

@@ -15,7 +15,7 @@ dotenv.config()
 const solidity = {
   compilers: [
     {
-      version: '0.8.9',
+      version: '0.8.28',
       settings: {
         optimizer: {
           enabled: true,

src/bridge/AbsBridge.sol

@@ -2,7 +2,7 @@
 // For license information, see https://github.com/nitro/blob/master/LICENSE
 // SPDX-License-Identifier: BUSL-1.1
 
-pragma solidity ^0.8.4;
+pragma solidity ^0.8.28;
 
 import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol";
@@ -42,7 +42,8 @@
     address[] public allowedDelayedInboxList;
     address[] public allowedOutboxList;
 
-    address internal _activeOutbox;
+    // @dev Deprecated in place of transient storage
+    address internal __activeOutbox;
 
     /// @inheritdoc IBridge
     bytes32[] public delayedInboxAccs;
@@ -55,7 +56,8 @@
 
     uint256 public override sequencerReportedSubMessageCount;
 
-    address internal constant EMPTY_ACTIVEOUTBOX = address(type(uint160).max);
+    // transient storage vars
+    address internal transient currentActiveOutbox;
 
     modifier onlyRollupOrOwner() {
         if (msg.sender != address(rollup)) {
@@ -75,13 +77,7 @@
 
     /// @dev returns the address of current active Outbox, or zero if no outbox is active
     function activeOutbox() public view returns (address) {
-        address outbox = _activeOutbox;
-        // address zero is returned if no outbox is set, but the value used in storage
-        // is non-zero to save users some gas (as storage refunds are usually maxed out)
-        // EIP-1153 would help here.
-        // we don't return `EMPTY_ACTIVEOUTBOX` to avoid a breaking change on the current api
-        if (outbox == EMPTY_ACTIVEOUTBOX) return address(0);
-        return outbox;
+        return currentActiveOutbox;
     }
 
     function allowedDelayedInboxes(address inbox) public view returns (bool) {
@@ -214,15 +210,16 @@
     ) external returns (bool success, bytes memory returnData) {
         if (!allowedOutboxes(msg.sender)) revert NotOutbox(msg.sender);
         if (data.length > 0 && !to.isContract()) revert NotContract(to);
-        address prevOutbox = _activeOutbox;
-        _activeOutbox = msg.sender;
-        // We set and reset active outbox around external call so activeOutbox remains valid during call
+
+        // We set and reset active outbox around external call so activeOutbox() remains valid during call
+        address prevOutbox = currentActiveOutbox;
+        currentActiveOutbox = msg.sender;
 
         // We use a low level call here since we want to bubble up whether it succeeded or failed to the caller
         // rather than reverting on failure as well as allow contract and non-contract calls
         (success, returnData) = _executeLowLevelCall(to, value, data);
 
-        _activeOutbox = prevOutbox;
+        currentActiveOutbox = prevOutbox;
         emit BridgeCallTriggered(msg.sender, to, value, data);
     }
 
@@ -252,7 +249,7 @@
     }
 
     function setOutbox(address outbox, bool enabled) external onlyRollupOrOwner {
-        if (outbox == EMPTY_ACTIVEOUTBOX) revert InvalidOutboxSet(outbox);
+        if (outbox == address(0)) revert InvalidOutboxSet(outbox);
 
         InOutInfo storage info = allowedOutboxesMap[outbox];
         bool alreadyEnabled = info.allowed;

src/bridge/AbsOutbox.sol

@@ -2,7 +2,7 @@
 // For license information, see https://github.com/nitro/blob/master/LICENSE
 // SPDX-License-Identifier: BUSL-1.1
 
-pragma solidity ^0.8.4;
+pragma solidity ^0.8.28;
 
 import {
     AlreadyInit,
@@ -45,49 +45,36 @@ abstract contract AbsOutbox is DelegateCallAware, IOutbox {
         uint256 withdrawalAmount;
     }
 
-    // Note, these variables are set and then wiped during a single transaction.
-    // Therefore their values don't need to be maintained, and their slots will
-    // hold default values (which are interpreted as empty values) outside of transactions
-    L2ToL1Context internal context;
-
-    // default context values to be used in storage instead of zero, to save on storage refunds
-    // it is assumed that arb-os never assigns these values to a valid leaf to be redeemed
-    uint128 private constant L2BLOCK_DEFAULT_CONTEXT = type(uint128).max;
-    uint96 private constant L1BLOCK_DEFAULT_CONTEXT = type(uint96).max;
-    uint128 private constant TIMESTAMP_DEFAULT_CONTEXT = type(uint128).max;
-    bytes32 private constant OUTPUTID_DEFAULT_CONTEXT = bytes32(type(uint256).max);
-    address private constant SENDER_DEFAULT_CONTEXT = address(type(uint160).max);
+    // @dev Deprecated in place of transient storage
+    L2ToL1Context internal __context;
 
     uint128 public constant OUTBOX_VERSION = 2;
 
+    // transient storage vars for L2ToL1Context
+    uint256 internal transient contextL2Block;
+    uint256 internal transient contextTimestamp;
+    bytes32 internal transient contextOutputId;
+    address internal transient contextSender;
+    uint256 internal transient contextL1Block;
+    uint256 internal transient contextWithdrawalAmount;
+
     function initialize(IBridge _bridge) external onlyDelegated {
         if (address(_bridge) == address(0)) revert HadZeroInit();
         if (address(bridge) != address(0)) revert AlreadyInit();
-        // address zero is returned if no context is set, but the values used in storage
-        // are non-zero to save users some gas (as storage refunds are usually maxed out)
-        // EIP-1153 would help here
-        context = L2ToL1Context({
-            l2Block: L2BLOCK_DEFAULT_CONTEXT,
-            l1Block: L1BLOCK_DEFAULT_CONTEXT,
-            timestamp: TIMESTAMP_DEFAULT_CONTEXT,
-            outputId: OUTPUTID_DEFAULT_CONTEXT,
-            sender: SENDER_DEFAULT_CONTEXT,
-            withdrawalAmount: _defaultContextAmount()
-        });
         bridge = _bridge;
         rollup = address(_bridge.rollup());
     }
 
     function postUpgradeInit() external onlyDelegated onlyProxyOwner {
         // prevent postUpgradeInit within a withdrawal
-        if (context.l2Block != L2BLOCK_DEFAULT_CONTEXT) revert BadPostUpgradeInit();
-        context = L2ToL1Context({
-            l2Block: L2BLOCK_DEFAULT_CONTEXT,
-            l1Block: L1BLOCK_DEFAULT_CONTEXT,
-            timestamp: TIMESTAMP_DEFAULT_CONTEXT,
-            outputId: OUTPUTID_DEFAULT_CONTEXT,
-            sender: SENDER_DEFAULT_CONTEXT,
-            withdrawalAmount: _defaultContextAmount()
+        if (__context.l2Block != type(uint128).max) revert BadPostUpgradeInit();
+        __context = L2ToL1Context({
+            l2Block: uint128(0),
+            l1Block: uint96(0),
+            timestamp: uint128(0),
+            outputId: bytes32(0),
+            sender: address(0),
+            withdrawalAmount: uint256(0)
         });
     }
 
@@ -108,34 +95,22 @@ abstract contract AbsOutbox is DelegateCallAware, IOutbox {
 
     /// @inheritdoc IOutbox
     function l2ToL1Sender() external view returns (address) {
-        address sender = context.sender;
-        // we don't return the default context value to avoid a breaking change in the API
-        if (sender == SENDER_DEFAULT_CONTEXT) return address(0);
-        return sender;
+        return contextSender;
     }
 
     /// @inheritdoc IOutbox
     function l2ToL1Block() external view returns (uint256) {
-        uint128 l2Block = context.l2Block;
-        // we don't return the default context value to avoid a breaking change in the API
-        if (l2Block == L2BLOCK_DEFAULT_CONTEXT) return uint256(0);
-        return uint256(l2Block);
+        return contextL2Block;
     }
 
     /// @inheritdoc IOutbox
     function l2ToL1EthBlock() external view returns (uint256) {
-        uint96 l1Block = context.l1Block;
-        // we don't return the default context value to avoid a breaking change in the API
-        if (l1Block == L1BLOCK_DEFAULT_CONTEXT) return uint256(0);
-        return uint256(l1Block);
+        return contextL1Block;
     }
 
     /// @inheritdoc IOutbox
     function l2ToL1Timestamp() external view returns (uint256) {
-        uint128 timestamp = context.timestamp;
-        // we don't return the default context value to avoid a breaking change in the API
-        if (timestamp == TIMESTAMP_DEFAULT_CONTEXT) return uint256(0);
-        return uint256(timestamp);
+        return contextTimestamp;
     }
 
     /// @notice batch number is deprecated and now always returns 0
@@ -145,10 +120,7 @@ abstract contract AbsOutbox is DelegateCallAware, IOutbox {
 
     /// @inheritdoc IOutbox
     function l2ToL1OutputId() external view returns (bytes32) {
-        bytes32 outputId = context.outputId;
-        // we don't return the default context value to avoid a breaking change in the API
-        if (outputId == OUTPUTID_DEFAULT_CONTEXT) return bytes32(0);
-        return outputId;
+        return contextOutputId;
     }
 
     /// @inheritdoc IOutbox
@@ -205,27 +177,37 @@ abstract contract AbsOutbox is DelegateCallAware, IOutbox {
     ) internal {
         emit OutBoxTransactionExecuted(to, l2Sender, 0, outputId);
 
+        // we temporarily store the previous values so the outbox can naturally
+        // unwind itself when there are nested calls to `executeTransaction`
+        uint256 prevL2Block = contextL2Block;
+        uint256 prevTimestamp = contextTimestamp;
+        bytes32 prevOutputId = contextOutputId;
+        address prevSender = contextSender;
+        uint256 prevL1Block = contextL1Block;
+        uint256 prevWithdrawalAmount = contextWithdrawalAmount;
+
         // get amount to unlock based on provided value. It might differ in case
         // of native token which uses number of decimals different than 18
         uint256 amountToUnlock = _getAmountToUnlock(value);
 
-        // we temporarily store the previous values so the outbox can naturally
-        // unwind itself when there are nested calls to `executeTransaction`
-        L2ToL1Context memory prevContext = context;
-
-        context = L2ToL1Context({
-            sender: l2Sender,
-            l2Block: uint128(l2Block),
-            l1Block: uint96(l1Block),
-            timestamp: uint128(l2Timestamp),
-            outputId: bytes32(outputId),
-            withdrawalAmount: _amountToSetInContext(amountToUnlock)
-        });
+        // store the new values into transient vars for the `executeTransaction` call
+        contextL2Block = l2Block;
+        contextTimestamp = l2Timestamp;
+        contextOutputId = bytes32(outputId);
+        contextSender = l2Sender;
+        contextL1Block = l1Block;
+        contextWithdrawalAmount = _amountToSetInContext(amountToUnlock);
 
         // set and reset vars around execution so they remain valid during call
         executeBridgeCall(to, amountToUnlock, data);
 
-        context = prevContext;
+        // restore the previous values
+        contextL2Block = prevL2Block;
+        contextTimestamp = prevTimestamp;
+        contextOutputId = prevOutputId;
+        contextSender = prevSender;
+        contextL1Block = prevL1Block;
+        contextWithdrawalAmount = prevWithdrawalAmount;
     }
 
     function _calcSpentIndexOffset(uint256 index)
@@ -311,10 +293,6 @@ abstract contract AbsOutbox is DelegateCallAware, IOutbox {
         return MerkleLib.calculateRoot(proof, path, keccak256(abi.encodePacked(item)));
     }
 
-    /// @notice default value to be used for 'amount' field in L2ToL1Context outside of transaction execution.
-    /// @return default 'amount' in case of ERC20-based rollup is type(uint256).max, or 0 in case of ETH-based rollup
-    function _defaultContextAmount() internal pure virtual returns (uint256);
-
     /// @notice based on provided value, get amount of ETH/token to unlock. In case of ETH-based rollup this amount
     ///         will always equal the provided value. In case of ERC20-based rollup, amount will be re-adjusted to
     ///         reflect the number of decimals used by native token, in case it is different than 18.

src/bridge/Bridge.sol

@@ -22,7 +22,6 @@ contract Bridge is AbsBridge, IEthBridge {
 
     /// @inheritdoc IEthBridge
     function initialize(IOwnable rollup_) external initializer onlyDelegated {
-        _activeOutbox = EMPTY_ACTIVEOUTBOX;
         rollup = rollup_;
     }
 

src/bridge/ERC20Bridge.sol

@@ -45,7 +45,6 @@ contract ERC20Bridge is AbsBridge, IERC20Bridge {
     function initialize(IOwnable rollup_, address nativeToken_) external initializer onlyDelegated {
         if (nativeToken_ == address(0)) revert InvalidTokenSet(nativeToken_);
         nativeToken = nativeToken_;
-        _activeOutbox = EMPTY_ACTIVEOUTBOX;
         rollup = rollup_;
 
         // store number of decimals used by native token

src/bridge/ERC20Outbox.sol

@@ -9,19 +9,8 @@ import {IERC20Bridge} from "./IERC20Bridge.sol";
 import {DecimalsConverterHelper} from "../libraries/DecimalsConverterHelper.sol";
 
 contract ERC20Outbox is AbsOutbox {
-    /// @dev it is assumed that arb-os never assigns this value to a valid leaf to be redeemed
-    uint256 private constant AMOUNT_DEFAULT_CONTEXT = type(uint256).max;
-
     function l2ToL1WithdrawalAmount() external view returns (uint256) {
-        uint256 amount = context.withdrawalAmount;
-        if (amount == AMOUNT_DEFAULT_CONTEXT) return 0;
-        return amount;
-    }
-
-    /// @inheritdoc AbsOutbox
-    function _defaultContextAmount() internal pure override returns (uint256) {
-        // we use type(uint256).max as representation of 0 native token withdrawal amount
-        return AMOUNT_DEFAULT_CONTEXT;
+        return contextWithdrawalAmount;
     }
 
     /// @inheritdoc AbsOutbox

src/bridge/Outbox.sol

@@ -7,13 +7,6 @@ pragma solidity ^0.8.4;
 import "./AbsOutbox.sol";
 
 contract Outbox is AbsOutbox {
-    /// @inheritdoc AbsOutbox
-    function _defaultContextAmount() internal pure override returns (uint256) {
-        // In ETH-based chains withdrawal amount can be read from msg.value. For that reason
-        // amount slot in context will never be accessed and it has 0 default value
-        return 0;
-    }
-
     /// @inheritdoc AbsOutbox
     function _getAmountToUnlock(uint256 value) internal pure override returns (uint256) {
         return value;

src/mocks/BridgeUnproxied.sol

@@ -13,7 +13,6 @@ contract BridgeUnproxied is Bridge {
     uint256 public DUMMYVAR = 0; // This is a dummy variable to disambiguous with the Bridge contract
 
     constructor() {
-        _activeOutbox = EMPTY_ACTIVEOUTBOX;
         rollup = IOwnable(msg.sender);
     }
 }

src/test-helpers/BridgeTester.sol

@@ -41,14 +41,17 @@ contract BridgeTester is Initializable, DelegateCallAware, IBridge, IEthBridge {
     address[] public allowedDelayedInboxList;
     address[] public allowedOutboxList;
 
-    address private _activeOutbox;
+    // @dev deprecated
+    address private __activeOutbox;
 
     IOwnable public rollup;
     address public sequencerInbox;
 
     address public nativeToken;
     uint8 public nativeTokenDecimals;
 
+    address internal transient currentActiveOutbox;
+
     modifier onlyRollupOrOwner() {
         if (msg.sender != address(rollup)) {
             address rollupOwner = rollup.owner();
@@ -70,10 +73,7 @@ contract BridgeTester is Initializable, DelegateCallAware, IBridge, IEthBridge {
     bytes32[] public override sequencerInboxAccs;
     uint256 public override sequencerReportedSubMessageCount;
 
-    address private constant EMPTY_ACTIVEOUTBOX = address(type(uint160).max);
-
     function initialize(IOwnable rollup_) external initializer {
-        _activeOutbox = EMPTY_ACTIVEOUTBOX;
         rollup = rollup_;
     }
 
@@ -82,8 +82,7 @@ contract BridgeTester is Initializable, DelegateCallAware, IBridge, IEthBridge {
     }
 
     function activeOutbox() public view returns (address) {
-        if (_activeOutbox == EMPTY_ACTIVEOUTBOX) return address(uint160(0));
-        return _activeOutbox;
+        return currentActiveOutbox;
     }
 
     function allowedDelayedInboxes(address inbox) external view override returns (bool) {
@@ -183,15 +182,17 @@ contract BridgeTester is Initializable, DelegateCallAware, IBridge, IEthBridge {
     ) external override returns (bool success, bytes memory returnData) {
         if (!allowedOutboxesMap[msg.sender].allowed) revert NotOutbox(msg.sender);
         if (data.length > 0 && !to.isContract()) revert NotContract(to);
-        address prevOutbox = _activeOutbox;
-        _activeOutbox = msg.sender;
-        // We set and reset active outbox around external call so activeOutbox remains valid during call
+
+        // We set and reset active outbox around external call so activeOutbox() remains valid during call
+        address prevOutbox = currentActiveOutbox;
+        currentActiveOutbox = msg.sender;
 
         // We use a low level call here since we want to bubble up whether it succeeded or failed to the caller
         // rather than reverting on failure as well as allow contract and non-contract calls
         // solhint-disable-next-line avoid-low-level-calls
         (success, returnData) = to.call{value: value}(data);
-        _activeOutbox = prevOutbox;
+
+        currentActiveOutbox = prevOutbox;
         emit BridgeCallTriggered(msg.sender, to, value, data);
     }