Skip to content

Commit

Permalink
[DEPLOY-694]: Adds zero-value check to ScrollSequencerUptimeFeed (#11710
Browse files Browse the repository at this point in the history
)

* adds zero value address check to ScrollSequencerUptimeFeed _setL1Sender helper function

* adds zero address check for l2CrossDomainMessenger

* adds more descriptive error messages to custom ZeroAddress error

* removing additional msg field from ZeroAddress error

* Adds zero address checks for ScrollSequencerUptimeFeed

* removes L1 zero address check and removes unnecessary encodeWithSelector calls

* updates gas snapshot
  • Loading branch information
chris-de-leon-cll authored Jan 24, 2024
1 parent 2586bc4 commit aa05727
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 23 deletions.
24 changes: 12 additions & 12 deletions contracts/gas-snapshots/l2ep.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ ArbitrumSequencerUptimeFeed_GasCosts:test_GasCosts() (gas: 97495)
ArbitrumSequencerUptimeFeed_ProtectReadsOnAggregatorV2V3InterfaceFunctions:test_AggregatorV2V3InterfaceAllowReadsIfConsumingContractIsWhitelisted() (gas: 602711)
ArbitrumSequencerUptimeFeed_ProtectReadsOnAggregatorV2V3InterfaceFunctions:test_AggregatorV2V3InterfaceDisallowReadsIfConsumingContractIsNotWhitelisted() (gas: 573802)
ArbitrumSequencerUptimeFeed_UpdateStatus:test_IgnoreOutOfOrderUpdates() (gas: 98976)
ArbitrumSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddr() (gas: 15776)
ArbitrumSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddr() (gas: 15416)
ArbitrumSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndNoTimeChange() (gas: 113269)
ArbitrumSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndTimeChange() (gas: 113329)
ArbitrumValidator_Validate:test_PostSequencerOffline() (gas: 69068)
Expand Down Expand Up @@ -76,16 +76,16 @@ OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRo
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRoundData() (gas: 57309)
OptimismSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestTimestamp() (gas: 56740)
OptimismSequencerUptimeFeed_AggregatorV3Interface:test_AggregatorV3Interface() (gas: 65617)
OptimismSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetAnswerWhenRoundDoesNotExistYet() (gas: 18039)
OptimismSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetRoundDataWhenRoundDoesNotExistYet() (gas: 18257)
OptimismSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetTimestampWhenRoundDoesNotExistYet() (gas: 17963)
OptimismSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetAnswerWhenRoundDoesNotExistYet() (gas: 17679)
OptimismSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetRoundDataWhenRoundDoesNotExistYet() (gas: 17897)
OptimismSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetTimestampWhenRoundDoesNotExistYet() (gas: 17603)
OptimismSequencerUptimeFeed_Constructor:test_InitialState() (gas: 21078)
OptimismSequencerUptimeFeed_GasCosts:test_GasCosts() (gas: 67197)
OptimismSequencerUptimeFeed_ProtectReadsOnAggregatorV2V3InterfaceFunctions:test_AggregatorV2V3InterfaceAllowReadsIfConsumingContractIsWhitelisted() (gas: 597640)
OptimismSequencerUptimeFeed_ProtectReadsOnAggregatorV2V3InterfaceFunctions:test_AggregatorV2V3InterfaceDisallowReadsIfConsumingContractIsNotWhitelisted() (gas: 573807)
OptimismSequencerUptimeFeed_UpdateStatus:test_IgnoreOutOfOrderUpdates() (gas: 66532)
OptimismSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddr() (gas: 13560)
OptimismSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddrAndNotL1SenderAddr() (gas: 23967)
OptimismSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddr() (gas: 13200)
OptimismSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddrAndNotL1SenderAddr() (gas: 23607)
OptimismSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenNoChange() (gas: 74035)
OptimismSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndNoTimeChange() (gas: 96155)
OptimismSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndTimeChange() (gas: 96215)
Expand Down Expand Up @@ -127,16 +127,16 @@ ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRoun
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestRoundData() (gas: 55473)
ScrollSequencerUptimeFeed_AggregatorInterfaceGasCosts:test_GasUsageForLatestTimestamp() (gas: 54758)
ScrollSequencerUptimeFeed_AggregatorV3Interface:test_AggregatorV3Interface() (gas: 63903)
ScrollSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetAnswerWhenRoundDoesNotExistYet() (gas: 18035)
ScrollSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetRoundDataWhenRoundDoesNotExistYet() (gas: 18253)
ScrollSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetTimestampWhenRoundDoesNotExistYet() (gas: 17959)
ScrollSequencerUptimeFeed_Constructor:test_InitialState() (gas: 21085)
ScrollSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetAnswerWhenRoundDoesNotExistYet() (gas: 17675)
ScrollSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetRoundDataWhenRoundDoesNotExistYet() (gas: 17893)
ScrollSequencerUptimeFeed_AggregatorV3Interface:test_RevertGetTimestampWhenRoundDoesNotExistYet() (gas: 17599)
ScrollSequencerUptimeFeed_Constructor:test_InitialState() (gas: 102485)
ScrollSequencerUptimeFeed_GasCosts:test_GasCosts() (gas: 64888)
ScrollSequencerUptimeFeed_ProtectReadsOnAggregatorV2V3InterfaceFunctions:test_AggregatorV2V3InterfaceAllowReadsIfConsumingContractIsWhitelisted() (gas: 597491)
ScrollSequencerUptimeFeed_ProtectReadsOnAggregatorV2V3InterfaceFunctions:test_AggregatorV2V3InterfaceDisallowReadsIfConsumingContractIsNotWhitelisted() (gas: 573807)
ScrollSequencerUptimeFeed_UpdateStatus:test_IgnoreOutOfOrderUpdates() (gas: 64417)
ScrollSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddr() (gas: 13560)
ScrollSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddrAndNotL1SenderAddr() (gas: 23967)
ScrollSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddr() (gas: 13200)
ScrollSequencerUptimeFeed_UpdateStatus:test_RevertIfNotL2CrossDomainMessengerAddrAndNotL1SenderAddr() (gas: 23607)
ScrollSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenNoChange() (gas: 71618)
ScrollSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndNoTimeChange() (gas: 92018)
ScrollSequencerUptimeFeed_UpdateStatus:test_UpdateStatusWhenStatusChangeAndTimeChange() (gas: 92078)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ contract ScrollSequencerUptimeFeed is
error InvalidSender();
/// @notice Replacement for AggregatorV3Interface "No data present"
error NoDataPresent();
/// @notice Address must not be the zero address
error ZeroAddress();

event L1SenderTransferred(address indexed from, address indexed to);
/// @dev Emitted when an `updateStatus` call is ignored due to unchanged status or stale timestamp
Expand Down Expand Up @@ -68,6 +70,10 @@ contract ScrollSequencerUptimeFeed is
/// @param l2CrossDomainMessengerAddr Address of the L2CrossDomainMessenger contract
/// @param initialStatus The initial status of the feed
constructor(address l1SenderAddress, address l2CrossDomainMessengerAddr, bool initialStatus) {
if (l2CrossDomainMessengerAddr == address(0)) {
revert ZeroAddress();
}

_setL1Sender(l1SenderAddress);
s_l2CrossDomainMessenger = IL2ScrollMessenger(l2CrossDomainMessengerAddr);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ contract ArbitrumSequencerUptimeFeed_UpdateStatus is ArbitrumSequencerUptimeFeed
vm.startPrank(s_strangerAddr, s_strangerAddr);

// Tries to update the status from an unauthorized account
vm.expectRevert(abi.encodeWithSelector(ArbitrumSequencerUptimeFeed.InvalidSender.selector));
vm.expectRevert(ArbitrumSequencerUptimeFeed.InvalidSender.selector);
s_arbitrumSequencerUptimeFeed.updateStatus(true, uint64(1));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ contract OptimismSequencerUptimeFeed_UpdateStatus is OptimismSequencerUptimeFeed
vm.startPrank(s_strangerAddr, s_strangerAddr);

// Tries to update the status from an unauthorized account
vm.expectRevert(abi.encodeWithSelector(OptimismSequencerUptimeFeed.InvalidSender.selector));
vm.expectRevert(OptimismSequencerUptimeFeed.InvalidSender.selector);
s_optimismSequencerUptimeFeed.updateStatus(true, uint64(1));
}

Expand All @@ -74,7 +74,7 @@ contract OptimismSequencerUptimeFeed_UpdateStatus is OptimismSequencerUptimeFeed
s_mockOptimismL2CrossDomainMessenger.setSender(s_strangerAddr);

// Tries to update the status from an unauthorized account
vm.expectRevert(abi.encodeWithSelector(OptimismSequencerUptimeFeed.InvalidSender.selector));
vm.expectRevert(OptimismSequencerUptimeFeed.InvalidSender.selector);
s_optimismSequencerUptimeFeed.updateStatus(true, uint64(1));
}

Expand Down Expand Up @@ -257,7 +257,7 @@ contract OptimismSequencerUptimeFeed_AggregatorV3Interface is OptimismSequencerU
vm.startPrank(s_l1OwnerAddr, s_l1OwnerAddr);

// Gets data from a round that has not happened yet
vm.expectRevert(abi.encodeWithSelector(OptimismSequencerUptimeFeed.NoDataPresent.selector));
vm.expectRevert(OptimismSequencerUptimeFeed.NoDataPresent.selector);
s_optimismSequencerUptimeFeed.getRoundData(2);
}

