Skip to content

Commit

Permalink
replaces requireValidSender virtual modifier with virtual function
Browse files Browse the repository at this point in the history
  • Loading branch information
chris-de-leon-cll committed Jan 24, 2024
1 parent 2f6402f commit 1038273
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 60 deletions.
86 changes: 43 additions & 43 deletions contracts/gas-snapshots/l2ep.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,23 @@ ArbitrumCrossDomainGovernor_TransferL1Ownership:test_CallableByL1Owner() (gas: 4
ArbitrumCrossDomainGovernor_TransferL1Ownership:test_CallableByL1OwnerOrZeroAddress() (gas: 19312)
ArbitrumCrossDomainGovernor_TransferL1Ownership:test_NotCallableByL2Owner() (gas: 18323)
ArbitrumCrossDomainGovernor_TransferL1Ownership:test_NotCallableByNonOwners() (gas: 13200)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetAnswer() (gas: 92127)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetRoundData() (gas: 92701)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetTimestamp() (gas: 92018)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestAnswer() (gas: 89790)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRound() (gas: 89670)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRoundData() (gas: 90391)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestTimestamp() (gas: 89660)
ArbitrumSequencerUptimeFeed_AggregatorV3Interface:test_AggregatorV3Interface() (gas: 99141)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetAnswer() (gas: 92151)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetRoundData() (gas: 92725)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetTimestamp() (gas: 92042)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestAnswer() (gas: 89814)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRound() (gas: 89694)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRoundData() (gas: 90415)
ArbitrumSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestTimestamp() (gas: 89684)
ArbitrumSequencerUptimeFeed_AggregatorV3Interface:test_AggregatorV3Interface() (gas: 99165)
ArbitrumSequencerUptimeFeed_AggregatorV3Interface:test_Return0WhenRoundDoesNotExistYet() (gas: 20401)
ArbitrumSequencerUptimeFeed_Constants:test_InitialState() (gas: 5684)
ArbitrumSequencerUptimeFeed_GasCosts:test_GasCosts() (gas: 97427)
ArbitrumSequencerUptimeFeed_GasCosts:test_GasCosts() (gas: 97475)
ArbitrumSequencerUptimeFeed_ProtectReadsOnAggregatorV2V3InterfaceFunctions:test_AggregatorV2V3InterfaceAllowReadsIfConsumingContractIsWhitelisted() (gas: 602759)
ArbitrumSequencerUptimeFeed_ProtectReadsOnAggregatorV2V3InterfaceFunctions:test_AggregatorV2V3InterfaceDisallowReadsIfConsumingContractIsNotWhitelisted() (gas: 573802)
ArbitrumSequencerUptimeFeed_UpdateStatus:test_IgnoreOutOfOrderUpdates() (gas: 98946)
ArbitrumSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddr() (gas: 15219)
ArbitrumSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndNoTimeChange() (gas: 113160)
ArbitrumSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndTimeChange() (gas: 113220)
ArbitrumSequencerUptimeFeed_UpdateStatus:test_IgnoreOutOfOrderUpdates() (gas: 98994)
ArbitrumSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddr() (gas: 15234)
ArbitrumSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndNoTimeChange() (gas: 113208)
ArbitrumSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndTimeChange() (gas: 113268)
ArbitrumValidator_Validate:test_PostSequencerOffline() (gas: 69035)
OptimismCrossDomainForwarder_AcceptL1Ownership:test_CallableByPendingL1Owner() (gas: 46884)
OptimismCrossDomainForwarder_AcceptL1Ownership:test_NotCallableByNonPendingOwners() (gas: 22152)
Expand Down Expand Up @@ -68,27 +68,27 @@ OptimismCrossDomainGovernor_TransferL1Ownership:test_CallableByL1Owner() (gas: 4
OptimismCrossDomainGovernor_TransferL1Ownership:test_CallableByL1OwnerOrZeroAddress() (gas: 28767)
OptimismCrossDomainGovernor_TransferL1Ownership:test_NotCallableByL2Owner() (gas: 16134)
OptimismCrossDomainGovernor_TransferL1Ownership:test_NotCallableByNonOwners() (gas: 11011)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetAnswer() (gas: 57637)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetRoundData() (gas: 58149)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetTimestamp() (gas: 57470)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestAnswer() (gas: 55264)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRound() (gas: 55144)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRoundData() (gas: 55843)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestTimestamp() (gas: 55134)
OptimismSequencerUptimeFeed_AggregatorV3Interface:test_AggregatorV3Interface() (gas: 64511)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetAnswer() (gas: 57661)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetRoundData() (gas: 58173)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetTimestamp() (gas: 57494)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestAnswer() (gas: 55288)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRound() (gas: 55168)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRoundData() (gas: 55867)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestTimestamp() (gas: 55158)
OptimismSequencerUptimeFeed_AggregatorV3Interface:test_AggregatorV3Interface() (gas: 64535)
OptimismSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetAnswerWhenRoundDoesNotExistYet() (gas: 19940)
OptimismSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetRoundDataWhenRoundDoesNotExistYet() (gas: 20158)
OptimismSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetTimestampWhenRoundDoesNotExistYet() (gas: 19864)
OptimismSequencerUptimeFeed_Constructor:test_InitialState() (gas: 21208)
OptimismSequencerUptimeFeed_GasCosts:test_GasCosts() (gas: 65583)
OptimismSequencerUptimeFeed_GasCosts:test_GasCosts() (gas: 65631)
OptimismSequencerUptimeFeed_ProtectReadsOnAggregatorV2V3InterfaceFunctions:test_AggregatorV2V3InterfaceAllowReadsIfConsumingContractIsWhitelisted() (gas: 597743)
OptimismSequencerUptimeFeed_ProtectReadsOnAggregatorV2V3InterfaceFunctions:test_AggregatorV2V3InterfaceDisallowReadsIfConsumingContractIsNotWhitelisted() (gas: 573807)
OptimismSequencerUptimeFeed_UpdateStatus:test_IgnoreOutOfOrderUpdates() (gas: 65007)
OptimismSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddr() (gas: 13067)
OptimismSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddrAndNotL1SenderAddr() (gas: 23474)
OptimismSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenNoChange() (gas: 72688)
OptimismSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndNoTimeChange() (gas: 92899)
OptimismSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndTimeChange() (gas: 92959)
OptimismSequencerUptimeFeed_UpdateStatus:test_IgnoreOutOfOrderUpdates() (gas: 65055)
OptimismSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddr() (gas: 13082)
OptimismSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddrAndNotL1SenderAddr() (gas: 23489)
OptimismSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenNoChange() (gas: 72736)
OptimismSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndNoTimeChange() (gas: 92947)
OptimismSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndTimeChange() (gas: 93007)
OptimismValidator_SetGasLimit:test_CorrectlyUpdatesTheGasLimit() (gas: 15503)
OptimismValidator_Validate:test_PostSequencerOffline() (gas: 74761)
OptimismValidator_Validate:test_PostSequencerStatusWhenThereIsNotStatusChange() (gas: 74817)
Expand Down Expand Up @@ -119,27 +119,27 @@ ScrollCrossDomainGovernor_TransferL1Ownership:test_CallableByL1Owner() (gas: 489
ScrollCrossDomainGovernor_TransferL1Ownership:test_CallableByL1OwnerOrZeroAddress() (gas: 28833)
ScrollCrossDomainGovernor_TransferL1Ownership:test_NotCallableByL2Owner() (gas: 16134)
ScrollCrossDomainGovernor_TransferL1Ownership:test_NotCallableByNonOwners() (gas: 11011)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetAnswer() (gas: 57660)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetRoundData() (gas: 58172)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetTimestamp() (gas: 57493)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestAnswer() (gas: 55287)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRound() (gas: 55167)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRoundData() (gas: 55866)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestTimestamp() (gas: 55157)
ScrollSequencerUptimeFeed_AggregatorV3Interface:test_AggregatorV3Interface() (gas: 64534)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetAnswer() (gas: 57684)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetRoundData() (gas: 58196)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForGetTimestamp() (gas: 57517)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestAnswer() (gas: 55311)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRound() (gas: 55191)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRoundData() (gas: 55890)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestTimestamp() (gas: 55181)
ScrollSequencerUptimeFeed_AggregatorV3Interface:test_AggregatorV3Interface() (gas: 64558)
ScrollSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetAnswerWhenRoundDoesNotExistYet() (gas: 19940)
ScrollSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetRoundDataWhenRoundDoesNotExistYet() (gas: 20158)
ScrollSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetTimestampWhenRoundDoesNotExistYet() (gas: 19864)
ScrollSequencerUptimeFeed_Constructor:test_InitialState() (gas: 126943)
ScrollSequencerUptimeFeed_GasCosts:test_GasCosts() (gas: 65629)
ScrollSequencerUptimeFeed_GasCosts:test_GasCosts() (gas: 65677)
ScrollSequencerUptimeFeed_ProtectReadsOnAggregatorV2V3InterfaceFunctions:test_AggregatorV2V3InterfaceAllowReadsIfConsumingContractIsWhitelisted() (gas: 597743)
ScrollSequencerUptimeFeed_ProtectReadsOnAggregatorV2V3InterfaceFunctions:test_AggregatorV2V3InterfaceDisallowReadsIfConsumingContractIsNotWhitelisted() (gas: 573807)
ScrollSequencerUptimeFeed_UpdateStatus:test_IgnoreOutOfOrderUpdates() (gas: 65053)
ScrollSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddr() (gas: 13067)
ScrollSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddrAndNotL1SenderAddr() (gas: 23474)
ScrollSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenNoChange() (gas: 72734)
ScrollSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndNoTimeChange() (gas: 92945)
ScrollSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndTimeChange() (gas: 93005)
ScrollSequencerUptimeFeed_UpdateStatus:test_IgnoreOutOfOrderUpdates() (gas: 65101)
ScrollSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddr() (gas: 13082)
ScrollSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddrAndNotL1SenderAddr() (gas: 23489)
ScrollSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenNoChange() (gas: 72782)
ScrollSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndNoTimeChange() (gas: 92993)
ScrollSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndTimeChange() (gas: 93053)
ScrollValidator_SetGasLimit:test_CorrectlyUpdatesTheGasLimit() (gas: 15503)
ScrollValidator_Validate:test_PostSequencerOffline() (gas: 75094)
ScrollValidator_Validate:test_PostSequencerStatusWhenThereIsNotStatusChange() (gas: 75156)
Expand Down
6 changes: 3 additions & 3 deletions contracts/src/v0.8/l2ep/dev/CrossDomainGovernor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,19 @@ abstract contract CrossDomainGovernor is
function crossDomainMessenger() external view virtual returns (address);

/// @notice The call MUST come from either the L1 owner (via cross-chain message) or the L2 owner. Reverts otherwise.
function requireLocalOrCrossDomainOwner() internal view virtual;
function _requireLocalOrCrossDomainOwner() internal view virtual;

/// @inheritdoc ForwarderInterface
/// @dev forwarded only if L2 Messenger calls with `msg.sender` being the L1 owner address, or called by the L2 owner
function forward(address target, bytes memory data) external override {
requireLocalOrCrossDomainOwner();
_requireLocalOrCrossDomainOwner();
Address.functionCall(target, data, "Governor call reverted");
}

/// @inheritdoc DelegateForwarderInterface
/// @dev forwarded only if L2 Messenger calls with `msg.sender` being the L1 owner address, or called by the L2 owner
function forwardDelegate(address target, bytes memory data) external override {
requireLocalOrCrossDomainOwner();
_requireLocalOrCrossDomainOwner();
Address.functionDelegateCall(target, data, "Governor delegatecall reverted");
}
}
11 changes: 7 additions & 4 deletions contracts/src/v0.8/l2ep/dev/SequencerUptimeFeed.sol
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ abstract contract SequencerUptimeFeed is AggregatorV2V3Interface, ITypeAndVersio
}

/// @notice Should revert if the sender is not authorized to call `updateStatus`
modifier requireValidSender() virtual {
_;
}
function _requireValidSender() internal view virtual;

/// @notice Check if a roundId is valid in this current contract state
/// @dev Mainly used for AggregatorV2V3Interface functions
Expand Down Expand Up @@ -128,7 +126,11 @@ abstract contract SequencerUptimeFeed is AggregatorV2V3Interface, ITypeAndVersio
/// @dev This function will revert if not called from `l1Sender` via the L1->L2 messenger.
/// @param status Sequencer status
/// @param timestamp Block timestamp of status update
function updateStatus(bool status, uint64 timestamp) external virtual requireInitialized requireValidSender {
function updateStatus(bool status, uint64 timestamp) external virtual requireInitialized {
// Checks that the sender can call updateStatus
_requireValidSender();

// Stores the feed state
FeedState memory feedState = s_feedState;

// Ignore if latest recorded timestamp is newer
Expand All @@ -137,6 +139,7 @@ abstract contract SequencerUptimeFeed is AggregatorV2V3Interface, ITypeAndVersio
return;
}

// Record a new round or update an existing one
if (feedState.latestStatus == status) {
s_feedState.updatedAt = uint64(block.timestamp);
s_rounds[feedState.latestRoundId].updatedAt = uint64(block.timestamp);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ contract ArbitrumCrossDomainGovernor is CrossDomainGovernor {
}

/// @notice The call MUST come from either the L1 owner (via cross-chain message) or the L2 owner. Reverts otherwise.
function requireLocalOrCrossDomainOwner() internal view override {
function _requireLocalOrCrossDomainOwner() internal view override {
// solhint-disable-next-line custom-errors
require(msg.sender == crossDomainMessenger() || msg.sender == owner(), "Sender is not the L2 messenger or owner");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,10 @@ contract ArbitrumSequencerUptimeFeed is SequencerUptimeFeed {
}

/// @notice Reverts if the sender is not allowed to call `updateStatus`
modifier requireValidSender() override {
function _requireValidSender() internal view override {
if (msg.sender != AddressAliasHelper.applyL1ToL2Alias(l1Sender())) {
revert InvalidSender();
}
_;
}

/// @notice Initialise the first round. Can't be done in the constructor,
Expand All @@ -62,7 +61,11 @@ contract ArbitrumSequencerUptimeFeed is SequencerUptimeFeed {
///
/// @param status Sequencer status
/// @param timestamp Block timestamp of status update
function updateStatus(bool status, uint64 timestamp) external override requireInitialized requireValidSender {
function updateStatus(bool status, uint64 timestamp) external override requireInitialized {
// Checks that the sender can call updateStatus
_requireValidSender();

// Stores the feed state
FeedState memory feedState = s_feedState;

// Ignore if status did not change or latest recorded timestamp is newer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ contract OptimismCrossDomainGovernor is CrossDomainGovernor {
}

/// @notice The call MUST come from either the L1 owner (via cross-chain message) or the L2 owner. Reverts otherwise.
function requireLocalOrCrossDomainOwner() internal view override {
function _requireLocalOrCrossDomainOwner() internal view override {
// 1. The delegatecall MUST come from either the L1 owner (via cross-chain message) or the L2 owner
// solhint-disable-next-line custom-errors
require(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,11 @@ contract OptimismSequencerUptimeFeed is SequencerUptimeFeed {
}

/// @notice Reverts if the sender is not allowed to call `updateStatus`
modifier requireValidSender() override {
function _requireValidSender() internal view override {
if (
msg.sender != address(s_l2CrossDomainMessenger) || s_l2CrossDomainMessenger.xDomainMessageSender() != l1Sender()
) {
revert InvalidSender();
}
_;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ contract ScrollCrossDomainGovernor is CrossDomainGovernor {
}

/// @notice The call MUST come from either the L1 owner (via cross-chain message) or the L2 owner. Reverts otherwise.
function requireLocalOrCrossDomainOwner() internal view override {
function _requireLocalOrCrossDomainOwner() internal view override {
// 1. The delegatecall MUST come from either the L1 owner (via cross-chain message) or the L2 owner
// solhint-disable-next-line custom-errors
require(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,11 @@ contract ScrollSequencerUptimeFeed is SequencerUptimeFeed {
}

/// @notice Reverts if the sender is not allowed to call `updateStatus`
modifier requireValidSender() override {
function _requireValidSender() internal view override {
if (
msg.sender != address(s_l2CrossDomainMessenger) || s_l2CrossDomainMessenger.xDomainMessageSender() != l1Sender()
) {
revert InvalidSender();
}
_;
}
}

0 comments on commit 1038273

Please sign in to comment.