Retry approval on availability failure if the check is still needed

[alexggh]

Recovering the POV can fail in situation where the node just restart and the DHT topology wasn't fully discovered yet, so the current node can't connect to most of its Peers. This is bad because for gossiping the assignment you need to be connected to just a few peers, so because we can't approve the candidate and other nodes will see this as a no show.

This becomes bad in the scenario where you've got a lot of nodes restarting at the same time, so you end up having a lot of no-shows in the network that are never covered, in that case it makes sense for nodes to actually retry approving the candidate at a later data in time and retry several times if the block containing the candidate wasn't approved.

TODO

• Add a subsystem test.

[Polkadot-Forum]

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/2025-11-25-kusama-parachains-spammening-aftermath/11108/1

[burdges]

Aside: JAM puts validators' IP etc on-chain. We could figure out if on-chain is better? Ideally we'd want on-chain to equivocation defesnes anyways.

[AndreiEres]

Can this somehow interfere in normal mode?

[alindima]

LGTM

[ordian]

Alternatively, we could change the

polkadot-sdk/polkadot/node/network/availability-recovery/src/task/strategy/full.rs

Line 89 in ca78179

IfDisconnected::ImmediateError,

to try connect if we are disconnected. This doesn't have a more granular retry functionality, but a much simpler change.

[alindima]

Alternatively, we could change the

polkadot-sdk/polkadot/node/network/availability-recovery/src/task/strategy/full.rs

Line 89 in ca78179

IfDisconnected::ImmediateError,

to try connect if we are disconnected. This doesn't have a more granular retry functionality, but a much simpler change.

We could do this, but I doubt this will make a difference. If the strategy of fetching from backers fails, we fall back to chunk fetching which uses IfDisconnected::TryConnect.

Alex is saying that:

current node can't connect to most of its Peers.

If it were just an issue of not trying to connect, then the chunk recovery should have worked

[ordian]

Aside: JAM puts validators' IP etc on-chain. We could figure out if on-chain is better? Ideally we'd want on-chain to equivocation defesnes anyways.

Agree, we should explore this. This would also solve the problem with restarts and empty DHT cache.

[ordian]

Approving it, but it feels like a workaround for an unreliable/slow authority discovery, which we should fix and then remove this workaround.

[alexggh]

Approving it, but it feels like a workaround for an unreliable/slow authority discovery, which we should fix and then remove this workaround.

I did discussed with @lexnv about speeding authority-discovery by saving the cache on disk to have it available on restart, however even with a speedy authority discovery, I still think this would be a good thing to have for robustness, because it is a fallback when networking calls fail from various other reasons, network calls are expected to fail every now and then.

[sandreim]

LGTM! @ordian proposed an alternative solution which should live in AD, but I agree that this fix adds some robustness on top of AD.

