Skip to content

N
nebula
Commit 66b29f34 authored by Mark Tyneway's avatar Mark Tyneway
Browse files
Options

system-config: refactor, test coverage

parent ba6b2a74
Hide whitespace changes
Inline Side-by-side
Showing with 117 additions and 35 deletions
packages/contracts-bedrock/.gas-snapshot
View file @ 66b29f34
......@@ -570,8 +570,10 @@ SequencerFeeVault_Test:test_withdraw_toL1_succeeds() (gas: 171357)
SetPrevBaseFee_Test:test_setPrevBaseFee_succeeds() (gas: 11549)
StandardBridge_Stateless_Test:test_isCorrectTokenPair_succeeds() (gas: 49936)
StandardBridge_Stateless_Test:test_isOptimismMintableERC20_succeeds() (gas: 33072)
SystemConfig_Initialize_Test:test_initialize_startBlockOverride_succeeds() (gas: 55905)
SystemConfig_Initialize_Test:test_initialize_values_succeeds() (gas: 64908)
SystemConfig_Initialize_TestFail:test_initialize_lowGasLimit_reverts() (gas: 94784)
SystemConfig_Initialize_TestFail:test_initialize_lowGasLimit_reverts() (gas: 94982)
SystemConfig_Initialize_TestFail:test_initialize_startBlock_reverts() (gas: 64772)
SystemConfig_Setters_TestFail:test_setBatcherHash_notOwner_reverts() (gas: 15607)
SystemConfig_Setters_TestFail:test_setGasConfig_notOwner_reverts() (gas: 15577)
SystemConfig_Setters_TestFail:test_setGasLimit_notOwner_reverts() (gas: 15676)
......
packages/contracts-bedrock/.storage-layout
View file @ 66b29f34
......@@ -89,6 +89,7 @@
| batcherHash | bytes32 | 103 | 0 | 32 | src/L1/SystemConfig.sol:SystemConfig |
| gasLimit | uint64 | 104 | 0 | 8 | src/L1/SystemConfig.sol:SystemConfig |
| _resourceConfig | struct ResourceMetering.ResourceConfig | 105 | 0 | 32 | src/L1/SystemConfig.sol:SystemConfig |
| startBlock | uint256 | 106 | 0 | 32 | src/L1/SystemConfig.sol:SystemConfig |
=======================
➡ src/legacy/DeployerWhitelist.sol:DeployerWhitelist
......
packages/contracts-bedrock/semver-lock.json
View file @ 66b29f34
......@@ -4,7 +4,7 @@
"src/L1/L1StandardBridge.sol": "0xa35dc0ab143043063c3bff73c8b065e401c23296a2017258ce8a87f4a4da9416",
"src/L1/L2OutputOracle.sol": "0x8f32ccb4c5cb63a459a0e79ee412177dc03fd279fdaaf1dac69e8c714902e857",
"src/L1/OptimismPortal.sol": "0xeaa47a63e8a3bcfdb7dfd3e6c8608369e34e362d9de82f3acf13cbc27c070bf7",
"src/L1/SystemConfig.sol": "0x6ee7793ce1fb655dae54f1fb639188b199001d406dfac18e2911969508aa6002",
"src/L1/SystemConfig.sol": "0xdf4524ecd93daf9ab84ab21871e8ce9d98d3b1a9326f58264577226fc3c5a998",
"src/L2/BaseFeeVault.sol": "0xa596e60762f16192cfa86459fcb9f4da9d8f756106eedac643a1ffeafbbfcc5f",
"src/L2/GasPriceOracle.sol": "0xc735a8bf01ad8bca194345748537bfd9924909c0342bc133c4a31e2fb8cb9882",
"src/L2/L1Block.sol": "0x7fbfc8b4da630386636c665570321fdec287b0867dbe0f91c2e7cd5b7697c220",
......
packages/contracts-bedrock/src/L1/SystemConfig.sol
View file @ 66b29f34
......@@ -183,15 +183,7 @@ contract SystemConfig is OwnableUpgradeable, Semver {
_setAddress(_addresses.optimismPortal, OPTIMISM_PORTAL_SLOT);
_setAddress(_addresses.optimismMintableERC20Factory, OPTIMISM_MINTABLE_ERC20_FACTORY_SLOT);
// The start block for the op-node to start searching for logs
// needs to be set in a backwards compatible way. Only allow setting
// the start block with an override if it has previously never been set.
if (_startBlock != 0) {
require(startBlock == 0, "SystemConfig: cannot override an already set start block");
startBlock = _startBlock;
} else if (startBlock == 0) {
startBlock = block.number;
}
_setStartBlock(_startBlock);
_setResourceConfig(_config);
require(_gasLimit >= minimumGasLimit(), "SystemConfig: gas limit too low");
......@@ -279,6 +271,20 @@ contract SystemConfig is OwnableUpgradeable, Semver {
return _getAddress(BATCH_INBOX_SLOT);
}
/// @notice Sets the start block in a backwards compatible way. Proxies
/// that were initialized before this existed will not have it
/// be set by `block.number`.
function _setStartBlock(uint256 _startBlock) internal {
if (_startBlock != 0) {
// There is an override, it cannot already be set.
require(startBlock == 0, "SystemConfig: cannot override an already set start block");
startBlock = _startBlock;
} else if (startBlock == 0) {
// There is no override and it is not set in storage. Set it to the block number.
startBlock = block.number;
}
}
/// @notice Updates the unsafe block signer address.
/// @param _unsafeBlockSigner New unsafe block signer address.
function setUnsafeBlockSigner(address _unsafeBlockSigner) external onlyOwner {
......
packages/contracts-bedrock/test/SystemConfig.t.sol
View file @ 66b29f34
......@@ -44,14 +44,14 @@ contract SystemConfig_Init is CommonTest {
abi.encodeCall(
SystemConfig.initialize,
(
alice, //_owner,
overhead, //_overhead,
scalar, //_scalar,
alice, // _owner,
overhead, // _overhead,
scalar, // _scalar,
batcherHash, // _batcherHash
gasLimit, //_gasLimit,
unsafeBlockSigner, //_unsafeBlockSigner,
Constants.DEFAULT_RESOURCE_CONFIG(), //_config,
0, //_startBlock
gasLimit, // _gasLimit,
unsafeBlockSigner, // _unsafeBlockSigner,
Constants.DEFAULT_RESOURCE_CONFIG(), // _config,
0, // _startBlock
batchInbox, // _batchInbox
SystemConfig.Addresses({ // _addresses
l1CrossDomainMessenger: l1CrossDomainMessenger,
......@@ -97,6 +97,47 @@ contract SystemConfig_Initialize_Test is SystemConfig_Init {
assertEq(actual.systemTxMaxGas, cfg.systemTxMaxGas);
assertEq(actual.maximumBaseFee, cfg.maximumBaseFee);
}
/// @dev Ensures that the start block override can be used to set the start block.
function test_initialize_startBlockOverride_succeeds() external {
uint256 startBlock = 100;
// Wipe out the initialized slot so the proxy can be initialized again
vm.store(address(sysConf), bytes32(0), bytes32(0));
assertEq(sysConf.startBlock(), block.number);
// the startBlock slot is 106, wipe it out
vm.store(address(sysConf), bytes32(uint256(106)), bytes32(0));
assertEq(sysConf.startBlock(), 0);
vm.prank(multisig);
Proxy(payable(address(sysConf))).upgradeToAndCall(
address(systemConfigImpl),
abi.encodeCall(
SystemConfig.initialize,
(
alice, // _owner,
overhead, // _overhead,
scalar, // _scalar,
batcherHash, // _batcherHash
gasLimit, // _gasLimit,
unsafeBlockSigner, // _unsafeBlockSigner,
Constants.DEFAULT_RESOURCE_CONFIG(), // _config,
startBlock, // _startBlock
batchInbox, // _batchInbox
SystemConfig.Addresses({ // _addresses
l1CrossDomainMessenger: l1CrossDomainMessenger,
l1ERC721Bridge: l1ERC721Bridge,
l1StandardBridge: l1StandardBridge,
l2OutputOracle: l2OutputOracle,
optimismPortal: optimismPortal,
optimismMintableERC20Factory: optimismMintableERC20Factory
})
)
)
);
assertEq(sysConf.startBlock(), startBlock);
}
}
contract SystemConfig_Initialize_TestFail is SystemConfig_Init {
......@@ -104,15 +145,7 @@ contract SystemConfig_Initialize_TestFail is SystemConfig_Init {
function test_initialize_lowGasLimit_reverts() external {
uint64 minimumGasLimit = sysConf.minimumGasLimit();
ResourceMetering.ResourceConfig memory cfg = ResourceMetering.ResourceConfig({
maxResourceLimit: 20_000_000,
elasticityMultiplier: 10,
baseFeeMaxChangeDenominator: 8,
minimumBaseFee: 1 gwei,
systemTxMaxGas: 1_000_000,
maximumBaseFee: type(uint128).max
});
// Wipe out the initialized slot so the proxy can be initialized again
vm.store(address(sysConf), bytes32(0), bytes32(0));
vm.prank(multisig);
// The call to initialize reverts due to: "SystemConfig: gas limit too low"
......@@ -123,14 +156,14 @@ contract SystemConfig_Initialize_TestFail is SystemConfig_Init {
abi.encodeCall(
SystemConfig.initialize,
(
alice, //_owner,
2100, //_overhead,
1000000, //_scalar,
bytes32(hex"abcd"), //_batcherHash,
minimumGasLimit - 1, //_gasLimit,
address(1), //_unsafeBlockSigner,
cfg, //_config,
0, //_startBlock
alice, // _owner,
2100, // _overhead,
1000000, // _scalar,
bytes32(hex"abcd"), // _batcherHash,
minimumGasLimit - 1, // _gasLimit,
address(1), // _unsafeBlockSigner,
Constants.DEFAULT_RESOURCE_CONFIG(), // _config,
0, // _startBlock
address(0), // _batchInbox
SystemConfig.Addresses({ // _addresses
l1CrossDomainMessenger: address(0),
......@@ -144,6 +177,46 @@ contract SystemConfig_Initialize_TestFail is SystemConfig_Init {
)
);
}
/// @dev Tests that initialization fails when the start block override is used
/// when the start block has already been set.
function test_initialize_startBlock_reverts() external {
// wipe out initialized slot so we can initialize again
vm.store(address(sysConf), bytes32(0), bytes32(0));
// the startBlock slot is 106, set it to something non zero
vm.store(address(sysConf), bytes32(uint256(106)), bytes32(uint256(block.number)));
// Initialize with a non zero start block, should see a revert
vm.prank(multisig);
// The call to initialize reverts due to: "SystemConfig: cannot override an already set start block"
// but the proxy revert message bubbles up.
vm.expectRevert("Proxy: delegatecall to new implementation contract failed");
Proxy(payable(address(sysConf))).upgradeToAndCall(
address(systemConfigImpl),
abi.encodeCall(
SystemConfig.initialize,
(
alice, // _owner,
overhead, // _overhead,
scalar, // _scalar,
batcherHash, // _batcherHash
gasLimit, // _gasLimit,
unsafeBlockSigner, // _unsafeBlockSigner,
Constants.DEFAULT_RESOURCE_CONFIG(), // _config,
1, // _startBlock
batchInbox, // _batchInbox
SystemConfig.Addresses({ // _addresses
l1CrossDomainMessenger: l1CrossDomainMessenger,
l1ERC721Bridge: l1ERC721Bridge,
l1StandardBridge: l1StandardBridge,
l2OutputOracle: l2OutputOracle,
optimismPortal: optimismPortal,
optimismMintableERC20Factory: optimismMintableERC20Factory
})
)
)
);
}
}
contract SystemConfig_Setters_TestFail is SystemConfig_Init {
......
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