Skip to content

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Oct 1, 2025

The finality notification was already carrying the information about the stale heads. However, most users of the stale heads were expanding these stale heads to all the stale blocks. So, we were iterating the same forks multiple times in the node for each finality notification. Also in a possible future where we start actually pruning headers as well, expanding these forks would fail.

So, this pull request is changing the finality notification to directly carry the stale blocks (which were calculated any way already).

The finality notification was already carrying the information about the stale heads.
However, most users of the stale heads were expanding these stale heads to all the stale blocks.
So, we were iterating the same forks multiple times in the node for each finality notification.
Also in a possible future where we start actually pruning headers as well, expanding these forks
would fail.

So, this pull request is changing the finality notification to directly carry the
stale blocks (which were calculated any way already).
@bkchr bkchr added the T0-node This PR/Issue is related to the topic “node”. label Oct 1, 2025
@bkchr
Copy link
Member Author

bkchr commented Oct 1, 2025

/cmd prdoc --audience node_dev --bump major

@skunert skunert self-requested a review October 1, 2025 11:56
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/18161222855
Failed job name: test-linux-stable-int

bkchr added 2 commits October 1, 2025 14:12
…ication-stale-blocks' into bkchr-finality-notfication-stale-blocks
Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

nice, LGTM!

finalized: vec![header.hash()],
stale_blocks: stale_blocks
.into_iter()
.map(|h| StaleBlock { hash: h, is_head: false })
Copy link
Contributor

Choose a reason for hiding this comment

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

is_head: false set for all stale_blocks here. not familiar with this trigger_finality_stream function, can it be called with any "head" among stale_blocks?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is test code and the implementation right now doesn't care if this is a head or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants