Move `[optimism]` section of chain configs into `[genesis]` section

[sebastianst]

The EIP-1559 config that sits inside the [optimism] section of chain configs belongs to the immutable genesis config, so we should move it there and avoid motivating chains to update it when they change it on-chain via Holocene SystemConfig parameter upgrades.

This has happened in https://github.com/ethereum-optimism/superchain-registry/pull/922 and we revert this now, current releases cannot be used for worldchain sync from genesis (both sepolia and mainnet).

[tynes]

We also need to move some fields out of the roles section because it will result in the same issue. The ~~unsafe block signer and~~ batch submitter are part of the genesis config.

Why do we have 2 sources of truth for the roles? Shouldn't we just say that onchain is the single source of truth for the roles at any one given moment?

[bitwiseguy]

@tynes we are close to fixing the 2 sources of truth for roles and most of the contract addresses. See this pr and the "follow-up" work section in its description.

[tessr]

Why do we have 2 sources of truth for the roles? Shouldn't we just say that onchain is the single source of truth for the roles at any one given moment?

Can we remove the immutable EIP-1559 config from the SR, or would that break any kind of validation flow? Is there an easy way to query for that info onchain? @bitwiseguy @mslipper

[bitwiseguy]

My understanding is that we still need the [optimism] section in the chain config .toml files, but we want to move it from its own section and instead embed it inside the [genesis] section of those configs. The reasoning being that everyone should understand the [genesis] section (and anything nested underneath) should not be changed. The current eip-1559 params can be read from the SystemConfig contract. When we move the [optimism] section, we will have to coordinate an update to op-node, op-geth, and op-reth(?) since they read those values and will now have to look in a different spot.

@tessr - perhaps later we want to display the current eip-1559 params (or maybe a transformed version that displays gasTarget, gasLimit) as part of the chainList files that get updated regularly?

[tessr]

perhaps later we want to display the current eip-1559 params (or maybe a transformed version that displays gasTarget, gasLimit) as part of the chainList files that get updated regularly?

yes, this is a good point -- I can easily imagine us deciding to add this later. Let's not wipe anything that would make it harder to display that kind of information.

[bitwiseguy]

Since this is a breaking change anyways, could we take this opportunity to rename [optimism] to [gas_params]? [optimism] is not a helpful name IMO

[sebastianst]

Since this is a breaking change anyways, could we take this opportunity to rename [optimism] to [gas_params]? [optimism] is not a helpful name IMO

I think the reason it's called optimism is because these fields are in the OptimsimConfig inside the ChainConfig in op-geth. They currently only hold the eip1559 params, but could contain more in the future. But still, the reality is that they today only contain the eip1559 params so I support a rename. And while we're at it, I suggest to rename even more, like so

[genesis]
  [genesis.eip1559_params]
    elasticity = ...
    denominator = ...
    denominator_canyon = ...

if we name the section eip1559_params we can remove the prefix eip1559 from the field names.

[tynes]

My understanding is that we still need the [optimism] section in the chain config .toml files, but we want to move it from its own section and instead embed it inside the [genesis] section of those configs. The reasoning being that everyone should understand the [genesis] section (and anything nested underneath) should not be changed. The current eip-1559 params can be read from the SystemConfig contract. When we move the [optimism] section, we will have to coordinate an update to op-node, op-geth, and op-reth(?) since they read those values and will now have to look in a different spot.

@tessr - perhaps later we want to display the current eip-1559 params (or maybe a transformed version that displays gasTarget, gasLimit) as part of the chainList files that get updated regularly?

We should identify which parts of the config refer to genesis parameters that are not currently in the genesis section. The following values are part of the genesis:

	// L1 address that batches are sent to.
	BatchInboxAddress common.Address `json:"batch_inbox_address"`
	// L1 Deposit Contract Address
	DepositContractAddress common.Address `json:"deposit_contract_address"`
	// L1 System Config Address
	L1SystemConfigAddress common.Address `json:"l1_system_config_address"`

	// ChainOpConfig is the OptimismConfig of the execution layer ChainConfig.
	// It is used during safe chain consolidation to translate zero SystemConfig EIP1559
	// parameters to the protocol values, like the execution layer does.
	// If missing, it is loaded by the op-node from the embedded superchain config at startup.
	ChainOpConfig *params.OptimismConfig `json:"chain_op_config,omitempty"`

source

where the optimism config is defined as:

	chainOpConfig := &params.OptimismConfig{
		EIP1559Elasticity:        d.EIP1559Elasticity,
		EIP1559Denominator:       d.EIP1559Denominator,
		EIP1559DenominatorCanyon: &d.EIP1559DenominatorCanyon,
	}

source

Since .batch_inbox_address is not mutable, it is fine to be outside of the genesis section, but if it ever becomes mutable, then it needs to be part of the genesis section. All three of .optimism.eip1559_elasticity, .optimism.eip1559_denominator, and .optimism.eip1559_denominator_canyon are part of genesis. .roles.BatchSubmitter is also part of genesis.

Rule of thumb of whether or not something should be part of genesis: it breaks historical sync if its changed.

More modern chains can use the SystemConfig.startBlock and fetch these genesis values from onchain events emitted in the start block. Legacy chains do not have that event in place. A big simplification of this would be to think of these hardcoded values as fallbacks if the onchain event doesn't exist in the SystemConfig.startBlock

[tynes]

We should be able to make this change completely contained within this library, no need to cascade the changes into other codebases. So please do not introduce a breaking API change that needs to be rolled out across all other codebases. It should be straightforward to keep the same API and just shuffle around the internal parsing in this library

[sebastianst]

We should be able to make this change completely contained within this library, no need to cascade the changes into other codebases. So please do not introduce a breaking API change that needs to be rolled out across all other codebases. It should be straightforward to keep the same API and just shuffle around the internal parsing in this library

@tynes I think there's no parsing happening inside this repo any more. op-geth now directly defines parsing code of the toml files in this repo.

So when the toml file format is changed, it automatically creates a breaking change.

[tynes]

We should be able to make this change completely contained within this library, no need to cascade the changes into other codebases. So please do not introduce a breaking API change that needs to be rolled out across all other codebases. It should be straightforward to keep the same API and just shuffle around the internal parsing in this library

@tynes I think there's no parsing happening inside this repo any more. op-geth now directly defines parsing code of the toml files in this repo.

Got it, in that case understood and then unfortunately we will need to coordinate releases with that then, thank you for the correction