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: Unify and Refactor set-/getHardforkByBlockNumber #2798

Merged
merged 4 commits into from
Jun 19, 2023

Conversation

holgerd77
Copy link
Member

@holgerd77 holgerd77 commented Jun 19, 2023

Partly solves #2774

This PR does the following:

  • Common.getHardforkByBlockNumber() -> Common.getHardforkBy() (+ strict bigint types)
  • Common.setHardforkByBlockNumber() -> Common.setHardforkBy() (+ strict bigint types) 
  • Blockchain.checkAndTransitionHardForkByNumber() BigIntLike -> strict bigint types 

@codecov
Copy link

codecov bot commented Jun 19, 2023

Codecov Report

Merging #2798 (d214934) into master (2fd4087) will decrease coverage by 0.01%.
The diff coverage is 81.91%.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 89.60% <100.00%> (+0.02%) ⬆️
blockchain 92.85% <100.00%> (+0.01%) ⬆️
client 87.09% <77.35%> (+0.01%) ⬆️
common 97.09% <100.00%> (-0.02%) ⬇️
devp2p 86.58% <ø> (ø)
ethash ∅ <ø> (∅)
evm 66.60% <ø> (ø)
rlp ∅ <ø> (∅)
statemanager 86.35% <ø> (ø)
trie 89.70% <ø> (-0.29%) ⬇️
tx 95.87% <ø> (ø)
util 85.31% <ø> (ø)
vm 77.55% <0.00%> (ø)

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

Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

Not approving yet as I'm reviewing this on mobile and CI is still running so want to take a second look later this morning but this looks great so far! Much more commonsense approach so far.

@acolytec3
Copy link
Contributor

A great first start on reducing confusion on how we do this!

@acolytec3 acolytec3 merged commit 3bf4692 into master Jun 19, 2023
@holgerd77 holgerd77 deleted the unify-get-hardfork-by-options branch June 19, 2023 19:46
@holgerd77 holgerd77 changed the title Common, Block, VM: Unify and Refactor getHardforkBy Options Common: Unify and Refactor set-/getHardforkByBlockNumber Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants