Skip to content
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

common: Rename shardingForkDev hf to Cancun #2659

Merged
merged 5 commits into from
Apr 24, 2023
Merged

Conversation

g11tech
Copy link
Contributor

@g11tech g11tech commented Apr 24, 2023

With Cancun hf shaping up nicely, this PR updates the references of the hf to Cancun (corresponding CL hf Deneb)

@g11tech g11tech added PR state: merge ready package: common type: test all hardforks This special label enables VM state and blockchain tests for all hardforks on the respective PR. target: develop-v7 labels Apr 24, 2023
@codecov
Copy link

codecov bot commented Apr 24, 2023

Codecov Report

❗ No coverage uploaded for pull request base (develop-v7@e39505e). Click here to learn what that means.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 90.64% <0.00%> (?)
blockchain 90.43% <0.00%> (?)
client 87.06% <0.00%> (?)
common 95.76% <0.00%> (?)
devp2p 91.83% <0.00%> (?)
ethash ∅ <0.00%> (?)
evm 79.35% <0.00%> (?)
rlp ∅ <0.00%> (?)
statemanager 88.19% <0.00%> (?)
trie 90.39% <0.00%> (?)
tx 95.45% <0.00%> (?)
util 81.50% <0.00%> (?)
vm 84.39% <0.00%> (?)

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

{
"name": "cancun",
"block": null,
"forkHash": null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if me might always want to take the sematics here:

If there is an entry on something with null, there is (will be) some value, we just don't know (yet) what it is, and completely leaving when there is just not a value.

This would be an argument to also add "timestamp": null for these HF entries (while having (obviously) left the timestamp entry for a HF like London).

I have absolutely no idea/overview, if this has practical implications or not, but it might help that we can do some things slightly cleaner already or in the future.

Totally no strong opinion here though, might help a bit, might be a total "non-thing", feel free to add or leave (if you e.g. do not want to re-run CI).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timestamp is currently undefined or string i think, can change/add null in followup PR 👍

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can already approve here independently from the one thing I mentioned.

If you want to still add I can directly re-approve, otherwise just merge in.

@g11tech g11tech merged commit 7a8af7e into develop-v7 Apr 24, 2023
@holgerd77 holgerd77 deleted the g11tec/cancun-hf branch June 6, 2023 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: common PR state: merge ready type: test all hardforks This special label enables VM state and blockchain tests for all hardforks on the respective PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants