rpc/chainHead: Fix pruned blocks events from forks

[lexnv]

This PR fixes the chainHead::follow edge cases:

1. The Finalized event reported pruned blocks from soon-to-be-pruned forks without issuing a NewBlock first
2. Finalized event reported as pruned just the stale heads, not the entire fork

Issue 1

The chainHead::follow method returns the Finalized event containing both finalized hashes and pruned block hashes. The substrate database will perform the pruning of height N at the finalization N + 1.

Considering the following tree structure:

A - A1 - A2 - A3
  - A1 - B2

Initial Condition:

• User subscribes after A2 is finalized, but before A3 is finalized
• Spec V2 dictates that in-memory blocks must first be reported.
  • first event: Initialized event containing the finalized hash (A2)
  • multiple events: NewBlock events for children of finalized (A3)

The NewBlock event contains the hash of the block + the parent hash. Taken from the spec:

parentBlockHash is guaranteed to be equal either to the current finalized block hash, or to a block reported in an earlier newBlock event.

Issues:

• When A2 is finalized, we know that the fork [A1 - B2] is stale; however pruning happens for height N - 1
• When A3 is finalized, we prune B2 (height 3 prunes stale forks on height 2)
• B2 is reported as pruned; however it was never reported by the initial events

Solution:

• When generating the initial events for in-memory blocks keep track of stale forks
• Ignore stale forks when reporting the finalized event

Issue 2

chainHead::follow method reported just the stale heads when declaring the pruned hashes.

prunedBlockHashes contains, in no particular order, a list of blocks that are not descendants of the latest finalized block. These blocks will never be finalized and can be discarded.

To fix this issue, the tree route is determined from the stale heads to the canonical chain and all blocks are reported.

Additional

Because the complexity of generating the follow events grew, the logic is moved to the chain_head_follow.rs file.
This also helps with code readability and maintenance.

Under intensive testing, it was possible for the Finalized event to be received before the NewBlock that led to flaky scenarios. To close the gap, the Finalized branch can now also generate NewBlock events.

Testing Done

• follow_forks_pruned_block and follow_report_multiple_pruned_block cover the issues described above
• created a loop of cargo test together with stress-ng --cpu 8 --cpu-load 100 --io 4 to filter out any flakiness

Review Notes

The entry point of the follow logic is moved to a dedicated file. While at it, have also simplified the notification stream logic:

substrate/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs

Lines 565 to 566 in 33f5f10

	/// Generate the block events for the `chainHead_follow` method.
	pub async fn generate_events(

The blocks to ignore as pruned are collected initially here:

substrate/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs

Lines 179 to 180 in 33f5f10

	/// Get the in-memory blocks of the client, starting from the provided finalized hash.
	fn get_init_blocks_with_forks(

Both issues are handled by this method:

substrate/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs

Lines 392 to 395 in 33f5f10

	/// Get all pruned block hashes from the provided stale heads.
	///
	/// The result does not include hashes from `to_ignore`.
	fn get_pruned_hashes(

Additionally, the finalized stream could also generate the NewBlock events here:

substrate/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs

Lines 447 to 448 in 33f5f10

	// If the finalized hashes were not reported yet, generate the `NewBlock` events.
	let mut events = self.generate_finalized_events(&finalized_block_hashes)?;

The other logic is moved from the chain_head to the chain_head_follow with minimal additions (mostly better error reporting).

Thanks @skunert and @davxy for the initial feedback and for raising the flakiness of the tests after the fix; plus the stale heads!

// CC: @paritytech/tools-team

[skunert]

Logic looks good to me, just some nits.

[davxy]

Looks like some tests are not passing on this branch but are passing on master (at least "in my machine™️")
(also tried with single thread)
The failures are not deterministic... sometime some of the following are passing:

    chain_head::tests::get_body
    chain_head::tests::get_header
    chain_head::tests::get_storage
    chain_head::tests::get_storage_wrong_key
    chain_head::tests::call_runtime

[davxy]

Not 100% familiar with this code part. But I've done my best 😁

Mostly looks good to me. Green light after the review points are resolved (or justified)

[melekes]

lgtm minus comments above

[davxy]

Looks good