common: Implement hardfork by time

[g11tech]

The new post merge hardforks which need to be coordinated with CL are based on timestamp.

this PR attempts the same.

• This PR also will trigger the forkid change based on timestamps as presented in the PR:
  • Add EIP-6122: Forkid checks based on timestamps ethereum/EIPs#6122 (review)

[acolytec3]

I'll have experiment more deeply with it but it looks okay so far. Let a couple of questions.

[codecov]

Codecov Report

Merging #2437 (04bff59) into master (7e77b0f) will decrease coverage by 0.01%.
The diff coverage is 85.22%.

Additional details and impacted files

Flag	Coverage Δ
block	89.51% <100.00%> (ø)
blockchain	90.04% <100.00%> (+0.01%)	⬆️
client	87.56% <84.00%> (+<0.01%)	⬆️
common	95.78% <85.50%> (-2.30%)	⬇️
devp2p	91.70% <0.00%> (+0.21%)	⬆️
ethash	∅ <ø> (∅)
evm	79.73% <ø> (ø)
rlp	∅ <ø> (∅)
statemanager	89.61% <ø> (ø)
trie	90.36% <ø> (ø)
tx	97.80% <ø> (ø)
util	84.59% <ø> (ø)
vm	85.68% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

[g11tech]

the withdrawals devnet based on shanghai time successfully synced!

(some cleanup still wip)

[acolytec3]

Looking good overall. I think we need to carefully consider some of these changes though and make sure we're handling edge cases. I know I haven't thought of all of them but will try and spin up the withdrawals devnet and validate directly to look for more edge cases.

[acolytec3]

Looking good overall. I think we need to carefully consider some of these changes though and make sure we're handling edge cases. I know I haven't thought of all of them but will try and spin up the withdrawals devnet and validate directly to look for more edge cases.

[acolytec3]

Just a couple of small nits at this point. Looks great!

[holgerd77]

Yeah, first: thanks a lot Gajinder, great work!!! 🎉 Super-important building block for future Ethereum updates.

I nevertheless already pray for the day when we can clean this code up again. 😋 This yet-another-mechanism addition gets really ugly, regarding the existing naming, the overall bloat of the code, chances that we have some edge cases in there are relatively high.

I would nevertheless have a tendency to for now take this in as we have, I guess that's very much a candidate for a first-round refactor on the next breaking release round, e.g. in a first round changing the names of the Common functions and take in a dictionary as Gajinder already did at some point.

Ok, so I would have one specific comment to address on our new internal forks, otherwise I am fine with merging! 🙂

[acolytec3]

LGTM - will merge once tests pass given @holgerd77 's written approval of the changes as well.

[holgerd77]

@acolytec3 Thanks a lot for updating, not sure - might not be pressing - just to make sure: weren't there more references of these hardforks - e.g. Eof - in the testnet scripts from @g11tech e.g.? 🤔

[acolytec3]

@acolytec3 Thanks a lot for updating, not sure - might not be pressing - just to make sure: weren't there more references of these hardforks - e.g. Eof - in the testnet scripts from @g11tech e.g.? thinking

Hmm, I don't see anything in any typescript files. I just renamed the enum bits so if we need to consider something else that I'm not thinking of, we can certainly pick it up in a future PR. I don't see anything obvious doing a quick search of the whole repo though.

[holgerd77]

@acolytec3 Thanks a lot for updating, not sure - might not be pressing - just to make sure: weren't there more references of these hardforks - e.g. Eof - in the testnet scripts from @g11tech e.g.? thinking

Hmm, I don't see anything in any typescript files. I just renamed the enum bits so if we need to consider something else that I'm not thinking of, we can certainly pick it up in a future PR. I don't see anything obvious doing a quick search of the whole repo though.

https://github.com/ethereumjs/ethereumjs-monorepo/blob/master/packages/client/test/sim/eof.spec.ts

[holgerd77]

What is this sharding HF for, is this for your 4844 work? Do we need this (in master) at all?

[acolytec3]

https://github.com/ethereumjs/ethereumjs-monorepo/blob/master/packages/client/test/sim/eof.spec.ts

I don't see anything in here that references the hardfork enum so not sure what you're pointing to.

What is this sharding HF for, is this for your 4844 work? Do we need this (in master) at all?

Yeah, that can be removed. I wasn't thinking too closely about it and I think we've been so focused on the 4844 work it just got conflated with the dummy EOF hardfork so left in.

[holgerd77]

https://github.com/ethereumjs/ethereumjs-monorepo/blob/master/packages/client/test/sim/eof.spec.ts

I don't see anything in here that references the hardfork enum so not sure what you're pointing to.

What is this sharding HF for, is this for your 4844 work? Do we need this (in master) at all?

Yeah, that can be removed. I wasn't thinking too closely about it and I think we've been so focused on the 4844 work it just got conflated with the dummy EOF hardfork so left in.

Ah, you are right, this is using the string directly, didn't look to closely.

Ok, I might delete the HF along release if you don't contradict.

Also: still planning to do the releases, maybe end of the week or beginning of next week.

[acolytec3]

Ok, I might delete the HF along release if you don't contradict.

Sounds good to me. Definitely support keeping master as clean as possible.

[g11tech]

yes eof.spec.ts doesn't directly refer the hardfork, but its refered in the geth genesis used for it in the config, eofBlock, since you updated the map for parsing geth genesis to the new Hardfork.eof_INTERNAL, so that should be correctly used now