chore(chainspec): Use SealedHeader in ChainSpec
#14441
Conversation
emhane
requested review from
onbjerg,
fgimenez,
rkrasiuk and
gakonst
as code owners
emhane
force-pushed
the
emhane/chainspec-header
branch
from
27844ea to
615ca4b
Compare
There was a problem hiding this comment.
will make issue to move to op alloy once merged so I can permalink this code
crates/chainspec/src/spec.rs
Outdated
| if self.is_dev() { | ||
| SealedHeader::new(header, DEV_GENESIS_HASH) | ||
| } else { |
There was a problem hiding this comment.
we can't do this, this shouldn't be necessary
There was a problem hiding this comment.
reverted this change, and now I get the fork id error again, as expected
thread 'spec::tests::dev_fork_ids' panicked at crates/chainspec/src/spec.rs:1008:9:
assertion `left == right` failed: Expected fork ID ForkId { hash: ForkHash("45b83612"), next: 0 }, computed fork ID ForkId { hash: ForkHash("d5bca3a4"), next: 0 } at block 0
left: ForkId { hash: ForkHash("45b83612"), next: 0 }
right: ForkId { hash: ForkHash("d5bca3a4"), next: 0 }
|
re-request review re: #14441 (comment) |
| pub genesis_header: OnceLock<Header>, | ||
| /// This is either stored at construction time if it is known, or computed once on the first | ||
| /// access. | ||
| pub genesis_header: OnceLock<SealedHeader>, |
There was a problem hiding this comment.
we still have OnceLock here requiring to define the lazy initializer per chainspec
can we just always initialize the header when creating a chainspec?
There was a problem hiding this comment.
yeah sure, but would like to do that as sep pr
| if self.is_optimism_mainnet() { | ||
| // for OP mainnet we have to do this because the genesis header can't be | ||
| // properly computed from the genesis.json file due to OVM history | ||
| return SealedHeader::new(header, OP_MAINNET_GENESIS_HASH) |
There was a problem hiding this comment.
this is problematic because this can also be a modified genesis file
There was a problem hiding this comment.
p sure that would make it a devnet
There was a problem hiding this comment.
what if you run a custom genesis with mainnet chainid?
we can't do that here this must be set on the static MAINNET
There was a problem hiding this comment.
ok. if that's not running on LAN then it's useful for dos:ing op mainnet 😅
sealed header is already init here, see calls to new methods if you're referring to the |
|
I'd prefer if we could only change it once and not merge something that is supposed to be temporary |
Closes #14355
Fixes bug: devnet genesis hash must be hard coded like op mainnet to match
alloy_consensus::constants::DEV_GENESIS_HASH