From the back of my head, a better place to implement PoV retry would is in the availability recovery subsystem. This would be superior in terms of latency (it's push vs pull/poll) as this subsystem could keep track of all these PoVs that failed to be retrieved and retry immediately to fetch chunks as soon as peers connect (or even more complex strategies - like speculative availability).

The only change needed in approval voting would be to notify av recovery that it's no longer interested in some PoV.

WDYT ?

[sandreim]

As time passes the chances of connecting to enough peers increases, wouldn't it make sense to decrease the back-off as the retry count increases ? This would help approve candidate faster.

[alexggh]

Reconnecting to peers is in the oder of minutes, so we wouldn't gain much by reducing the backoff, also usually with backoffs you actually want to increase it as the number of attempts increase because you don't want to end up in a situation where you many failed attempts start stampeding, which makes things worse, however we can't increase it here because of this: #6807 (comment), so I think 1min is an acceptable compromise.

[alindima]

From the back of my head, a better place to implement PoV retry would is in the availability recovery subsystem. This would be superior in terms of latency (it's push vs pull/poll) as this subsystem could keep track of all these PoVs that failed to be retrieved and retry immediately to fetch chunks as soon as peers connect (or even more complex strategies - like speculative availability).

The only change needed in approval voting would be to notify av recovery that it's no longer interested in some PoV.

I don't think this is particularly better. Latency of transmitting messages between subsystems should be negligible.
av-recovery is being used by multiple actors: dispute-coordinator, approval-voting and collator pov-recovery. I doubt that they all need the same amount of retries and in the same scenarios. In addition, I'd like to keep this separation of concerns that we have now. av-recovery is tasked with recovering the available data. If it's not possible, higher level code decides what needs to be done

[sandreim]

I don't think this is particularly better. Latency of transmitting messages between subsystems should be negligible. av-recovery is being used by multiple actors: dispute-coordinator, approval-voting and collator pov-recovery.

Yes, but why is this a bad thing ? If PoV recovery fails dispute-coordinator currently gives up and it would be more robust to use a retry mechanism.

I doubt that they all need the same amount of retries and in the same scenarios. In addition, I'd like to keep this separation of concerns that we have now. av-recovery is tasked with recovering the available data.

My proposal doesn't change the concerns of av-recovery, but makes it more robust. It should be easy to pass the retry params in the RecoverAvailableData message.

[alexggh]

From the back of my head, a better place to implement PoV retry would is in the availability recovery subsystem. This would be superior in terms of latency (it's push vs pull/poll) as this subsystem could keep track of all these PoVs that failed to be retrieved and retry immediately to fetch chunks as soon as peers connect (or even more complex strategies - like speculative availability).

The only change needed in approval voting would be to notify av recovery that it's no longer interested in some PoV.

I don't think this is particularly better. Latency of transmitting messages between subsystems should be negligible. av-recovery is being used by multiple actors: dispute-coordinator, approval-voting and collator pov-recovery. I doubt that they all need the same amount of retries and in the same scenarios. In addition, I'd like to keep this separation of concerns that we have now. av-recovery is tasked with recovering the available data. If it's not possible, higher level code decides what needs to be done

Alright, I looked a bit on this and moving it in the availability-recovery wouldn't be a straight-forward task, the reason for this is because the implementers of RecoveryStrategy are stateful, consumed and can't be cloned, so you can't simply re-run the strategies from here:

polkadot-sdk/polkadot/node/network/availability-recovery/src/task/mod.rs

Line 147 in baa3bcc

while let Some(current_strategy) = self.strategies.pop_front() {

, so you would end-up having to propagate the failure here:

polkadot-sdk/polkadot/node/network/availability-recovery/src/lib.rs

Line 800 in baa3bcc

output = state.ongoing_recoveries.select_next_some() => {

and having to re-run the logic for building the recovery-strategy from there, that means that you also have to carry along the parameters needed for rebuilding the strategies array.

I don't think that would be better and easier and less risky to understand and implement than the current proposed approach, but it would give us the benefit that disputes could also opt-in to use it.

Given the current PR improves the situation, I would be inclined to keep the current approach rather than invest in getting this knob in availability-recovery, let me know what you think!

[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/12654933804
Failed job name: cargo-clippy

[sandreim]

Alright, I looked a bit on this and moving it in the availability-recovery wouldn't be a straight-forward task, the reason for this is because the implementers of RecoveryStrategy are stateful, consumed and can't be cloned, so you can't simply re-run the strategies from here:

polkadot-sdk/polkadot/node/network/availability-recovery/src/task/mod.rs

Line 147 in baa3bcc

while let Some(current_strategy) = self.strategies.pop_front() {

, so you would end-up having to propagate the failure here:

polkadot-sdk/polkadot/node/network/availability-recovery/src/lib.rs

Line 800 in baa3bcc

output = state.ongoing_recoveries.select_next_some() => {

and having to re-run the logic for building the recovery-strategy from there, that means that you also have to carry along the parameters needed for rebuilding the strategies array.
I don't think that would be better and easier and less risky to understand and implement than the current proposed approach, but it would give us the benefit that disputes could also opt-in to use it.

Given the current PR improves the situation, I would be inclined to keep the current approach rather than invest in getting this knob in availability-recovery, let me know what you think!

The only major downside of the current approach is that for each retry we re-fetch the same chunks from the previous attempt, so worse case scenario - we'd be roughly fetching the PoV 16 times, which increases the bandwidth cost.

[paritytech-cmd-bot-polkadot-sdk]

Created backport PR for stable2407:

• [stable2407] Backport #6807 #7155 with remaining conflicts!

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-6807-to-stable2407
git worktree add --checkout .worktree/backport-6807-to-stable2407 backport-6807-to-stable2407
cd .worktree/backport-6807-to-stable2407
git reset --hard HEAD^
git cherry-pick -x 6878ba1f399b628cf456ad3abfe72f2553422e1f
git push --force-with-lease

[paritytech-cmd-bot-polkadot-sdk]

Created backport PR for stable2409:

• [stable2409] Backport #6807 #7156 with remaining conflicts!

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-6807-to-stable2409
git worktree add --checkout .worktree/backport-6807-to-stable2409 backport-6807-to-stable2409
cd .worktree/backport-6807-to-stable2409
git reset --hard HEAD^
git cherry-pick -x 6878ba1f399b628cf456ad3abfe72f2553422e1f
git push --force-with-lease

[paritytech-cmd-bot-polkadot-sdk]

Successfully created backport PR for stable2412:

• [stable2412] Backport #6807 #7157