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

Removed Kovan (EOL) / Common Mainnet Merge Default HF Fix #2206

Merged
merged 14 commits into from Aug 23, 2022

Conversation

holgerd77
Copy link
Member

This PR removes support for Kovan which is EOL for quite some time and support for the only supporting client (Open Ethereum) has also ended.

I would still be inclined to keep Ropsten and Rinkeby since - while both networks are now deprecated - deprecation announcement is still pretty recent and both networks are still up and running and will likely still be used for testing purposes.

This is WIP, first push is for seeing if Kovan removal works for all packages on CI.

Will add some Common default HF update to the PR.

@codecov
Copy link

codecov bot commented Aug 22, 2022

Codecov Report

Merging #2206 (e10594e) into master (001e147) will increase coverage by 0.01%.
The diff coverage is 85.71%.

Impacted file tree graph

Flag Coverage Δ
block 92.98% <100.00%> (-0.06%) ⬇️
blockchain 88.88% <50.00%> (+0.06%) ⬆️
client 87.00% <ø> (ø)
common 98.08% <100.00%> (-0.01%) ⬇️
devp2p 92.41% <100.00%> (+0.16%) ⬆️
ethash ∅ <ø> (∅)
evm 79.07% <ø> (ø)
rlp ?
statemanager 88.09% <ø> (ø)
trie 89.52% <ø> (ø)
tx 97.98% <ø> (ø)
util 92.33% <ø> (ø)
vm 85.24% <ø> (ø)

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

@holgerd77
Copy link
Member Author

Haha, mainnet wasn't actually defaulting yet to Merge. 😜 Have fixed this in the last commits, lot's of tests to fix, at least one client test is still failing, ready to be picked up if someone wants to.

@holgerd77
Copy link
Member Author

Side note: I also thought about removing the defaultHardfork entries completely in Common JSON files where the file default HF is the same as the class default HF (so: merge) (so the basic mechanism is that it is possible to override the class default HF with one from the chain file for cases where one would want to deviate from the "normal" default HF, e.g. for Rinkeby which didn't (and won't) transition to the Merge).

I then decided to keep the defaultHardfork setting also in the files where this is set to merge since e.g. the mainnet file is often taken as a basis for own HF files and then people can see that this setting exists and they can change.

@holgerd77
Copy link
Member Author

@gabrocheleau finally, the real "success" is entering the repository, @ryanio will be reliefed. 😂

Nice, I guess there should be no reason to not merge this here (actually I will do)?

Copy link
Member Author

@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.

LGTM.

Will admin-merge since technically reviewed by @gabrocheleau by continuing on the PR (and I in turn now reviewed the additional changes by Gabriel).

@holgerd77 holgerd77 merged commit 5dc3fd7 into master Aug 23, 2022
@holgerd77 holgerd77 deleted the common-cleanup branch August 23, 2022 08:48
@acolytec3 acolytec3 mentioned this pull request Aug 23, 2022
@holgerd77 holgerd77 changed the title Common, Block, Blockchain, Client: removed Kovan chain support (EOL) Common, Block, Blockchain, Client: removed Kovan (EOL) / Common Mainnet Merge Default HF Fix Aug 23, 2022
@holgerd77 holgerd77 changed the title Common, Block, Blockchain, Client: removed Kovan (EOL) / Common Mainnet Merge Default HF Fix Common, Other: removed Kovan (EOL) / Common Mainnet Merge Default HF Fix Aug 23, 2022
@holgerd77 holgerd77 changed the title Common, Other: removed Kovan (EOL) / Common Mainnet Merge Default HF Fix Removed Kovan (EOL) / Common Mainnet Merge Default HF Fix Aug 23, 2022
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.

None yet

2 participants