Expand All @@ -267,7 +267,7 @@ contract OptimismSequencerUptimeFeed_AggregatorV3Interface is OptimismSequencerU
vm.startPrank(s_l1OwnerAddr, s_l1OwnerAddr);

// Gets data from a round that has not happened yet
vm.expectRevert(abi.encodeWithSelector(OptimismSequencerUptimeFeed.NoDataPresent.selector));
vm.expectRevert(OptimismSequencerUptimeFeed.NoDataPresent.selector);
s_optimismSequencerUptimeFeed.getAnswer(2);
}

Expand All @@ -277,7 +277,7 @@ contract OptimismSequencerUptimeFeed_AggregatorV3Interface is OptimismSequencerU
vm.startPrank(s_l1OwnerAddr, s_l1OwnerAddr);

// Gets data from a round that has not happened yet
vm.expectRevert(abi.encodeWithSelector(OptimismSequencerUptimeFeed.NoDataPresent.selector));
vm.expectRevert(OptimismSequencerUptimeFeed.NoDataPresent.selector);
s_optimismSequencerUptimeFeed.getTimestamp(2);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ contract ScrollSequencerUptimeFeedTest is L2EPTest {
contract ScrollSequencerUptimeFeed_Constructor is ScrollSequencerUptimeFeedTest {
/// @notice it should have been deployed with the correct initial state
function test_InitialState() public {
// L2 cross domain messenger address must not be the zero address
vm.expectRevert(ScrollSequencerUptimeFeed.ZeroAddress.selector);
new ScrollSequencerUptimeFeed(s_l1OwnerAddr, address(0), false);

// Sets msg.sender and tx.origin to a valid address
vm.startPrank(s_l1OwnerAddr, s_l1OwnerAddr);

Expand All @@ -61,7 +65,7 @@ contract ScrollSequencerUptimeFeed_UpdateStatus is ScrollSequencerUptimeFeedTest
vm.startPrank(s_strangerAddr, s_strangerAddr);

// Tries to update the status from an unauthorized account
vm.expectRevert(abi.encodeWithSelector(ScrollSequencerUptimeFeed.InvalidSender.selector));
vm.expectRevert(ScrollSequencerUptimeFeed.InvalidSender.selector);
s_scrollSequencerUptimeFeed.updateStatus(true, uint64(1));
}

Expand All @@ -74,7 +78,7 @@ contract ScrollSequencerUptimeFeed_UpdateStatus is ScrollSequencerUptimeFeedTest
s_mockScrollL2CrossDomainMessenger.setSender(s_strangerAddr);

// Tries to update the status from an unauthorized account
vm.expectRevert(abi.encodeWithSelector(ScrollSequencerUptimeFeed.InvalidSender.selector));
vm.expectRevert(ScrollSequencerUptimeFeed.InvalidSender.selector);
s_scrollSequencerUptimeFeed.updateStatus(true, uint64(1));
}

Expand Down Expand Up @@ -257,7 +261,7 @@ contract ScrollSequencerUptimeFeed_AggregatorV3Interface is ScrollSequencerUptim
vm.startPrank(s_l1OwnerAddr, s_l1OwnerAddr);

// Gets data from a round that has not happened yet
vm.expectRevert(abi.encodeWithSelector(ScrollSequencerUptimeFeed.NoDataPresent.selector));
vm.expectRevert(ScrollSequencerUptimeFeed.NoDataPresent.selector);
s_scrollSequencerUptimeFeed.getRoundData(2);
}

Expand All @@ -267,7 +271,7 @@ contract ScrollSequencerUptimeFeed_AggregatorV3Interface is ScrollSequencerUptim
vm.startPrank(s_l1OwnerAddr, s_l1OwnerAddr);

// Gets data from a round that has not happened yet
vm.expectRevert(abi.encodeWithSelector(ScrollSequencerUptimeFeed.NoDataPresent.selector));
vm.expectRevert(ScrollSequencerUptimeFeed.NoDataPresent.selector);
s_scrollSequencerUptimeFeed.getAnswer(2);
}

Expand All @@ -277,7 +281,7 @@ contract ScrollSequencerUptimeFeed_AggregatorV3Interface is ScrollSequencerUptim
vm.startPrank(s_l1OwnerAddr, s_l1OwnerAddr);

// Gets data from a round that has not happened yet
vm.expectRevert(abi.encodeWithSelector(ScrollSequencerUptimeFeed.NoDataPresent.selector));
vm.expectRevert(ScrollSequencerUptimeFeed.NoDataPresent.selector);
s_scrollSequencerUptimeFeed.getTimestamp(2);
}
}
Expand Down

0 comments on commit aa05727

Please sign in to comment.