contracts-bedrock: more cleanup

op-chain-ops/genesis/layer_one.go

@@ -23,18 +23,37 @@ import (
 	"github.com/ethereum-optimism/optimism/op-chain-ops/state"
 )
 
-var proxies = []string{
+var (
+	proxies = []string{
 		"SystemConfigProxy",
 		"L2OutputOracleProxy",
 		"L1CrossDomainMessengerProxy",
 		"L1StandardBridgeProxy",
 		"OptimismPortalProxy",
 		"OptimismMintableERC20FactoryProxy",
-}
+	}
 
-var portalMeteringSlot = common.Hash{31: 0x01}
+	portalMeteringSlot = common.Hash{31: 0x01}
+	zeroHash           = common.Hash{}
+	uint128Max         = new(big.Int)
 
-var zeroHash common.Hash
+	defaultResourceConfig = bindings.SystemConfigResourceConfig{
+		MaxResourceLimit:            20_000_000,
+		ElasticityMultiplier:        10,
+		BaseFeeMaxChangeDenominator: 8,
+		MinimumBaseFee:              params.GWei,
+		SystemTxMaxGas:              1_000_000,
+	}
+)
+
+func init() {
+	var ok bool
+	uint128Max, ok = new(big.Int).SetString("ffffffffffffffffffffffffffffffff", 16)
+	if !ok {
+		panic("bad uint128Max")
+	}
+	defaultResourceConfig.MaximumBaseFee = uint128Max
+}
 
 func BuildL1DeveloperGenesis(config *DeployConfig) (*core.Genesis, error) {
 	if config.L2OutputOracleStartingTimestamp != -1 {
@@ -97,11 +116,6 @@ func BuildL1DeveloperGenesis(config *DeployConfig) (*core.Genesis, error) {
 		gasLimit = defaultL2GasLimit
 	}
 
-	uint128Max, ok := new(big.Int).SetString("ffffffffffffffffffffffffffffffff", 16)
-	if !ok {
-		return nil, errors.New("bad uint128Max")
-	}
-
 	data, err = sysCfgABI.Pack(
 		"initialize",
 		config.FinalSystemOwner,
@@ -110,14 +124,7 @@ func BuildL1DeveloperGenesis(config *DeployConfig) (*core.Genesis, error) {
 		config.BatchSenderAddress.Hash(),
 		gasLimit,
 		config.P2PSequencerAddress,
-		bindings.SystemConfigResourceConfig{
-			MaxResourceLimit:            20_000_000,
-			ElasticityMultiplier:        10,
-			BaseFeeMaxChangeDenominator: 8,
-			MinimumBaseFee:              params.GWei,
-			SystemTxMaxGas:              1_000_000,
-			MaximumBaseFee:              uint128Max,
-		},
+		defaultResourceConfig,
 	)
 	if err != nil {
 		return nil, fmt.Errorf("cannot abi encode initialize for SystemConfig: %w", err)
@@ -292,6 +299,7 @@ func deployL1Contracts(config *DeployConfig, backend *backends.SimulatedBackend)
 				config.BatchSenderAddress.Hash(), // left-padded 32 bytes value, version is zero anyway
 				gasLimit,
 				config.P2PSequencerAddress,
+				defaultResourceConfig,
 			},
 		},
 		{
@@ -361,6 +369,7 @@ func l1Deployer(backend *backends.SimulatedBackend, opts *bind.TransactOpts, dep
 			deployment.Args[3].(common.Hash),
 			deployment.Args[4].(uint64),
 			deployment.Args[5].(common.Address),
+			deployment.Args[6].(bindings.SystemConfigResourceConfig),
 		)
 	case "L2OutputOracle":
 		_, tx, _, err = bindings.DeployL2OutputOracle(

op-chain-ops/genesis/layer_one_test.go

@@ -100,6 +100,12 @@ func TestBuildL1DeveloperGenesis(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, "Wrapped Ether", name)
 
+	sysCfg, err := bindings.NewSystemConfig(predeploys.DevSystemConfigAddr, sim)
+	require.NoError(t, err)
+	cfg, err := sysCfg.ResourceConfig(&bind.CallOpts{})
+	require.NoError(t, err)
+	require.Equal(t, cfg, defaultResourceConfig)
+
 	// test that we can do deposits, etc.
 	priv, err := crypto.HexToECDSA("ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80")
 	require.NoError(t, err)

packages/contracts-bedrock/contracts/L1/SystemConfig.sol

@@ -81,12 +81,12 @@ contract SystemConfig is OwnableUpgradeable, Semver {
     uint64 public constant MINIMUM_GAS_LIMIT = 8_000_000;
 
     /**
-     * @notice Fixed L2 gas overhead.
+     * @notice Fixed L2 gas overhead. Used as part of the L2 fee calculation.
      */
     uint256 public overhead;
 
     /**
-     * @notice Dynamic L2 gas overhead.
+     * @notice Dynamic L2 gas overhead. Used as part of the L2 fee calculation.
      */
     uint256 public scalar;
 
@@ -97,12 +97,13 @@ contract SystemConfig is OwnableUpgradeable, Semver {
     bytes32 public batcherHash;
 
     /**
-     * @notice L2 gas limit.
+     * @notice L2 block gas limit.
      */
     uint64 public gasLimit;
 
     /**
-     * @notice
+     * @notice The configuration for the deposit fee market. Used by the OptimismPortal
+     *         to meter the cost of buying L2 gas on L1.
      */
     ResourceConfig internal _resourceConfig;
 
@@ -124,6 +125,7 @@ contract SystemConfig is OwnableUpgradeable, Semver {
      * @param _batcherHash       Initial batcher hash.
      * @param _gasLimit          Initial gas limit.
      * @param _unsafeBlockSigner Initial unsafe block signer address.
+     * @param _config            Initial resource config.
      */
     constructor(
         address _owner,
@@ -131,18 +133,10 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         uint256 _scalar,
         bytes32 _batcherHash,
         uint64 _gasLimit,
-        address _unsafeBlockSigner
+        address _unsafeBlockSigner,
+        ResourceConfig memory _config
     ) Semver(1, 1, 0) {
-        ResourceConfig memory config = ResourceConfig({
-            maxResourceLimit: 20_000_000,
-            elasticityMultiplier: 10,
-            baseFeeMaxChangeDenominator: 8,
-            minimumBaseFee: 1 gwei,
-            systemTxMaxGas: 1_000_000,
-            maximumBaseFee: type(uint128).max
-        });
-
-        initialize(_owner, _overhead, _scalar, _batcherHash, _gasLimit, _unsafeBlockSigner, config);
+        initialize(_owner, _overhead, _scalar, _batcherHash, _gasLimit, _unsafeBlockSigner, _config);
     }
 
     /**
@@ -259,7 +253,8 @@ contract SystemConfig is OwnableUpgradeable, Semver {
     }
 
     /**
-     * @notice A getter for the resource config.
+     * @notice A getter for the resource config. Ensures that the struct is
+     *         returned instead of a tuple.
      */
     function resourceConfig() external view returns (ResourceConfig memory) {
         return _resourceConfig;
@@ -284,6 +279,7 @@ contract SystemConfig is OwnableUpgradeable, Semver {
      *         - max resource limit plus system tx gas must be less than or
      *           equal to the L2 gas limit
      *         - elasticity multiplier must be greater than 0
+     *         - no precision loss when computing target resource limit
      *
      * @param _config The new resource config
      */
@@ -304,7 +300,7 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         require(
             ((_config.maxResourceLimit / _config.elasticityMultiplier) *
                 _config.elasticityMultiplier) == _config.maxResourceLimit,
-            "SystemConfig: precision loss with max and elasticity"
+            "SystemConfig: precision loss with target resource limit"
         );
 
         _resourceConfig = _config;

packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortal.sol

@@ -10,20 +10,30 @@ contract EchidnaFuzzOptimismPortal {
     bool internal failedToComplete;
 
     constructor() {
-        SystemConfig config = new SystemConfig({
+        SystemConfig.ResourceConfig memory config = SystemConfig.ResourceConfig({
+            maxResourceLimit: 20_000_000,
+            elasticityMultiplier: 10,
+            baseFeeMaxChangeDenominator: 8,
+            minimumBaseFee: 1 gwei,
+            systemTxMaxGas: 1_000_000,
+            maximumBaseFee: type(uint128).max
+        });
+
+        SystemConfig systemConfig = new SystemConfig({
             _owner: address(1),
             _overhead: 0,
             _scalar: 10000,
             _batcherHash: bytes32(0),
             _gasLimit: 30_000_000,
-            _unsafeBlockSigner: address(0)
+            _unsafeBlockSigner: address(0),
+            _config: config
         });
 
         portal = new OptimismPortal({
             _l2Oracle: L2OutputOracle(address(0)),
             _guardian: address(0),
             _paused: false,
-            _config: config
+            _config: systemConfig
         });
     }
 

packages/contracts-bedrock/contracts/test/CommonTest.t.sol

@@ -171,13 +171,23 @@ contract Portal_Initializer is L2OutputOracle_Initializer {
     function setUp() public virtual override {
         super.setUp();
 
+        SystemConfig.ResourceConfig memory config = SystemConfig.ResourceConfig({
+            maxResourceLimit: 20_000_000,
+            elasticityMultiplier: 10,
+            baseFeeMaxChangeDenominator: 8,
+            minimumBaseFee: 1 gwei,
+            systemTxMaxGas: 1_000_000,
+            maximumBaseFee: type(uint128).max
+        });
+
         systemConfig = new SystemConfig({
             _owner: address(1),
             _overhead: 0,
             _scalar: 10000,
             _batcherHash: bytes32(0),
             _gasLimit: 30_000_000,
-            _unsafeBlockSigner: address(0)
+            _unsafeBlockSigner: address(0),
+            _config: config
         });
 
         opImpl = new OptimismPortal({

packages/contracts-bedrock/contracts/test/SystemConfig.t.sol

@@ -9,13 +9,24 @@ contract SystemConfig_Init is CommonTest {
 
     function setUp() public virtual override {
         super.setUp();
+
+        SystemConfig.ResourceConfig memory config = SystemConfig.ResourceConfig({
+            maxResourceLimit: 20_000_000,
+            elasticityMultiplier: 10,
+            baseFeeMaxChangeDenominator: 8,
+            minimumBaseFee: 1 gwei,
+            systemTxMaxGas: 1_000_000,
+            maximumBaseFee: type(uint128).max
+        });
+
         sysConf = new SystemConfig({
             _owner: alice,
             _overhead: 2100,
             _scalar: 1000000,
             _batcherHash: bytes32(hex"abcd"),
             _gasLimit: 30_000_000,
-            _unsafeBlockSigner: address(1)
+            _unsafeBlockSigner: address(1),
+            _config: config
         });
     }
 }
@@ -24,6 +35,15 @@ contract SystemConfig_Initialize_TestFail is SystemConfig_Init {
     function test_initialize_lowGasLimit_reverts() external {
         uint64 minimumGasLimit = sysConf.minimumGasLimit();
 
+        SystemConfig.ResourceConfig memory cfg = SystemConfig.ResourceConfig({
+            maxResourceLimit: 20_000_000,
+            elasticityMultiplier: 10,
+            baseFeeMaxChangeDenominator: 8,
+            minimumBaseFee: 1 gwei,
+            systemTxMaxGas: 1_000_000,
+            maximumBaseFee: type(uint128).max
+        });
+
         vm.expectRevert("SystemConfig: gas limit too low");
         new SystemConfig({
             _owner: alice,
@@ -31,7 +51,8 @@ contract SystemConfig_Initialize_TestFail is SystemConfig_Init {
             _scalar: 0,
             _batcherHash: bytes32(hex""),
             _gasLimit: minimumGasLimit - 1,
-            _unsafeBlockSigner: address(1)
+            _unsafeBlockSigner: address(1),
+            _config: cfg
         });
     }
 }

packages/contracts-bedrock/contracts/test/invariants/SystemConfig.t.sol

@@ -7,13 +7,23 @@ contract SystemConfig_GasLimitLowerBound_Invariant is Test {
     SystemConfig public config;
 
     function setUp() public {
+        SystemConfig.ResourceConfig memory cfg = SystemConfig.ResourceConfig({
+            maxResourceLimit: 20_000_000,
+            elasticityMultiplier: 10,
+            baseFeeMaxChangeDenominator: 8,
+            minimumBaseFee: 1 gwei,
+            systemTxMaxGas: 1_000_000,
+            maximumBaseFee: type(uint128).max
+        });
+
         config = new SystemConfig({
             _owner: address(0xbeef),
             _overhead: 2100,
             _scalar: 1000000,
             _batcherHash: bytes32(hex"abcd"),
             _gasLimit: 30_000_000,
-            _unsafeBlockSigner: address(1)
+            _unsafeBlockSigner: address(1),
+            _config: cfg
         });
 
         // Set the target contract to the `config`

packages/contracts-bedrock/deploy/017-SystemConfigImpl.ts

 import { DeployFunction } from 'hardhat-deploy/dist/types'
 import '@eth-optimism/hardhat-deploy-config'
+import { ethers } from 'ethers'
 
 import { assertContractVariable, deploy } from '../src/deploy-utils'
 
@@ -18,6 +19,16 @@ const deployFn: DeployFunction = async (hre) => {
       batcherHash,
       hre.deployConfig.l2GenesisBlockGasLimit,
       hre.deployConfig.p2pSequencerAddress,
+      {
+        maxResourceLimit: 20_000_000,
+        elasticityMultiplier: 10,
+        baseFeeMaxChangeDenominator: 8,
+        systemTxMaxGas: 1_000_000,
+        minimumBaseFee: ethers.utils.parseUnits('1', 'gwei'),
+        maximumBaseFee: ethers.BigNumber.from(
+          '0xffffffffffffffffffffffffffffffff'
+        ),
+      },
     ],
     postDeployAction: async (contract) => {
       await assertContractVariable(