-
Notifications
You must be signed in to change notification settings - Fork 276
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
feat(trace): allow override fork #836
base: develop
Are you sure you want to change the base?
Conversation
a4768f3
to
4a4eef7
Compare
chainConfig.CurieBlock = curie | ||
if block.Number().Cmp(curie) > 0 { | ||
// set non zero values for these slots | ||
misc.ApplyCurieHardFork(statedb) |
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.
if Curie fork is already enabled, this will overwrite L1GasPriceOracle
's storage.
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.
if block.Number().Cmp(api.backend.ChainConfig().CurieBlock) < 0 && block.Number().Cmp(curie) > 0 {
. sounds more reasonable?
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.
Yeah, that is better. But I am afraid not complete. Maybe something like below?
!api.backend.ChainConfig().IsCurie(block.Number()) && block.Number().Cmp(curie) >= 0
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.
in fact if block.Number().Cmp(curie) == 0, i think we don't need to inject system upgrades twice.
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.
in fact if block.Number().Cmp(curie) == 0, i think we don't need to inject system upgrades twice.
as long as !api.backend.ChainConfig().IsCurie(block.Number())
is true, I don't think we would be applying the upgrade twice. If we apply only when block.Number().Cmp(curie) > 0
, ApplyCurieHardFork will be applied one block late if I am not mistkaen.
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.
"ApplyCurieHardFork will be applied one block late": you mean the bytecode force upgrade does not happen exactly at the fork height? i don't think so..
37dfa4c
to
ecb1720
Compare
1. Purpose or design rationale of this PR
so we can get traces as if blocks are executed post curie hardfork
2. PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
3. Deployment tag versioning
Has the version in
params/version.go
been updated?4. Breaking change label
Does this PR have the
breaking-change
label?