From 9febcfe48735a499875934d3c62bca1ec0e86d8b Mon Sep 17 00:00:00 2001 From: gzeon Date: Thu, 3 Oct 2024 12:15:25 +0800 Subject: [PATCH] refactor: CallerChecker --- src/bridge/AbsInbox.sol | 7 +++---- src/bridge/SequencerInbox.sol | 13 +++++-------- src/libraries/CallerChecker.sol | 16 ++++++++++++++++ src/libraries/Error.sol | 4 ++-- src/libraries/GasRefundEnabled.sol | 3 ++- 5 files changed, 28 insertions(+), 15 deletions(-) create mode 100644 src/libraries/CallerChecker.sol diff --git a/src/bridge/AbsInbox.sol b/src/bridge/AbsInbox.sol index 945b300f..085748d8 100644 --- a/src/bridge/AbsInbox.sol +++ b/src/bridge/AbsInbox.sol @@ -8,12 +8,11 @@ import { DataTooLarge, Deprecated, GasLimitTooLarge, - HasCode, InsufficientValue, InsufficientSubmissionCost, L1Forked, NotAllowedOrigin, - NotOrigin, + NotTopLevel, NotRollupOrOwner, RetryableData } from "../libraries/Error.sol"; @@ -21,6 +20,7 @@ import "./IInboxBase.sol"; import "./ISequencerInbox.sol"; import "./IBridge.sol"; import "../libraries/AddressAliasHelper.sol"; +import "../libraries/CallerChecker.sol"; import "../libraries/DelegateCallAware.sol"; import { L1MessageType_submitRetryableTx, @@ -142,8 +142,7 @@ abstract contract AbsInbox is DelegateCallAware, PausableUpgradeable, IInboxBase ) external whenNotPaused onlyAllowed returns (uint256) { if (_chainIdChanged()) revert L1Forked(); // solhint-disable-next-line avoid-tx-origin - if (msg.sender != tx.origin) revert NotOrigin(); - if (msg.sender.code.length != 0) revert HasCode(); + if (!CallerChecker.isCallerTopLevel()) revert NotTopLevel(); if (messageData.length > maxDataSize) revert DataTooLarge(messageData.length, maxDataSize); uint256 msgNum = _deliverToBridge(L2_MSG, msg.sender, keccak256(messageData), 0); emit InboxMessageDeliveredFromOrigin(msgNum); diff --git a/src/bridge/SequencerInbox.sol b/src/bridge/SequencerInbox.sol index c2057909..1e1902e9 100644 --- a/src/bridge/SequencerInbox.sol +++ b/src/bridge/SequencerInbox.sol @@ -12,7 +12,6 @@ import { DelayedBackwards, DelayedTooFar, ForceIncludeBlockTooSoon, - HasCode, IncorrectMessagePreimage, NotBatchPoster, BadSequencerNumber, @@ -20,6 +19,7 @@ import { NoSuchKeyset, NotForked, NotBatchPosterManager, + NotTopLevel, RollupNotChanged, DataBlobsNotSupported, InitParamZero, @@ -43,6 +43,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 "../libraries/DelegateCallAware.sol"; @@ -334,9 +335,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox uint256 prevMessageCount, uint256 newMessageCount ) external refundsGas(gasRefunder, IReader4844(address(0))) { - // solhint-disable-next-line avoid-tx-origin - if (msg.sender != tx.origin) revert NotOrigin(); - if (msg.sender.code.length != 0) revert HasCode(); + if (!CallerChecker.isCallerTopLevel()) revert NotTopLevel(); if (!isBatchPoster[msg.sender]) revert NotBatchPoster(); if (isDelayProofRequired(afterDelayedMessagesRead)) revert DelayProofRequired(); @@ -389,9 +388,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox uint256 newMessageCount, DelayProof calldata delayProof ) external refundsGas(gasRefunder, IReader4844(address(0))) { - // solhint-disable-next-line avoid-tx-origin - if (msg.sender != tx.origin) revert NotOrigin(); - if (msg.sender.code.length != 0) revert HasCode(); + if (!CallerChecker.isCallerTopLevel()) revert NotTopLevel(); if (!isBatchPoster[msg.sender]) revert NotBatchPoster(); if (!isDelayBufferable) revert NotDelayBufferable(); @@ -442,7 +439,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox // same as using calldata, we only submit spending report if the caller is the origin of the tx // 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 && msg.sender.code.length == 0 && !isUsingFeeToken) { + if (CallerChecker.isCallerTopLevel() && !isUsingFeeToken) { submitBatchSpendingReport(dataHash, seqMessageIndex, block.basefee, blobGas); } } diff --git a/src/libraries/CallerChecker.sol b/src/libraries/CallerChecker.sol new file mode 100644 index 00000000..f4ef6efe --- /dev/null +++ b/src/libraries/CallerChecker.sol @@ -0,0 +1,16 @@ +// 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 for top level caller, used to ensure the calldata is available in the tx + * @return bool true if the caller is a top level caller, false otherwise + */ + function isCallerTopLevel() internal view returns (bool) { + // solhint-disable-next-line avoid-tx-origin + return msg.sender == tx.origin && msg.sender.code.length == 0; + } +} diff --git a/src/libraries/Error.sol b/src/libraries/Error.sol index 2b962cb8..c600ff3a 100644 --- a/src/libraries/Error.sol +++ b/src/libraries/Error.sol @@ -13,8 +13,8 @@ error HadZeroInit(); /// @dev Thrown when post upgrade init validation fails error BadPostUpgradeInit(); -/// @dev Thrown when the sender has code -error HasCode(); +/// @dev Thrown when the caller is not a top level caller +error NotTopLevel(); /// @dev Thrown when non owner tries to access an only-owner function /// @param sender The msg.sender who is not the owner diff --git a/src/libraries/GasRefundEnabled.sol b/src/libraries/GasRefundEnabled.sol index 12e02f76..42bd3174 100644 --- a/src/libraries/GasRefundEnabled.sol +++ b/src/libraries/GasRefundEnabled.sol @@ -7,6 +7,7 @@ pragma solidity ^0.8.0; import "./IReader4844.sol"; import "./IGasRefunder.sol"; +import "../libraries/CallerChecker.sol"; abstract contract GasRefundEnabled { uint256 internal immutable gasPerBlob = 2 ** 17; @@ -25,7 +26,7 @@ abstract contract GasRefundEnabled { // if triggered in a contract call, the spender may be overrefunded by appending dummy data to the call // so we check if it is a top level call, which would mean the sender paid calldata as part of tx.input // solhint-disable-next-line avoid-tx-origin - if (msg.sender != tx.origin || msg.sender.code.length != 0) { + if (!CallerChecker.isCallerTopLevel()) { // We can't be sure if this calldata came from the top level tx, // so to be safe we tell the gas refunder there was no calldata. calldataSize = 0;