Commit 30b3cddf authored by Michael Amadi's avatar Michael Amadi Committed by GitHub

Interop: Add new role for dependency set management (#11407)

* add role based auth for dependency related setters with tests

* support systemConfig initialize, rename foundation multisig to dependency manager

* use dependency manager role only, update SystemConfigInterop and Specs tests

* rebase to current develop head, run pre-pr script

* use constant hash with comments over computing at compile time, update semver-lock

* use dependencyManager() fn internally, import directly not indirectly in SystemConfigInterop
parent 18eebaa4
...@@ -60,8 +60,8 @@ ...@@ -60,8 +60,8 @@
"sourceCodeHash": "0xaed39fb8a0ce4b8d7a97ead42074e0c672fa18a58a57227b9d32abe2c3600223" "sourceCodeHash": "0xaed39fb8a0ce4b8d7a97ead42074e0c672fa18a58a57227b9d32abe2c3600223"
}, },
"src/L1/SystemConfigInterop.sol": { "src/L1/SystemConfigInterop.sol": {
"initCodeHash": "0x710484da188ec16a0ade97d99ecd4f5e910bc87ad01e448db4cf3f0e5050aaee", "initCodeHash": "0x42e1000d8542f9cbb197314d423760543d4d716d9640b2134c0d881557b22f4f",
"sourceCodeHash": "0x40d708140ee6345e146e358c169a191dbbc991782560a2dcbf90ba45a82f7117" "sourceCodeHash": "0xccb5b1ea5f1d5c4a583a2a6941db072fc3ad60ac3d08d085f17a672f6a7e2851"
}, },
"src/L2/BaseFeeVault.sol": { "src/L2/BaseFeeVault.sol": {
"initCodeHash": "0x623bf6892f0bdb536f2916bc9eb45e52012ad2c80893ff87d750757966b9be68", "initCodeHash": "0x623bf6892f0bdb536f2916bc9eb45e52012ad2c80893ff87d750757966b9be68",
......
...@@ -194,6 +194,19 @@ ...@@ -194,6 +194,19 @@
"stateMutability": "view", "stateMutability": "view",
"type": "function" "type": "function"
}, },
{
"inputs": [],
"name": "dependencyManager",
"outputs": [
{
"internalType": "address",
"name": "",
"type": "address"
}
],
"stateMutability": "view",
"type": "function"
},
{ {
"inputs": [], "inputs": [],
"name": "disputeGameFactory", "name": "disputeGameFactory",
...@@ -338,6 +351,133 @@ ...@@ -338,6 +351,133 @@
"name": "_batchInbox", "name": "_batchInbox",
"type": "address" "type": "address"
}, },
{
"components": [
{
"internalType": "address",
"name": "l1CrossDomainMessenger",
"type": "address"
},
{
"internalType": "address",
"name": "l1ERC721Bridge",
"type": "address"
},
{
"internalType": "address",
"name": "l1StandardBridge",
"type": "address"
},
{
"internalType": "address",
"name": "disputeGameFactory",
"type": "address"
},
{
"internalType": "address",
"name": "optimismPortal",
"type": "address"
},
{
"internalType": "address",
"name": "optimismMintableERC20Factory",
"type": "address"
},
{
"internalType": "address",
"name": "gasPayingToken",
"type": "address"
}
],
"internalType": "struct SystemConfig.Addresses",
"name": "_addresses",
"type": "tuple"
},
{
"internalType": "address",
"name": "_dependencyManager",
"type": "address"
}
],
"name": "initialize",
"outputs": [],
"stateMutability": "nonpayable",
"type": "function"
},
{
"inputs": [
{
"internalType": "address",
"name": "_owner",
"type": "address"
},
{
"internalType": "uint32",
"name": "_basefeeScalar",
"type": "uint32"
},
{
"internalType": "uint32",
"name": "_blobbasefeeScalar",
"type": "uint32"
},
{
"internalType": "bytes32",
"name": "_batcherHash",
"type": "bytes32"
},
{
"internalType": "uint64",
"name": "_gasLimit",
"type": "uint64"
},
{
"internalType": "address",
"name": "_unsafeBlockSigner",
"type": "address"
},
{
"components": [
{
"internalType": "uint32",
"name": "maxResourceLimit",
"type": "uint32"
},
{
"internalType": "uint8",
"name": "elasticityMultiplier",
"type": "uint8"
},
{
"internalType": "uint8",
"name": "baseFeeMaxChangeDenominator",
"type": "uint8"
},
{
"internalType": "uint32",
"name": "minimumBaseFee",
"type": "uint32"
},
{
"internalType": "uint32",
"name": "systemTxMaxGas",
"type": "uint32"
},
{
"internalType": "uint128",
"name": "maximumBaseFee",
"type": "uint128"
}
],
"internalType": "struct ResourceMetering.ResourceConfig",
"name": "_config",
"type": "tuple"
},
{
"internalType": "address",
"name": "_batchInbox",
"type": "address"
},
{ {
"components": [ "components": [
{ {
......
...@@ -8,12 +8,60 @@ import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; ...@@ -8,12 +8,60 @@ import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import { SystemConfig } from "src/L1/SystemConfig.sol"; import { SystemConfig } from "src/L1/SystemConfig.sol";
import { ConfigType } from "src/L2/L1BlockInterop.sol"; import { ConfigType } from "src/L2/L1BlockInterop.sol";
import { StaticConfig } from "src/libraries/StaticConfig.sol"; import { StaticConfig } from "src/libraries/StaticConfig.sol";
import { ResourceMetering } from "src/L1/ResourceMetering.sol";
import { Storage } from "src/libraries/Storage.sol";
/// @title SystemConfigInterop /// @title SystemConfigInterop
/// @notice The SystemConfig contract is used to manage configuration of an Optimism network. /// @notice The SystemConfig contract is used to manage configuration of an Optimism network.
/// All configuration is stored on L1 and picked up by L2 as part of the derviation of /// All configuration is stored on L1 and picked up by L2 as part of the derviation of
/// the L2 chain. /// the L2 chain.
contract SystemConfigInterop is SystemConfig { contract SystemConfigInterop is SystemConfig {
/// @notice Storage slot where the dependency manager address is stored
/// @dev Equal to bytes32(uint256(keccak256("systemconfig.dependencymanager")) - 1)
bytes32 internal constant DEPENDENCY_MANAGER_SLOT =
0x1708e077affb93e89be2665fb0fb72581be66f84dc00d25fed755ae911905b1c;
/// @notice Initializer.
/// @param _owner Initial owner of the contract.
/// @param _basefeeScalar Initial basefee scalar value.
/// @param _blobbasefeeScalar Initial blobbasefee scalar value.
/// @param _batcherHash Initial batcher hash.
/// @param _gasLimit Initial gas limit.
/// @param _unsafeBlockSigner Initial unsafe block signer address.
/// @param _config Initial ResourceConfig.
/// @param _batchInbox Batch inbox address. An identifier for the op-node to find
/// canonical data.
/// @param _addresses Set of L1 contract addresses. These should be the proxies.
/// @param _dependencyManager The addressed allowed to add/remove from the dependency set
function initialize(
address _owner,
uint32 _basefeeScalar,
uint32 _blobbasefeeScalar,
bytes32 _batcherHash,
uint64 _gasLimit,
address _unsafeBlockSigner,
ResourceMetering.ResourceConfig memory _config,
address _batchInbox,
SystemConfig.Addresses memory _addresses,
address _dependencyManager
)
external
initializer
{
initialize({
_owner: _owner,
_basefeeScalar: _basefeeScalar,
_blobbasefeeScalar: _blobbasefeeScalar,
_batcherHash: _batcherHash,
_gasLimit: _gasLimit,
_unsafeBlockSigner: _unsafeBlockSigner,
_config: _config,
_batchInbox: _batchInbox,
_addresses: _addresses
});
Storage.setAddress(DEPENDENCY_MANAGER_SLOT, _dependencyManager);
}
/// @custom:semver +interop /// @custom:semver +interop
function version() public pure override returns (string memory) { function version() public pure override returns (string memory) {
return string.concat(super.version(), "+interop"); return string.concat(super.version(), "+interop");
...@@ -48,19 +96,26 @@ contract SystemConfigInterop is SystemConfig { ...@@ -48,19 +96,26 @@ contract SystemConfigInterop is SystemConfig {
} }
} }
/// @notice Adds a chain to the interop dependency set. Can only be called by the owner. /// @notice Adds a chain to the interop dependency set. Can only be called by the dependency manager.
/// @param _chainId Chain ID of chain to add. /// @param _chainId Chain ID of chain to add.
function addDependency(uint256 _chainId) external onlyOwner { function addDependency(uint256 _chainId) external {
require(msg.sender == dependencyManager(), "SystemConfig: caller is not the dependency manager");
OptimismPortal(payable(optimismPortal())).setConfig( OptimismPortal(payable(optimismPortal())).setConfig(
ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId) ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)
); );
} }
/// @notice Removes a chain from the interop dependency set. Can only be called by the owner. /// @notice Removes a chain from the interop dependency set. Can only be called by the dependency manager
/// @param _chainId Chain ID of the chain to remove. /// @param _chainId Chain ID of the chain to remove.
function removeDependency(uint256 _chainId) external onlyOwner { function removeDependency(uint256 _chainId) external {
require(msg.sender == dependencyManager(), "SystemConfig: caller is not the dependency manager");
OptimismPortal(payable(optimismPortal())).setConfig( OptimismPortal(payable(optimismPortal())).setConfig(
ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(_chainId) ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(_chainId)
); );
} }
/// @notice getter for the dependency manager address
function dependencyManager() public view returns (address) {
return Storage.getAddress(DEPENDENCY_MANAGER_SLOT);
}
} }
...@@ -71,13 +71,13 @@ contract SystemConfigInterop_Test is CommonTest { ...@@ -71,13 +71,13 @@ contract SystemConfigInterop_Test is CommonTest {
) )
); );
vm.prank(systemConfig.owner()); vm.prank(_systemConfigInterop().dependencyManager());
_systemConfigInterop().addDependency(_chainId); _systemConfigInterop().addDependency(_chainId);
} }
/// @dev Tests that adding a dependency as not the owner reverts. /// @dev Tests that adding a dependency as not the dependency manager reverts.
function testFuzz_addDependency_notOwner_reverts(uint256 _chainId) public { function testFuzz_addDependency_notDependencyManager_reverts(uint256 _chainId) public {
vm.expectRevert("Ownable: caller is not the owner"); vm.expectRevert("SystemConfig: caller is not the dependency manager");
_systemConfigInterop().addDependency(_chainId); _systemConfigInterop().addDependency(_chainId);
} }
...@@ -91,13 +91,13 @@ contract SystemConfigInterop_Test is CommonTest { ...@@ -91,13 +91,13 @@ contract SystemConfigInterop_Test is CommonTest {
) )
); );
vm.prank(systemConfig.owner()); vm.prank(_systemConfigInterop().dependencyManager());
_systemConfigInterop().removeDependency(_chainId); _systemConfigInterop().removeDependency(_chainId);
} }
/// @dev Tests that removing a dependency as not the owner reverts. /// @dev Tests that removing a dependency as not the dependency manager reverts.
function testFuzz_removeDependency_notOwner_reverts(uint256 _chainId) public { function testFuzz_removeDependency_notDependencyManager_reverts(uint256 _chainId) public {
vm.expectRevert("Ownable: caller is not the owner"); vm.expectRevert("SystemConfig: caller is not the dependency manager");
_systemConfigInterop().removeDependency(_chainId); _systemConfigInterop().removeDependency(_chainId);
} }
......
...@@ -35,7 +35,8 @@ contract Specification_Test is CommonTest { ...@@ -35,7 +35,8 @@ contract Specification_Test is CommonTest {
DISPUTEGAMEFACTORYOWNER, DISPUTEGAMEFACTORYOWNER,
DELAYEDWETHOWNER, DELAYEDWETHOWNER,
COUNCILSAFE, COUNCILSAFE,
COUNCILSAFEOWNER COUNCILSAFEOWNER,
DEPENDENCYMANAGER
} }
/// @notice Represents the specification of a function. /// @notice Represents the specification of a function.
...@@ -528,11 +529,18 @@ contract Specification_Test is CommonTest { ...@@ -528,11 +529,18 @@ contract Specification_Test is CommonTest {
_addSpec({ _name: "SystemConfigInterop", _sel: _getSel("basefeeScalar()") }); _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("basefeeScalar()") });
_addSpec({ _name: "SystemConfigInterop", _sel: _getSel("blobbasefeeScalar()") }); _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("blobbasefeeScalar()") });
_addSpec({ _name: "SystemConfigInterop", _sel: _getSel("maximumGasLimit()") }); _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("maximumGasLimit()") });
_addSpec({ _name: "SystemConfigInterop", _sel: _getSel("addDependency(uint256)"), _auth: Role.SYSTEMCONFIGOWNER }); _addSpec({ _name: "SystemConfigInterop", _sel: _getSel("addDependency(uint256)"), _auth: Role.DEPENDENCYMANAGER });
_addSpec({ _addSpec({
_name: "SystemConfigInterop", _name: "SystemConfigInterop",
_sel: _getSel("removeDependency(uint256)"), _sel: _getSel("removeDependency(uint256)"),
_auth: Role.SYSTEMCONFIGOWNER _auth: Role.DEPENDENCYMANAGER
});
_addSpec({ _name: "SystemConfigInterop", _sel: _getSel("dependencyManager()") });
_addSpec({
_name: "SystemConfigInterop",
_sel: _getSel(
"initialize(address,uint32,uint32,bytes32,uint64,address,(uint32,uint8,uint8,uint32,uint32,uint128),address,(address,address,address,address,address,address,address),address)"
)
}); });
// ProxyAdmin // ProxyAdmin
......
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