Skip to content

Commit

Permalink
Add a separate manager role for adding and removing signers (#67)
Browse files Browse the repository at this point in the history
* Add a separate manager role for adding and removing signers

* forge fmt

* Fix comment

* Switch to owner or manager modifiers
  • Loading branch information
mdehoog authored Feb 11, 2025
1 parent 8b5d357 commit 3859c40
Show file tree
Hide file tree
Showing 5 changed files with 421 additions and 23 deletions.
254 changes: 240 additions & 14 deletions bindings/system_config_global.go

Large diffs are not rendered by default.

15 changes: 14 additions & 1 deletion contracts/script/DeploySystem.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pragma solidity ^0.8.15;

import {Deploy} from "@eth-optimism-bedrock/scripts/deploy/Deploy.s.sol";
import {DeployConfig} from "@eth-optimism-bedrock/scripts/deploy/DeployConfig.s.sol";
import {Config} from "@eth-optimism-bedrock/scripts/libraries/Config.sol";
import {Types} from "@eth-optimism-bedrock/scripts/libraries/Types.sol";
import {ChainAssertions} from "@eth-optimism-bedrock/scripts/deploy/ChainAssertions.sol";
import {SystemConfig} from "@eth-optimism-bedrock/src/L1/SystemConfig.sol";
Expand All @@ -23,6 +24,7 @@ import {ResourceMetering} from "@eth-optimism-bedrock/src/L1/ResourceMetering.so
import {IResourceMetering} from "@eth-optimism-bedrock/src/L1/interfaces/IResourceMetering.sol";
import {ICertManager} from "@nitro-validator/ICertManager.sol";

import {stdJson} from "forge-std/StdJson.sol";
import {console2 as console} from "forge-std/console2.sol";

contract DeploySystem is Deploy {
Expand Down Expand Up @@ -266,10 +268,21 @@ contract DeploySystem is Deploy {
address systemConfigGlobalProxy = mustGetAddress("SystemConfigGlobalProxy");
address systemConfigGlobal = mustGetAddress("SystemConfigGlobal");

string memory _json;
string memory _path = Config.deployConfigPath();
try vm.readFile(_path) returns (string memory data) {
_json = data;
} catch {
require(false, string.concat("Cannot find deploy config file at ", _path));
}
address systemConfigGlobalManager = stdJson.readAddress(_json, "$.systemConfigGlobalManager");

_upgradeAndCallViaSafe({
_proxy: payable(systemConfigGlobalProxy),
_implementation: systemConfigGlobal,
_innerCallData: abi.encodeWithSelector(SystemConfigGlobal.initialize.selector, cfg.finalSystemOwner())
_innerCallData: abi.encodeCall(
SystemConfigGlobal.initialize, (cfg.finalSystemOwner(), systemConfigGlobalManager)
)
});

SystemConfigGlobal config = SystemConfigGlobal(systemConfigGlobalProxy);
Expand Down
158 changes: 158 additions & 0 deletions contracts/src/OwnableManagedUpgradeable.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import {ContextUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol";

/**
* @dev Extension of OpenZepplin's OwnableUpgradeable contract that adds an additional manager role.
*/
abstract contract OwnableManagedUpgradeable is Initializable, ContextUpgradeable {
address private _owner;
address private _manager;

event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
event ManagementTransferred(address indexed previousManager, address indexed newManager);

/**
* @dev Initializes the contract setting the deployer as the initial owner + manager.
*/
function __OwnableManaged_init() internal onlyInitializing {
__OwnableManaged_init_unchained();
}

function __OwnableManaged_init_unchained() internal onlyInitializing {
_transferOwnership(_msgSender());
_transferManagement(_msgSender());
}

/**
* @dev Throws if called by any account other than the owner.
*/
modifier onlyOwner() {
_checkOwner();
_;
}

/**
* @dev Throws if called by any account other than the manager.
*/
modifier onlyManager() {
_checkManager();
_;
}

/**
* @dev Throws if called by any account other than the owner or manager.
*/
modifier onlyOwnerOrManager() {
_checkOwnerOrManager();
_;
}

/**
* @dev Returns the address of the current owner.
*/
function owner() public view virtual returns (address) {
return _owner;
}

/**
* @dev Returns the address of the current manager.
*/
function manager() public view virtual returns (address) {
return _manager;
}

/**
* @dev Throws if the sender is not the owner.
*/
function _checkOwner() internal view virtual {
require(owner() == _msgSender(), "OwnableManaged: caller is not the owner");
}

/**
* @dev Throws if the sender is not the manager.
*/
function _checkManager() internal view virtual {
require(manager() == _msgSender(), "OwnableManaged: caller is not the manager");
}

/**
* @dev Throws if the sender is not the owner or the manager.
*/
function _checkOwnerOrManager() internal view virtual {
require(
owner() == _msgSender() || manager() == _msgSender(),
"OwnableManaged: caller is not the owner or the manager"
);
}

/**
* @dev Leaves the contract without owner. It will not be possible to call
* `onlyOwner` functions anymore. Can only be called by the current owner.
*
* NOTE: Renouncing ownership will leave the contract without an owner,
* thereby removing any functionality that is only available to the owner.
*/
function renounceOwnership() public virtual onlyOwner {
_transferOwnership(address(0));
}

/**
* @dev Leaves the contract without manager. It will not be possible to call
* `onlyManager` functions anymore. Can only be called by the current owner or manager.
*
* NOTE: Renouncing management will leave the contract without an manager,
* thereby removing any functionality that is only available to the manager.
*/
function renounceManagement() public virtual onlyOwnerOrManager {
_transferManagement(address(0));
}

/**
* @dev Transfers ownership of the contract to a new account (`newOwner`).
* Can only be called by the current owner.
*/
function transferOwnership(address newOwner) public virtual onlyOwner {
require(newOwner != address(0), "OwnableManaged: new owner is the zero address");
_transferOwnership(newOwner);
}

/**
* @dev Transfers management of the contract to a new account (`newManager`).
* Can only be called by the current owner or manager.
*/
function transferManagement(address newManager) public virtual onlyOwnerOrManager {
require(newManager != address(0), "OwnableManaged: new manager is the zero address");
_transferManagement(newManager);
}

/**
* @dev Transfers ownership of the contract to a new account (`newOwner`).
* Internal function without access restriction.
*/
function _transferOwnership(address newOwner) internal virtual {
address oldOwner = _owner;
_owner = newOwner;
emit OwnershipTransferred(oldOwner, newOwner);
}

/**
* @dev Transfers management of the contract to a new account (`newManager`).
* Internal function without access restriction.
*/
function _transferManagement(address newManager) internal virtual {
address oldManager = _manager;
_manager = newManager;
emit ManagementTransferred(oldManager, newManager);
}

/**
* @dev This empty reserved space is put in place to allow future versions to add new
* variables without shifting down storage in the inheritance chain.
* See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
*/
uint256[48] private __gap;
}
15 changes: 8 additions & 7 deletions contracts/src/SystemConfigGlobal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

pragma solidity ^0.8.0;

import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import {OwnableManagedUpgradeable} from "./OwnableManagedUpgradeable.sol";
import {ISemver} from "@eth-optimism-bedrock/src/universal/interfaces/ISemver.sol";
import {NitroValidator} from "@nitro-validator/NitroValidator.sol";
import {LibBytes} from "@nitro-validator/LibBytes.sol";
import {LibCborElement, CborElement, CborDecode} from "@nitro-validator/CborDecode.sol";
import {ICertManager} from "@nitro-validator/ICertManager.sol";

contract SystemConfigGlobal is OwnableUpgradeable, ISemver, NitroValidator {
contract SystemConfigGlobal is OwnableManagedUpgradeable, ISemver, NitroValidator {
using LibBytes for bytes;
using CborDecode for bytes;
using LibCborElement for CborElement;
Expand All @@ -32,12 +32,13 @@ contract SystemConfigGlobal is OwnableUpgradeable, ISemver, NitroValidator {
}

constructor(ICertManager certManager) NitroValidator(certManager) {
initialize({_owner: address(0xdEaD)});
initialize({_owner: address(0xdEaD), _manager: address(0xdEaD)});
}

function initialize(address _owner) public initializer {
__Ownable_init();
function initialize(address _owner, address _manager) public initializer {
__OwnableManaged_init();
transferOwnership(_owner);
transferManagement(_manager);
}

function setProposer(address _proposer) external onlyOwner {
Expand All @@ -52,7 +53,7 @@ contract SystemConfigGlobal is OwnableUpgradeable, ISemver, NitroValidator {
delete validPCR0s[keccak256(pcr0)];
}

function registerSigner(bytes calldata attestationTbs, bytes calldata signature) external onlyOwner {
function registerSigner(bytes calldata attestationTbs, bytes calldata signature) external onlyOwnerOrManager {
Ptrs memory ptrs = validateAttestation(attestationTbs, signature);
bytes32 pcr0 = attestationTbs.keccak(ptrs.pcrs[0]);
require(validPCR0s[pcr0], "invalid pcr0 in attestation");
Expand All @@ -66,7 +67,7 @@ contract SystemConfigGlobal is OwnableUpgradeable, ISemver, NitroValidator {
validSigners[enclaveAddress] = true;
}

function deregisterSigner(address signer) external onlyOwner {
function deregisterSigner(address signer) external onlyOwnerOrManager {
delete validSigners[signer];
}
}
2 changes: 1 addition & 1 deletion contracts/test/OutputOracle.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ contract OutputOracleTest is Test {
SystemConfigGlobal scgImpl = new SystemConfigGlobal(ICertManager(address(0)));
SystemConfigGlobal scg =
SystemConfigGlobal(ResolvingProxyFactory.setupProxy(address(scgImpl), address(admin), 0x00));
scg.initialize({_owner: address(this)});
scg.initialize({_owner: address(this), _manager: address(this)});
scg.setProposer(address(this));
OutputOracle outputOracleImpl = new OutputOracle({_systemConfigGlobal: scg, _maxOutputCount: 6});
outputOracle = OutputOracle(ResolvingProxyFactory.setupProxy(address(outputOracleImpl), address(admin), 0x00));
Expand Down

0 comments on commit 3859c40

Please sign in to comment.