Add stale branches heads to finality notifications

[davxy]

Closes #10605

Includes stale branches heads to finality notifications sent by the client.

Warning

Previous implementation was sending a notification for each block between the previous (explicitly) finalized block and
the new finalized one (with an hardcoded limit of 256). Thus finalization messages were sent also for implicitly finalized blocks.

Now finality notification is sent only for the new finalized head and it contains the hash of the new finalized head hash, new finalized head header, a list of all the implicitly finalized blocks and a list of stale branches heads (i.e. heads that are not part of the canonical chain anymore).

[davxy]

@bkchr @andresilva

TL;DR

A fairly meticulous inspection of finality_notification_stream consumers;

In particular assumptions about sequential block finalization messages were checked and eventually fixed.

Looks like receiving the last finalized block message was sufficient for almost all consumers with the exception of txpool, where a fix is already contained in this PR.

A review from more experienced people for the single components is still required.

Follows a recap for Substrate, Cumulus and Polkadot with code references to simplify the inspection.

Substrate

Transaction Pool

• Stream read by notification_future here
• Validated transaction pool assumes to receive a notification message for each finalized block.
• Message handler has been adjusted to send the full finalized list to pool.validated_pool().on_block_finalized()

See here

Beefy

• Stream read by BeefyWorker here

• Message is then processed here

• Looks happy with final head notification.

Networking

• Stream is read within build_network_future here
• A wrapper to find the last finalized block was present here. This is not required anymore.
• build_network_future forwards finality notification to network.on_block_finalized(hash, number) here
• message arrives to ChainSync::on_block_finalized here.
• message is forwarded to ExtraRequests::on_block_finalized in here

RPC

• Stream read messages are forwarded to subscribers of subscribe_finalized_heads API here

• WARNING: we are now sending notifications only for finalized heads but external applications may rely on previous behaviour?

Cumulus

PovRecovery

• Stream is read within PovRecovery::run here

• Calls handle_block_finalized(finalized_number) that is used to remove from self.pending_candidates blocks with number <= finalized_number here

• Seems happy with final head notification

RelaychainClient

• Stream is read within RelaychainClient for RCInterface :: finalized_heads implementation here

• The stream result is then used in follow_finalized_head. here

  • for each new finalized head the parachain.finalize_block(finalized_hash) is called.
  • this is already correctly handled by Substrate Client implementation for Finalizer trait.

• Seems happy with final head notification

Polkadot

Overseer

node/overseer/src/lib.rs

• forward_events() forwards stream messages to the overseer wrapper using Handle::block_finalized here
• Handle::block_finalized sends messages to Overseer task.
• The event is catched within the Overseer::run and managed using Overseer::block_finalized here
  • This prunes leaves below the finalized head here

[bkchr]

@davxy ty for the detailed report. Looks good as far as I can see :)

[andresilva]

Good work @davxy, LGTM overall. For the RPC I'm not sure whether we should change it to take the summary and emit a notification for each block. We need to add some tests for the creation of stale heads because I am not sure if all cases are covered or whether we might be missing some heads.

@jacogr When multiple blocks are finalized at once, would getting a single finality notification for the latest block, as opposed to now getting one for each finalized block, have any effect on polkadot.js/apps?

[andresilva]

LGTM! Nice work @davxy

[bkchr]

Looks already very good, but needs some more adjustments.

[bkchr]

Ahh and @davxy please don't use force pushes :)