Skip to content

consensus, core, internal, miner: remove FinalizeAndAssemble - #34726

Merged
gballet merged 4 commits into
ethereum:masterfrom
rjl493456442:block-creation
Apr 21, 2026
Merged

consensus, core, internal, miner: remove FinalizeAndAssemble#34726
gballet merged 4 commits into
ethereum:masterfrom
rjl493456442:block-creation

Conversation

@rjl493456442

Copy link
Copy Markdown
Member

This PR removes FinalizeAndAssemble from the consensus engine interface
and relocates block assembly logic outside of the consensus engine.

Block assembly is consensus-agnostic. Most validations can be performed
by the caller. For example:

  • Withdrawals must be nil prior to Shanghai
  • After Shanghai upgrade, withdrawals must be non-nil, even if empty.

The only notable consensus-specific validation is related to uncles. In clique,
the concept of uncles does not exist, and any block containing uncles should
be considered invalid.

Within the block production package, the policy is to produce blocks according
to the latest chain specification. As a result, Clique-specific block production
is no longer supported. This tradeoff is considered acceptable.

@jwasinger jwasinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

mostly LGTM. Should we preserve the telemetry calls for the miner path?

@rjl493456442

Copy link
Copy Markdown
Member Author

mostly LGTM. Should we preserve the telemetry calls for the miner path?

The engine.Finalize will still be called. I guess open-telemetry hooks will be invoked there.

@MariusVanDerWijden MariusVanDerWijden left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sgtm

@MariusVanDerWijden

Copy link
Copy Markdown
Member

ah but now we don't measure the state root calculation anymore, right?

@rjl493456442

Copy link
Copy Markdown
Member Author

ah but now we don't measure the state root calculation anymore, right?

Good catch, i guess we need to following PR to fix the opentelemetry tracing

@gballet gballet added this to the 1.17.3 milestone Apr 21, 2026
@gballet
gballet merged commit d422ab3 into ethereum:master Apr 21, 2026
11 of 13 checks passed
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.

4 participants