Commit ccaf5bc8 authored by Kelvin Fichter's avatar Kelvin Fichter

fix(ctb): allow same L2OO owner and proposer

Tweaks the L2OutputOracle to allow owner and proposer addresses to be
the same. Extra logic required to prevent this case added smart contract
code to a critical contract and it's unlikely that this will be a
problem in practice.
parent ee7abf3b
---
'@eth-optimism/contracts-bedrock': patch
---
Allows owner and proposer addresses to be the same in L2OutputOracle
This diff is collapsed.
...@@ -86,7 +86,7 @@ L2CrossDomainMessenger_Test:test_L2MessengerTwiceSendMessage() (gas: 134632) ...@@ -86,7 +86,7 @@ L2CrossDomainMessenger_Test:test_L2MessengerTwiceSendMessage() (gas: 134632)
L2CrossDomainMessenger_Test:test_L2MessengerXDomainSenderReverts() (gas: 10568) L2CrossDomainMessenger_Test:test_L2MessengerXDomainSenderReverts() (gas: 10568)
L2CrossDomainMessenger_Test:test_L2MessengerxDomainMessageSenderResets() (gas: 52615) L2CrossDomainMessenger_Test:test_L2MessengerxDomainMessageSenderResets() (gas: 52615)
L2OutputOracleTest:testCannot_ProposeWithUnmatchedBlockhash() (gas: 26850) L2OutputOracleTest:testCannot_ProposeWithUnmatchedBlockhash() (gas: 26850)
L2OutputOracleTest:testCannot_constructWithBadTimestamp() (gas: 50654) L2OutputOracleTest:testCannot_constructWithBadTimestamp() (gas: 50456)
L2OutputOracleTest:testCannot_deleteL2Output_ifNotOwner() (gas: 25090) L2OutputOracleTest:testCannot_deleteL2Output_ifNotOwner() (gas: 25090)
L2OutputOracleTest:testCannot_deleteL2Output_withWrongRoot() (gas: 91399) L2OutputOracleTest:testCannot_deleteL2Output_withWrongRoot() (gas: 91399)
L2OutputOracleTest:testCannot_deleteL2Output_withWrongTime() (gas: 87382) L2OutputOracleTest:testCannot_deleteL2Output_withWrongTime() (gas: 87382)
...@@ -95,7 +95,7 @@ L2OutputOracleTest:testCannot_proposeFutureTimetamp() (gas: 26095) ...@@ -95,7 +95,7 @@ L2OutputOracleTest:testCannot_proposeFutureTimetamp() (gas: 26095)
L2OutputOracleTest:testCannot_proposeL2OutputIfNotProposer() (gas: 23563) L2OutputOracleTest:testCannot_proposeL2OutputIfNotProposer() (gas: 23563)
L2OutputOracleTest:testCannot_proposeOnWrongFork() (gas: 26368) L2OutputOracleTest:testCannot_proposeOnWrongFork() (gas: 26368)
L2OutputOracleTest:testCannot_proposeUnexpectedBlockNumber() (gas: 26003) L2OutputOracleTest:testCannot_proposeUnexpectedBlockNumber() (gas: 26003)
L2OutputOracleTest:test_changeProposer() (gas: 55872) L2OutputOracleTest:test_changeProposer() (gas: 47047)
L2OutputOracleTest:test_computeL2Timestamp() (gas: 30215) L2OutputOracleTest:test_computeL2Timestamp() (gas: 30215)
L2OutputOracleTest:test_constructor() (gas: 45612) L2OutputOracleTest:test_constructor() (gas: 45612)
L2OutputOracleTest:test_deleteOutput() (gas: 77197) L2OutputOracleTest:test_deleteOutput() (gas: 77197)
...@@ -104,7 +104,7 @@ L2OutputOracleTest:test_latestBlockNumber() (gas: 76240) ...@@ -104,7 +104,7 @@ L2OutputOracleTest:test_latestBlockNumber() (gas: 76240)
L2OutputOracleTest:test_nextBlockNumber() (gas: 15187) L2OutputOracleTest:test_nextBlockNumber() (gas: 15187)
L2OutputOracleTest:test_proposeWithBlockhashAndHeight() (gas: 75044) L2OutputOracleTest:test_proposeWithBlockhashAndHeight() (gas: 75044)
L2OutputOracleTest:test_proposingAnotherOutput() (gas: 76869) L2OutputOracleTest:test_proposingAnotherOutput() (gas: 76869)
L2OutputOracleTest:test_updateOwner() (gas: 46134) L2OutputOracleTest:test_updateOwner() (gas: 36063)
L2OutputOracleUpgradeable_Test:test_cannotInitImpl() (gas: 17403) L2OutputOracleUpgradeable_Test:test_cannotInitImpl() (gas: 17403)
L2OutputOracleUpgradeable_Test:test_cannotInitProxy() (gas: 22398) L2OutputOracleUpgradeable_Test:test_cannotInitProxy() (gas: 22398)
L2OutputOracleUpgradeable_Test:test_initValuesOnProxy() (gas: 36094) L2OutputOracleUpgradeable_Test:test_initValuesOnProxy() (gas: 36094)
......
...@@ -140,7 +140,6 @@ contract L2OutputOracle is OwnableUpgradeable, Semver { ...@@ -140,7 +140,6 @@ contract L2OutputOracle is OwnableUpgradeable, Semver {
address _proposer, address _proposer,
address _owner address _owner
) public initializer { ) public initializer {
require(_proposer != _owner, "L2OutputOracle: proposer cannot be the same as the owner");
l2Outputs[STARTING_BLOCK_NUMBER] = Types.OutputProposal(_genesisL2Output, block.timestamp); l2Outputs[STARTING_BLOCK_NUMBER] = Types.OutputProposal(_genesisL2Output, block.timestamp);
latestBlockNumber = STARTING_BLOCK_NUMBER; latestBlockNumber = STARTING_BLOCK_NUMBER;
__Ownable_init(); __Ownable_init();
...@@ -265,16 +264,6 @@ contract L2OutputOracle is OwnableUpgradeable, Semver { ...@@ -265,16 +264,6 @@ contract L2OutputOracle is OwnableUpgradeable, Semver {
return output; return output;
} }
/**
* @notice Overrides the standard implementation of transferOwnership
* to add the requirement that the owner and proposer are distinct.
* Can only be called by the current owner.
*/
function transferOwnership(address _newOwner) public override onlyOwner {
require(_newOwner != proposer, "L2OutputOracle: owner cannot be the same as the proposer");
super.transferOwnership(_newOwner);
}
/** /**
* @notice Transfers the proposer role to a new account (`newProposer`). * @notice Transfers the proposer role to a new account (`newProposer`).
* Can only be called by the current owner. * Can only be called by the current owner.
...@@ -285,11 +274,6 @@ contract L2OutputOracle is OwnableUpgradeable, Semver { ...@@ -285,11 +274,6 @@ contract L2OutputOracle is OwnableUpgradeable, Semver {
"L2OutputOracle: new proposer cannot be the zero address" "L2OutputOracle: new proposer cannot be the zero address"
); );
require(
_newProposer != owner(),
"L2OutputOracle: proposer cannot be the same as the owner"
);
emit ProposerChanged(proposer, _newProposer); emit ProposerChanged(proposer, _newProposer);
proposer = _newProposer; proposer = _newProposer;
} }
......
...@@ -132,9 +132,6 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer { ...@@ -132,9 +132,6 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
vm.expectRevert("L2OutputOracle: new proposer cannot be the zero address"); vm.expectRevert("L2OutputOracle: new proposer cannot be the zero address");
oracle.changeProposer(address(0)); oracle.changeProposer(address(0));
vm.expectRevert("L2OutputOracle: proposer cannot be the same as the owner");
oracle.changeProposer(owner);
// Double check proposer has not changed. // Double check proposer has not changed.
assertEq(proposer, oracle.proposer()); assertEq(proposer, oracle.proposer());
...@@ -154,11 +151,6 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer { ...@@ -154,11 +151,6 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
assertEq(owner, oracle.owner()); assertEq(owner, oracle.owner());
vm.startPrank(owner); vm.startPrank(owner);
vm.expectRevert("L2OutputOracle: owner cannot be the same as the proposer");
oracle.transferOwnership(proposer);
// Double check owner has not changed.
assertEq(owner, oracle.owner());
vm.expectEmit(true, true, true, true); vm.expectEmit(true, true, true, true);
emit OwnershipTransferred(owner, newOwner); emit OwnershipTransferred(owner, newOwner);
oracle.transferOwnership(newOwner); oracle.transferOwnership(newOwner);
......
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