Populate topology after restart if finality is lagging behind current session

[alexggh]

There is a small issue on restart, where if finality is lagging across session boundary and the validator restarts, then the validator won't be able to contribute anymore with assginments/approvals and gossiping for the blocks from the previous session, because after restart it builds the Topology only for the new session, so without a topology it won't be able to distribute assignments and approvals because everything in approval-distribution is gated on having a topology for the block.

The fix is to also keep track of the last finalized block and its session and if it is different from the list of encountered sessions, build its topology and send it to the rest of subsystems.

[AndreiEres]

Do we need to import it?

[alindima]

How will this work for subsystems like statement-distribution and bitfield-ditribution which need to keep backing new candidates on the latest session? AFAIU, with this PR, they would receive the topologies of the old session that has some unfinalized blocks, which I expect will break them

[alexggh]

How will this work for subsystems like statement-distribution and bitfield-ditribution which need to keep backing new candidates on the latest session? AFAIU, with this PR, they would receive the topologies of the old session that has some unfinalized blocks, which I expect will break them

Good catch, Thanks! In statement-distribution we keep the topology in a per-session structure, so there is no problem there, however the bitfield-distribution does keep only the last received topology and that is a problem, looking into how to fix that.

[alindima]

Maybe a better approach would be to have approval-distribution request an old topology from the gossip-support subsystem. With the current form it's also a bit confusing that we're sending NewGossipTopology for an old session

[alexggh]

Maybe a better approach would be to have approval-distribution request an old topology from the gossip-support subsystem.

I've looked closer and it is not that simple because the flow right now is like:

1. Gossip-support sends NetworkBridgeRxMessage::NewGossipTopology
2. Network bridge rx receives this message adds some useful information and converts it to NetworkBridgeEvent::NewGossipTopology
3. approval-distribution, statement-distribution and bitfield-distribution receive NetworkBridgeEvent::NewGossipTopology

Requesting would mean reproducing the same flow, but in reverse: approval-distribution -> network-bridge-rx -> gossip-support, where which subsystem blocks waiting for the other to respond, I don't think we should introduce this because it is easy to produce deadlocks with this type of interactions..

With the current form it's also a bit confusing that we're sending NewGossipTopology for an old session

The message has session_index on it that clearly tells you which session the topology is for, so that allow callers to determine if they need the older ones or not.

In the end I opted to ignore older topologies for bitfield-distributions, so let me know what you think about going this route.

[alindima]

Requesting would mean reproducing the same flow, but in reverse: approval-distribution -> network-bridge-rx -> gossip-support, where which subsystem blocks waiting for the other to respond, I don't think we should introduce this because it is easy to produce deadlocks with this type of interactions..

To avoid this multi-hop, we could store them with the already enriched data in network-bridge-rx.

The message has session_index on it that clearly tells you which session the topology is for, so that allow callers to determine if they need the older ones or not.

You do have the session index there but the naming is confusing. Subsystems that use this code could easily overlook this (like a new leaf update containing an old leaf).

In the end I opted to ignore older topologies for bitfield-distributions, so let me know what you think about going this route.

There is still code that is being run in bitfield-distribution even if the update_topologies did nothing.
Maybe a good compromise would be adding a new message type? So that this change is explicit and is only handled by subsystems that need it

[paritytech-workflow-stopper]

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