Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DEPLOY-694]: Adds zero-value check to ScrollSequencerUptimeFeed #11710

Merged
merged 7 commits into from
Jan 24, 2024
Merged

[DEPLOY-694]: Adds zero-value check to ScrollSequencerUptimeFeed #11710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
22018eb
adds zero value address check to ScrollSequencerUptimeFeed _setL1Send…
chris-de-leon-cll Jan 8, 2024
ef1d193
adds zero address check for l2CrossDomainMessenger
chris-de-leon-cll Jan 9, 2024
b5cff5e
adds more descriptive error messages to custom ZeroAddress error
chris-de-leon-cll Jan 9, 2024
c0f26a1
removing additional msg field from ZeroAddress error
chris-de-leon-cll Jan 16, 2024
7ede863
Adds zero address checks for ScrollSequencerUptimeFeed
chris-de-leon-cll Jan 22, 2024
a881c6e
removes L1 zero address check and removes unnecessary encodeWithSelec…
chris-de-leon-cll Jan 23, 2024
bb434b1
updates gas snapshot
chris-de-leon-cll Jan 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions contracts/gas-snapshots/l2ep.gas-snapshot
View file
Open in desktop
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
6 changes: 6 additions & 0 deletions contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol
View file
Open in desktop
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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect a test for each revert. Since this adds a revert I would expect a test. Same for the other one

}

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

Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/l2ep/test/v1_0_0/arbitrum/ArbitrumSequencerUptimeFeed.t.sol
View file
Open in desktop
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
10 changes: 5 additions & 5 deletions contracts/src/v0.8/l2ep/test/v1_0_0/optimism/OptimismSequencerUptimeFeed.t.sol
View file
Open in desktop
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
14 changes: 9 additions & 5 deletions contracts/src/v0.8/l2ep/test/v1_0_0/scroll/ScrollSequencerUptimeFeed.t.sol
View file
Open in desktop
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
Loading