-
Notifications
You must be signed in to change notification settings - Fork 172
chain config: move 'optimism' to 'genesis.fee_params' #975
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
1640f9a
to
c9fdbb2
Compare
8a4dbcf
to
79b30ac
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #975 +/- ##
==========================================
+ Coverage 41.92% 41.93% +0.01%
==========================================
Files 40 40
Lines 2674 2673 -1
==========================================
Hits 1121 1121
+ Misses 1450 1449 -1
Partials 103 103 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
BaseFeeScalar *uint64 `json:"baseFeeScalar,omitempty" toml:"baseFeeScalar,omitempty"` | ||
BlobBaseFeeScalar *uint64 `json:"blobBaseFeeScalar,omitempty" toml:"blobBaseFeeScalar,omitempty"` | ||
BatcherAddr ChecksummedAddress `json:"batcherAddr" toml:"batcherAddress"` | ||
BatchInboxAddr *ChecksummedAddress `json:"batchInboxAddr" toml:"batch_inbox_addr"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an interesting one. The batch inbox address isn't actually in the system config that gets derived, just the contract.
The genesis system config vars, in my head, are things that can change (that the protocol supports changing in normal operation) - imo, it's fine to lift into genesis
, but not genesis.system_config
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh ok got it. Will lift it one level to genesis
then. And will update the op-geth and monorepo prs accordingly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the inconsistency is that we would put this address that can be read from the system config (and that cannot be changed) into genesis.SystemConfig
, but would still leave others in the addresses
section. So @clabby @bitwiseguy wdyt about putting the batch inbox address into the addresses
section instead?
Overview
Makes the following changes to the chain config .toml struct:
batch_inbox_addr
field moved from top level to[genesis.system_config]
[optimism]
struct renamed/moved to [genesis.fee_params]Related Issues
Closes #969