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.

Sign up for GitHub

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

Jump to bottom

rm Clique consensus method support and Goerli network #2219

Merged
merged 2 commits into from
May 25, 2024
Merged

rm Clique consensus method support and Goerli network #2219

merged 2 commits into from
May 25, 2024

Conversation

tersec
Copy link
Contributor

@tersec tersec commented May 25, 2024

Geth v1.14.0 stops automatically constructing the pending block (ethereum/go-ethereum#28623). Performance wise there is a significant cost to creating a potential pending block, yet most node operators do not care about it. Not even validators need the pending block. This work also drops support for mining/signing a block "off-demand" (i.e. not via the engine API, rather by Geth itself), meaning Clique signing is also removed going forward.

With Clique mining removed, Geth v1.14.0 cannot be a Clique signer any more. This however goes hand-in-hand with the removal or pre-merge chain selection and synchronisation support described earlier.

danger

Clique is not suitable for production environments. Use only in development environments.

Regarding Goerli, https://blog.ethereum.org/2023/11/30/goerli-lts-update and status-im/nimbus-eth2#6057

arnetheduck
arnetheduck reviewed May 25, 2024
View reviewed changes
hive_integration/nodocker/engine/engine_env.nim
@@ -93,7 +91,7 @@ proc newEngineEnv*(conf: var NimbusConf, chainFile: string, enableAuth: bool): E
chain = newChain(com)

com.initializeEmptyDb()
let txPool = TxPoolRef.new(com, conf.engineSigner)
let txPool = TxPoolRef.new(com, ZERO_ADDRESS)
Copy link
Member

Choose a reason for hiding this comment

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

can we get rid of this last arg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, as a follow-up PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In particular, it appears, after POW has been removed more thoroughly, because this miner variable is also used for that purpose, and some tests still therefore refer to it. This requires more care than POA, because currently it appears to start in POW in several instances, then transition to POS almost, but not quite, immediately on startup.

It might be cleaner to address POW first, but ultimately, either ordering works, as long as there's not an expectation that POW mode produces blocks with non-zero miners in the meantime, which should be fine insofar as it doesn't seem important it produces block at all at this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#2235

@tersec tersec merged commit e895c0b into master May 25, 2024
9 checks passed
@tersec tersec deleted the rm-clique-goerli branch May 25, 2024 14:12
@tersec tersec mentioned this pull request May 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arnetheduck arnetheduck arnetheduck approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tersec @arnetheduck