Skip to content

N
nebula
Commit c71500a7 authored by Kelvin Fichter's avatar Kelvin Fichter
Browse files
Options

feat(ctb): allow multiple output deletions 

Updates the L2OutputOracle to allow deletion of multiple outputs at the
same time. This is a safety mechanism that prevents a compromised
sequencer from forcing the multisig into deleting potentially thousands
of bad outputs one by one.
parent 49685f81
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 188 additions and 178 deletions
.changeset/lovely-sheep-wash.md 0 → 100644
View file @ c71500a7
---
'@eth-optimism/contracts-bedrock': patch
---
Updates L2OutputOracle to easily delete multiple outputs at once
op-bindings/bindings/l2outputoracle.go
View file @ c71500a7
This diff is collapsed.
packages/contracts-bedrock/.gas-snapshot
View file @ c71500a7
......@@ -85,29 +85,30 @@ L2CrossDomainMessenger_Test:test_L2MessengerSendMessage() (gas: 122423)
L2CrossDomainMessenger_Test:test_L2MessengerTwiceSendMessage() (gas: 134632)
L2CrossDomainMessenger_Test:test_L2MessengerXDomainSenderReverts() (gas: 10568)
L2CrossDomainMessenger_Test:test_L2MessengerxDomainMessageSenderResets() (gas: 52615)
L2OutputOracleTest:testCannot_ProposeWithUnmatchedBlockhash() (gas: 26798)
L2OutputOracleTest:testCannot_constructWithBadTimestamp() (gas: 50365)
L2OutputOracleTest:testCannot_deleteL2Output_ifNotOwner() (gas: 25064)
L2OutputOracleTest:testCannot_deleteL2Output_withWrongRoot() (gas: 91319)
L2OutputOracleTest:testCannot_deleteL2Output_withWrongTime() (gas: 87302)
L2OutputOracleTest:testCannot_proposeEmptyOutput() (gas: 24074)
L2OutputOracleTest:testCannot_ProposeWithUnmatchedBlockhash() (gas: 26776)
L2OutputOracleTest:testCannot_constructWithBadTimestamp() (gas: 50351)
L2OutputOracleTest:testCannot_deleteL2Outputs_afterLatest() (gas: 194584)
L2OutputOracleTest:testCannot_deleteL2Outputs_ifNotOwner() (gas: 18871)
L2OutputOracleTest:testCannot_deleteL2Outputs_nonExistent() (gas: 86711)
L2OutputOracleTest:testCannot_proposeEmptyOutput() (gas: 24085)
L2OutputOracleTest:testCannot_proposeFutureTimetamp() (gas: 26043)
L2OutputOracleTest:testCannot_proposeL2OutputIfNotProposer() (gas: 23537)
L2OutputOracleTest:testCannot_proposeOnWrongFork() (gas: 26316)
L2OutputOracleTest:testCannot_proposeUnexpectedBlockNumber() (gas: 25977)
L2OutputOracleTest:test_changeProposer() (gas: 47047)
L2OutputOracleTest:test_computeL2Timestamp() (gas: 30483)
L2OutputOracleTest:test_constructor() (gas: 45586)
L2OutputOracleTest:test_deleteOutput() (gas: 77092)
L2OutputOracleTest:test_getL2Output() (gas: 88702)
L2OutputOracleTest:test_latestBlockNumber() (gas: 76186)
L2OutputOracleTest:test_nextBlockNumber() (gas: 15187)
L2OutputOracleTest:test_proposeWithBlockhashAndHeight() (gas: 74990)
L2OutputOracleTest:test_proposingAnotherOutput() (gas: 76815)
L2OutputOracleTest:testCannot_proposeL2OutputIfNotProposer() (gas: 23515)
L2OutputOracleTest:testCannot_proposeOnWrongFork() (gas: 26371)
L2OutputOracleTest:testCannot_proposeUnexpectedBlockNumber() (gas: 25955)
L2OutputOracleTest:test_changeProposer() (gas: 47212)
L2OutputOracleTest:test_computeL2Timestamp() (gas: 30430)
L2OutputOracleTest:test_constructor() (gas: 45691)
L2OutputOracleTest:test_deleteOutputs_multipleOutputs() (gas: 203841)
L2OutputOracleTest:test_deleteOutputs_singleOutput() (gas: 94524)
L2OutputOracleTest:test_getL2Output() (gas: 84525)
L2OutputOracleTest:test_latestBlockNumber() (gas: 76229)
L2OutputOracleTest:test_nextBlockNumber() (gas: 15120)
L2OutputOracleTest:test_proposeWithBlockhashAndHeight() (gas: 75035)
L2OutputOracleTest:test_proposingAnotherOutput() (gas: 76837)
L2OutputOracleTest:test_updateOwner() (gas: 36063)
L2OutputOracleUpgradeable_Test:test_cannotInitImpl() (gas: 17403)
L2OutputOracleUpgradeable_Test:test_cannotInitProxy() (gas: 22398)
L2OutputOracleUpgradeable_Test:test_initValuesOnProxy() (gas: 36068)
L2OutputOracleUpgradeable_Test:test_cannotInitImpl() (gas: 17381)
L2OutputOracleUpgradeable_Test:test_cannotInitProxy() (gas: 22376)
L2OutputOracleUpgradeable_Test:test_initValuesOnProxy() (gas: 38217)
L2OutputOracleUpgradeable_Test:test_upgrading() (gas: 180457)
L2StandardBridge_Test:test_cannotWithdrawEthWithoutSendingIt() (gas: 21749)
L2StandardBridge_Test:test_finalizeBridgeETH_incorrectValueReverts() (gas: 23733)
......@@ -149,21 +150,21 @@ OptimismPortalUpgradeable_Test:test_initialize_cannotInitImpl_reverts() (gas: 10
OptimismPortalUpgradeable_Test:test_initialize_cannotInitProxy_reverts() (gas: 15767)
OptimismPortalUpgradeable_Test:test_params_initValuesOnProxy_success() (gas: 16010)
OptimismPortalUpgradeable_Test:test_upgradeToAndCall_upgrading_success() (gas: 180435)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputRootChanges_reverts() (gas: 192770)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputTimestampIsNotFinalized_reverts() (gas: 195058)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputRootChanges_reverts() (gas: 194874)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputTimestampIsNotFinalized_reverts() (gas: 197162)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalNotProven_reverts() (gas: 39589)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalProofNotOldEnough_reverts() (gas: 190512)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onInsufficientGas_reverts() (gas: 192953)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onRecentWithdrawal_reverts() (gas: 173010)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReentrancy_reverts() (gas: 233210)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReplay_reverts() (gas: 232604)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_provenWithdrawalHash_success() (gas: 224624)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_targetFails_fails() (gas: 327261)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_timestampLessThanL2OracleStart_reverts() (gas: 191268)
OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onInvalidOutputRootProof_reverts() (gas: 81307)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalProofNotOldEnough_reverts() (gas: 192616)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onInsufficientGas_reverts() (gas: 192931)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onRecentWithdrawal_reverts() (gas: 172988)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReentrancy_reverts() (gas: 233188)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReplay_reverts() (gas: 234938)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_provenWithdrawalHash_success() (gas: 226854)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_targetFails_fails() (gas: 329491)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_timestampLessThanL2OracleStart_reverts() (gas: 193394)
OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onInvalidOutputRootProof_reverts() (gas: 83433)
OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onSelfCall_reverts() (gas: 50776)
OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_oninvalidWithdrawalProof_reverts() (gas: 130079)
OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_validWithdrawalProof_success() (gas: 176821)
OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_oninvalidWithdrawalProof_reverts() (gas: 132205)
OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_validWithdrawalProof_success() (gas: 178947)
OptimismPortal_Test:test_OptimismPortalConstructor() (gas: 17298)
OptimismPortal_Test:test_OptimismPortalReceiveEth_success() (gas: 127483)
OptimismPortal_Test:test_depositTransaction_NoValueContract_success() (gas: 76706)
......@@ -175,7 +176,7 @@ OptimismPortal_Test:test_depositTransaction_withEthValueAndContractContractCreat
OptimismPortal_Test:test_depositTransaction_withEthValueAndEOAContractCreation_success() (gas: 75852)
OptimismPortal_Test:test_depositTransaction_withEthValueFromContract_success() (gas: 83370)
OptimismPortal_Test:test_depositTransaction_withEthValueFromEOA_success() (gas: 83964)
OptimismPortal_Test:test_isBlockFinalized_success() (gas: 113327)
OptimismPortal_Test:test_isBlockFinalized_success() (gas: 104802)
OptimismPortal_Test:test_simple_isBlockFinalized_success() (gas: 24142)
Proxy_Test:test_clashingFunctionSignatures() (gas: 101347)
Proxy_Test:test_implementationKey() (gas: 20887)
......
packages/contracts-bedrock/contracts/L1/L2OutputOracle.sol
View file @ c71500a7
......@@ -67,17 +67,11 @@ contract L2OutputOracle is OwnableUpgradeable, Semver {
);
/**
* @notice Emitted when an output is deleted.
* @notice Emitted when outputs are deleted.
*
* @param outputRoot The output root.
* @param l1Timestamp The L1 timestamp when proposed.
* @param l2BlockNumber The L2 block number of the output root.
* @param l2BlockNumber First L2 block number deleted.
*/
event OutputDeleted(
bytes32 indexed outputRoot,
uint256 indexed l1Timestamp,
uint256 indexed l2BlockNumber
);
event OutputsDeleted(uint256 indexed l2BlockNumber);
/**
* @notice Emitted when the proposer address is changed.
......@@ -148,32 +142,35 @@ contract L2OutputOracle is OwnableUpgradeable, Semver {
}
/**
* @notice Deletes the most recent output. This is used to remove the most recent output in the
* event that an erreneous output is submitted. It can only be called by the contract's
* owner, not the proposer. Longer term, this should be replaced with a more robust
* mechanism which will allow deletion of proposals shown to be invalid by a fault
* proof.
* @notice Deletes all output proposals after and including the proposal that corresponds to
* the given block number. Can only be called by the owner, but will be replaced with
* a mechanism that allows a challenger contract to delete proposals.
*
* @param _proposal Represents the output proposal to delete
* @param _l2BlockNumber L2 block number of the first output root to delete.
*/
// solhint-disable-next-line ordering
function deleteL2Output(Types.OutputProposal memory _proposal) external onlyOwner {
Types.OutputProposal memory outputToDelete = l2Outputs[latestBlockNumber];
function deleteL2Outputs(uint256 _l2BlockNumber) external onlyOwner {
// Simple check that accomplishes two things:
// 1. Prevents deleting anything from before the genesis block.
// 2. Prevents deleting anything other than a checkpoint block.
require(
_proposal.outputRoot == outputToDelete.outputRoot,
"L2OutputOracle: output root to delete does not match the latest output proposal"
l2Outputs[_l2BlockNumber].outputRoot != bytes32(0),
"L2OutputOracle: cannot delete a non-existent output"
);
// Prevent deleting beyond latest block number. Above check will miss this case if we
// already deleted an output and then the user tries to delete a later output.
require(
_proposal.timestamp == outputToDelete.timestamp,
"L2OutputOracle: timestamp to delete does not match the latest output proposal"
_l2BlockNumber <= latestBlockNumber,
"L2OutputOracle: cannot delete outputs after the latest block number"
);
emit OutputDeleted(outputToDelete.outputRoot, outputToDelete.timestamp, latestBlockNumber);
// We're setting the latest block number back to the checkpoint block before the given L2
// block number. Next proposal will overwrite the deleted output and following proposals
// will delete any outputs after that.
latestBlockNumber = _l2BlockNumber - SUBMISSION_INTERVAL;
delete l2Outputs[latestBlockNumber];
latestBlockNumber = latestBlockNumber - SUBMISSION_INTERVAL;
emit OutputsDeleted(_l2BlockNumber);
}
/**
......@@ -242,6 +239,11 @@ contract L2OutputOracle is OwnableUpgradeable, Semver {
view
returns (Types.OutputProposal memory)
{
require(
_l2BlockNumber <= latestBlockNumber,
"L2OutputOracle: block number cannot be greater than the latest block number"
);
// Find the distance between _l2BlockNumber, and the checkpoint block before it.
uint256 offset = (_l2BlockNumber - STARTING_BLOCK_NUMBER) % SUBMISSION_INTERVAL;
......
packages/contracts-bedrock/contracts/test/L2OutputOracle.t.sol
View file @ c71500a7
......@@ -79,7 +79,9 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
oracle.getL2Output(0);
// The block number is larger than the latest proposed output:
vm.expectRevert("L2OutputOracle: no output found for the given block number");
vm.expectRevert(
"L2OutputOracle: block number cannot be greater than the latest block number"
);
oracle.getL2Output(nextBlockNumber + 1);
}
......@@ -269,36 +271,52 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
* Delete Tests - Happy Path *
*****************************/
event OutputDeleted(
bytes32 indexed l2Output,
uint256 indexed l1Timestamp,
uint256 indexed l2BlockNumber
);
event OutputsDeleted(uint256 indexed l2BlockNumber);
function test_deleteOutput() external {
function test_deleteOutputs_singleOutput() external {
test_proposingAnotherOutput();
uint256 latestBlockNumber = oracle.latestBlockNumber();
Types.OutputProposal memory proposalToDelete = oracle.getL2Output(latestBlockNumber);
Types.OutputProposal memory newLatestOutput = oracle.getL2Output(
latestBlockNumber - submissionInterval
);
vm.prank(owner);
vm.expectEmit(true, true, false, false);
emit OutputDeleted(
proposalToDelete.outputRoot,
proposalToDelete.timestamp,
latestBlockNumber
);
oracle.deleteL2Output(proposalToDelete);
emit OutputsDeleted(latestBlockNumber);
oracle.deleteL2Outputs(latestBlockNumber);
// validate latestBlockNumber has been reduced
uint256 latestBlockNumberAfter = oracle.latestBlockNumber();
assertEq(latestBlockNumber - submissionInterval, latestBlockNumberAfter);
// validate that the new latest output is as expected.
Types.OutputProposal memory proposal = oracle.getL2Output(latestBlockNumberAfter);
assertEq(newLatestOutput.outputRoot, proposal.outputRoot);
assertEq(newLatestOutput.timestamp, proposal.timestamp);
}
function test_deleteOutputs_multipleOutputs() external {
test_proposingAnotherOutput();
test_proposingAnotherOutput();
test_proposingAnotherOutput();
uint256 latestBlockNumber = oracle.latestBlockNumber();
Types.OutputProposal memory newLatestOutput = oracle.getL2Output(
latestBlockNumber - submissionInterval * 3
);
vm.prank(owner);
vm.expectEmit(true, true, false, false);
emit OutputsDeleted(latestBlockNumber - submissionInterval * 2);
oracle.deleteL2Outputs(latestBlockNumber - submissionInterval * 2);
// validate latestBlockNumber has been reduced
uint256 latestBlockNumberAfter = oracle.latestBlockNumber();
assertEq(latestBlockNumber - submissionInterval * 3, latestBlockNumberAfter);
// validate that the new latest output is as expected.
Types.OutputProposal memory proposal = oracle.getL2Output(latestBlockNumberAfter);
assertEq(newLatestOutput.outputRoot, proposal.outputRoot);
assertEq(newLatestOutput.timestamp, proposal.timestamp);
}
......@@ -307,40 +325,38 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
* Delete Tests - Sad Path *
***************************/
function testCannot_deleteL2Output_ifNotOwner() external {
function testCannot_deleteL2Outputs_ifNotOwner() external {
uint256 latestBlockNumber = oracle.latestBlockNumber();
Types.OutputProposal memory proposal = oracle.getL2Output(latestBlockNumber);
vm.expectRevert("Ownable: caller is not the owner");
oracle.deleteL2Output(proposal);
oracle.deleteL2Outputs(latestBlockNumber);
}
function testCannot_deleteL2Output_withWrongRoot() external {
function testCannot_deleteL2Outputs_nonExistent() external {
test_proposingAnotherOutput();
uint256 previousBlockNumber = oracle.latestBlockNumber() - submissionInterval;
Types.OutputProposal memory proposalToDelete = oracle.getL2Output(previousBlockNumber);
uint256 latestBlockNumber = oracle.latestBlockNumber();
vm.prank(owner);
vm.expectRevert(
"L2OutputOracle: output root to delete does not match the latest output proposal"
);
oracle.deleteL2Output(proposalToDelete);
vm.expectRevert("L2OutputOracle: cannot delete a non-existent output");
oracle.deleteL2Outputs(latestBlockNumber + 1);
}
function testCannot_deleteL2Output_withWrongTime() external {
function testCannot_deleteL2Outputs_afterLatest() external {
// Start by proposing three outputs
test_proposingAnotherOutput();
test_proposingAnotherOutput();
test_proposingAnotherOutput();
// Delete the latest two outputs
uint256 latestBlockNumber = oracle.latestBlockNumber();
Types.OutputProposal memory proposalToDelete = oracle.getL2Output(latestBlockNumber);
vm.prank(owner);
oracle.deleteL2Outputs(latestBlockNumber - submissionInterval * 2);
// Modify the timestamp so that it does not match.
proposalToDelete.timestamp -= 1;
// Now try to delete the same output again
vm.prank(owner);
vm.expectRevert(
"L2OutputOracle: timestamp to delete does not match the latest output proposal"
);
oracle.deleteL2Output(proposalToDelete);
vm.expectRevert("L2OutputOracle: cannot delete outputs after the latest block number");
oracle.deleteL2Outputs(latestBlockNumber - submissionInterval * 2);
}
}
......
packages/contracts-bedrock/contracts/test/OptimismPortal.t.sol
View file @ c71500a7
......@@ -242,7 +242,9 @@ contract OptimismPortal_Test is Portal_Initializer {
// The checkpointed block should not be finalized until 1 second from now.
assertEq(op.isBlockFinalized(checkpoint), false);
// Nor should a block after it
vm.expectRevert("L2OutputOracle: no output found for the given block number");
vm.expectRevert(
"L2OutputOracle: block number cannot be greater than the latest block number"
);
assertEq(op.isBlockFinalized(checkpoint + 1), false);
// Nor a block before it, even though the finalization period has passed, there is
// not yet a checkpoint block on top of it for which that is true.
......@@ -255,7 +257,9 @@ contract OptimismPortal_Test is Portal_Initializer {
// So should the block before it.
assertEq(op.isBlockFinalized(checkpoint - 1), true);
// But not the block after it.
vm.expectRevert("L2OutputOracle: no output found for the given block number");
vm.expectRevert(
"L2OutputOracle: block number cannot be greater than the latest block number"
);
assertEq(op.isBlockFinalized(checkpoint + 1), false);
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment