Skip to content

Commit

Permalink
feat(ctb): Remove SAURON role (ethereum-optimism#9452)
Browse files Browse the repository at this point in the history 
* Remove `SAURON` role

* bindings
  • Loading branch information
@clabby
clabby authored Feb 9, 2024
1 parent ab3a99e commit 8a3e411
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 94 deletions.
35 changes: 2 additions & 33 deletions op-bindings/bindingspreview/optimismportal2.go
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion op-bindings/bindingspreview/optimismportal2_more.go
View file

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions packages/contracts-bedrock/semver-lock.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
"sourceCodeHash": "0xdc27421279afb6c3b26fc8c589c5d213695f666c74d2c2c41cb7df719d172f37"
},
"src/L1/OptimismPortal2.sol": {
"initCodeHash": "0xbbf753c1df3e4eabdd910124948afc5bfda7e219ece0f3a16804ea2129d512ed",
"sourceCodeHash": "0x78457c09e7e80b1e950f794999dcd5f8426dde850793fbaf9aba9718d29d9141"
"initCodeHash": "0x3c081d9769220a8b4a7538830905be3ebb058d0c01312bdbd9feae7f707a2866",
"sourceCodeHash": "0xc853bb1763d0b6b2008ebfa14148cd0840691f412873fed4ac7d203b7c69e7a7"
},
"src/L1/ProtocolVersions.sol": {
"initCodeHash": "0x72cd467e8bcf019c02675d72ab762e088bcc9cc0f1a4e9f587fa4589f7fdd1b8",
Expand Down
13 changes: 0 additions & 13 deletions packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,19 +474,6 @@
"stateMutability": "view",
"type": "function"
},
{
"inputs": [],
"name": "sauron",
"outputs": [
{
"internalType": "address",
"name": "",
"type": "address"
}
],
"stateMutability": "pure",
"type": "function"
},
{
"inputs": [
{
Expand Down
18 changes: 3 additions & 15 deletions packages/contracts-bedrock/src/L1/OptimismPortal2.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
uint64 timestamp;
}

/// @dev Remove this in favor of a configurable sauron role. This should probably live in the superchain config,
/// but need to confirm with security.
address internal constant SAURON = address(0xdead);

/// @notice The delay between when a withdrawal transaction is proven and when it may be finalized.
uint256 internal immutable PROOF_MATURITY_DELAY_SECONDS;

Expand Down Expand Up @@ -183,14 +179,6 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
return superchainConfig.guardian();
}

/// @notice Getter function for the address of the sauron role.
/// Public getter is legacy and will be removed in the future. Use `SuperchainConfig.sauron()` instead
/// once it's added.
/// @custom:deprecated
function sauron() public pure returns (address) {
return SAURON;
}

/// @notice Getter for the current paused status.
function paused() public view returns (bool) {
return superchainConfig.paused();
Expand Down Expand Up @@ -422,23 +410,23 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
/// @notice Blacklists a dispute game. Should only be used in the event that a dispute game resolves incorrectly.
/// @param _disputeGame Dispute game to blacklist.
function blacklistDisputeGame(IDisputeGame _disputeGame) external {
require(msg.sender == SAURON, "OptimismPortal: only sauron can blacklist dispute games");
require(msg.sender == guardian(), "OptimismPortal: only the guardian can blacklist dispute games");
disputeGameBlacklist[_disputeGame] = true;
}

/// @notice Deletes a proven withdrawal from the `provenWithdrawals` mapping in the case that a MPT proof was
/// incorrectly verified by the `MerkleTrie` verifier contract.
/// @param _withdrawalHash Hash of the withdrawal transaction to delete from the `pendingWithdrawals` mapping.
function deleteProvenWithdrawal(bytes32 _withdrawalHash) external {
require(msg.sender == SAURON, "OptimismPortal: only sauron can delete proven withdrawals");
require(msg.sender == guardian(), "OptimismPortal: only the guardian can delete proven withdrawals");
delete provenWithdrawals[_withdrawalHash];
}

/// @notice Sets the respected game type. Changing this value can alter the security properties of the system,
/// depending on the new game's behavior.
/// @param _gameType The game type to consult for output proposals.
function setRespectedGameType(GameType _gameType) external {
require(msg.sender == SAURON, "OptimismPortal: only sauron can set the respected game type");
require(msg.sender == guardian(), "OptimismPortal: only the guardian can set the respected game type");
respectedGameType = _gameType;
}

Expand Down
50 changes: 25 additions & 25 deletions packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,16 +359,16 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
assertFalse(optimismPortal2.finalizedWithdrawals(Hashing.hashWithdrawal(_defaultTx)));
}

/// @dev Tests that `deleteProvenWithdrawal` reverts when called by a non-SAURON.
function testFuzz_deleteProvenWithdrawal_onlySauron_reverts(address _act, bytes32 _wdHash) external {
vm.assume(_act != address(optimismPortal2.sauron()));
/// @dev Tests that `deleteProvenWithdrawal` reverts when called by a non-guardian.
function testFuzz_deleteProvenWithdrawal_onlyGuardian_reverts(address _act, bytes32 _wdHash) external {
vm.assume(_act != address(optimismPortal2.guardian()));

vm.expectRevert("OptimismPortal: only sauron can delete proven withdrawals");
vm.expectRevert("OptimismPortal: only the guardian can delete proven withdrawals");
optimismPortal2.deleteProvenWithdrawal(_wdHash);
}

/// @dev Tests that the SAURON role can delete any proven withdrawal.
function test_deleteProvenWithdrawal_sauron_succeeds() external {
/// @dev Tests that the guardian role can delete any proven withdrawal.
function test_deleteProvenWithdrawal_guardian_succeeds() external {
vm.expectEmit(true, true, true, true);
emit WithdrawalProven(_withdrawalHash, alice, bob);
optimismPortal2.proveWithdrawalTransaction({
Expand All @@ -383,42 +383,42 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
assertEq(timestamp, block.timestamp);

// Delete the proven withdrawal.
vm.prank(optimismPortal2.sauron());
vm.prank(optimismPortal2.guardian());
optimismPortal2.deleteProvenWithdrawal(_withdrawalHash);

// Ensure the withdrawal has been deleted
(, timestamp) = optimismPortal2.provenWithdrawals(_withdrawalHash);
assertEq(timestamp, 0);
}

/// @dev Tests that `deleteProvenWithdrawal` reverts when called by a non-SAURON.
function testFuzz_blacklist_onlySauron_reverts(address _act) external {
vm.assume(_act != address(optimismPortal2.sauron()));
/// @dev Tests that `deleteProvenWithdrawal` reverts when called by a non-guardian.
function testFuzz_blacklist_onlyGuardian_reverts(address _act) external {
vm.assume(_act != address(optimismPortal2.guardian()));

vm.expectRevert("OptimismPortal: only sauron can blacklist dispute games");
vm.expectRevert("OptimismPortal: only the guardian can blacklist dispute games");
optimismPortal2.blacklistDisputeGame(IDisputeGame(address(0xdead)));
}

/// @dev Tests that the SAURON role can blacklist any dispute game.
function testFuzz_blacklist_sauron_succeeds(address _addr) external {
vm.prank(optimismPortal2.sauron());
/// @dev Tests that the guardian role can blacklist any dispute game.
function testFuzz_blacklist_guardian_succeeds(address _addr) external {
vm.prank(optimismPortal2.guardian());
optimismPortal2.blacklistDisputeGame(IDisputeGame(_addr));

assertTrue(optimismPortal2.disputeGameBlacklist(IDisputeGame(_addr)));
}

/// @dev Tests that `setRespectedGameType` reverts when called by a non-SAURON.
function testFuzz_setRespectedGameType_onlySauron_reverts(address _act, GameType _ty) external {
vm.assume(_act != address(optimismPortal2.sauron()));
/// @dev Tests that `setRespectedGameType` reverts when called by a non-guardian.
function testFuzz_setRespectedGameType_onlyGuardian_reverts(address _act, GameType _ty) external {
vm.assume(_act != address(optimismPortal2.guardian()));

vm.prank(_act);
vm.expectRevert("OptimismPortal: only sauron can set the respected game type");
vm.expectRevert("OptimismPortal: only the guardian can set the respected game type");
optimismPortal2.setRespectedGameType(_ty);
}

/// @dev Tests that the SAURON role can set the respected game type to anything they want.
function testFuzz_setRespectedGameType_sauron_succeeds(GameType _ty) external {
vm.prank(optimismPortal2.sauron());
/// @dev Tests that the guardian role can set the respected game type to anything they want.
function testFuzz_setRespectedGameType_guardian_succeeds(GameType _ty) external {
vm.prank(optimismPortal2.guardian());
optimismPortal2.setRespectedGameType(_ty);

assertEq(optimismPortal2.respectedGameType().raw(), _ty.raw());
Expand Down Expand Up @@ -528,7 +528,7 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
});

// Blacklist the dispute dispute game.
vm.prank(optimismPortal2.sauron());
vm.prank(optimismPortal2.guardian());
optimismPortal2.blacklistDisputeGame(IDisputeGame(address(game)));

vm.expectEmit(true, true, true, true);
Expand Down Expand Up @@ -579,7 +579,7 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
});

// Update the respected game type to 0xbeef.
vm.prank(optimismPortal2.sauron());
vm.prank(optimismPortal2.guardian());
optimismPortal2.setRespectedGameType(GameType.wrap(0xbeef));

// Create a new game and mock the game type as 0xbeef in the factory.
Expand Down Expand Up @@ -964,7 +964,7 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
game.resolveClaim(0);
game.resolve();

vm.prank(optimismPortal2.sauron());
vm.prank(optimismPortal2.guardian());
optimismPortal2.blacklistDisputeGame(IDisputeGame(address(game)));

vm.warp(block.timestamp + optimismPortal2.proofMaturityDelaySeconds() + 1);
Expand Down Expand Up @@ -1022,7 +1022,7 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
game.resolve();

// Change the respected game type in the portal.
vm.prank(optimismPortal2.sauron());
vm.prank(optimismPortal2.guardian());
optimismPortal2.setRespectedGameType(GameType.wrap(0xFF));

vm.expectRevert("OptimismPortal: invalid game type");
Expand Down
8 changes: 3 additions & 5 deletions packages/contracts-bedrock/test/Specs.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ contract Specification_Test is CommonTest {
CHALLENGER,
SYSTEMCONFIGOWNER,
GUARDIAN,
SAURON,
MESSENGER,
L1PROXYADMINOWNER,
GOVERNANCETOKENOWNER,
Expand Down Expand Up @@ -236,7 +235,6 @@ contract Specification_Test is CommonTest {
});
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("finalizedWithdrawals(bytes32)") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("guardian()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("sauron()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("initialize(address,address,address)") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("l2Sender()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("minimumGasLimit(uint64)") });
Expand All @@ -250,9 +248,9 @@ contract Specification_Test is CommonTest {
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("disputeGameFactory()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("disputeGameBlacklist(address)") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("respectedGameType()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("blacklistDisputeGame(address)"), _auth: Role.SAURON });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("deleteProvenWithdrawal(bytes32)"), _auth: Role.SAURON });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("setRespectedGameType(uint32)"), _auth: Role.SAURON });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("blacklistDisputeGame(address)"), _auth: Role.GUARDIAN });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("deleteProvenWithdrawal(bytes32)"), _auth: Role.GUARDIAN });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("setRespectedGameType(uint32)"), _auth: Role.GUARDIAN });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("checkWithdrawal(bytes32)") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("proofMaturityDelaySeconds()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("disputeGameFinalityDelaySeconds()") });
Expand Down

0 comments on commit 8a3e411

Please sign in to comment.