Skip to content

Commit

Permalink
fix(ctb): allow same L2OO owner and proposer
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
smartcontracts committed Nov 21, 2022
1 parent ee7abf3 commit ccaf5bc
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 38 deletions.
5 changes: 5 additions & 0 deletions .changeset/strange-geckos-lay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@eth-optimism/contracts-bedrock': patch
---

Allows owner and proposer addresses to be the same in L2OutputOracle
22 changes: 11 additions & 11 deletions op-bindings/bindings/l2outputoracle.go

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions packages/contracts-bedrock/.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ L2CrossDomainMessenger_Test:test_L2MessengerTwiceSendMessage() (gas: 134632)
L2CrossDomainMessenger_Test:test_L2MessengerXDomainSenderReverts() (gas: 10568)
L2CrossDomainMessenger_Test:test_L2MessengerxDomainMessageSenderResets() (gas: 52615)
L2OutputOracleTest:testCannot_ProposeWithUnmatchedBlockhash() (gas: 26850)
L2OutputOracleTest:testCannot_constructWithBadTimestamp() (gas: 50654)
L2OutputOracleTest:testCannot_constructWithBadTimestamp() (gas: 50456)
L2OutputOracleTest:testCannot_deleteL2Output_ifNotOwner() (gas: 25090)
L2OutputOracleTest:testCannot_deleteL2Output_withWrongRoot() (gas: 91399)
L2OutputOracleTest:testCannot_deleteL2Output_withWrongTime() (gas: 87382)
Expand All @@ -95,7 +95,7 @@ L2OutputOracleTest:testCannot_proposeFutureTimetamp() (gas: 26095)
L2OutputOracleTest:testCannot_proposeL2OutputIfNotProposer() (gas: 23563)
L2OutputOracleTest:testCannot_proposeOnWrongFork() (gas: 26368)
L2OutputOracleTest:testCannot_proposeUnexpectedBlockNumber() (gas: 26003)
L2OutputOracleTest:test_changeProposer() (gas: 55872)
L2OutputOracleTest:test_changeProposer() (gas: 47047)
L2OutputOracleTest:test_computeL2Timestamp() (gas: 30215)
L2OutputOracleTest:test_constructor() (gas: 45612)
L2OutputOracleTest:test_deleteOutput() (gas: 77197)
Expand All @@ -104,7 +104,7 @@ L2OutputOracleTest:test_latestBlockNumber() (gas: 76240)
L2OutputOracleTest:test_nextBlockNumber() (gas: 15187)
L2OutputOracleTest:test_proposeWithBlockhashAndHeight() (gas: 75044)
L2OutputOracleTest:test_proposingAnotherOutput() (gas: 76869)
L2OutputOracleTest:test_updateOwner() (gas: 46134)
L2OutputOracleTest:test_updateOwner() (gas: 36063)
L2OutputOracleUpgradeable_Test:test_cannotInitImpl() (gas: 17403)
L2OutputOracleUpgradeable_Test:test_cannotInitProxy() (gas: 22398)
L2OutputOracleUpgradeable_Test:test_initValuesOnProxy() (gas: 36094)
Expand Down
16 changes: 0 additions & 16 deletions packages/contracts-bedrock/contracts/L1/L2OutputOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ contract L2OutputOracle is OwnableUpgradeable, Semver {
address _proposer,
address _owner
) public initializer {
require(_proposer != _owner, "L2OutputOracle: proposer cannot be the same as the owner");
l2Outputs[STARTING_BLOCK_NUMBER] = Types.OutputProposal(_genesisL2Output, block.timestamp);
latestBlockNumber = STARTING_BLOCK_NUMBER;
__Ownable_init();
Expand Down Expand Up @@ -265,16 +264,6 @@ contract L2OutputOracle is OwnableUpgradeable, Semver {
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`).
* Can only be called by the current owner.
Expand All @@ -285,11 +274,6 @@ contract L2OutputOracle is OwnableUpgradeable, Semver {
"L2OutputOracle: new proposer cannot be the zero address"
);

require(
_newProposer != owner(),
"L2OutputOracle: proposer cannot be the same as the owner"
);

emit ProposerChanged(proposer, _newProposer);
proposer = _newProposer;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,6 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
vm.expectRevert("L2OutputOracle: new proposer cannot be the zero address");
oracle.changeProposer(address(0));

vm.expectRevert("L2OutputOracle: proposer cannot be the same as the owner");
oracle.changeProposer(owner);

// Double check proposer has not changed.
assertEq(proposer, oracle.proposer());

Expand All @@ -154,11 +151,6 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
assertEq(owner, oracle.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);
emit OwnershipTransferred(owner, newOwner);
oracle.transferOwnership(newOwner);
Expand Down

0 comments on commit ccaf5bc

Please sign in to comment.