approval-voting: Make sure we always mark approved candidates approved in a different relay chain context

[alexggh]

... see for more detail why this is needed
#4149 (comment)

TODO:

• Unittests
• Replicate scenario from Small network finality stall (Westend) #4149 and confirm this fixes it: Small network finality stall (Westend) #4149 [ Replicated on a zombienet with some hacked nodes, that we can end up in this state where no-wake is schedule and the nodes are pending new assignments]

[ordian]

Great find!

The root cause analysis seems on point, but I'm not sure about the fix itself. I mean it does fix this scenario, but I don't know if can create other issues or can be abused, so maybe it's a good idea to summon @rphmeier for discussion here.

[sandreim]

Great find!

The root cause analysis seems on point, but I'm not sure about the fix itself. I mean it does fix this scenario, but I don't know if can create other issues or can be abused, so maybe it's a good idea to summon @rphmeier for discussion here.

The fix seems very subtle to me. I didn't think this too deep, but an alternative would be to approve it under all blocks known in CandidateEntry::block_assignments once it is approved on either of them.

[alexggh]

Great find!
The root cause analysis seems on point, but I'm not sure about the fix itself. I mean it does fix this scenario, but I don't know if can create other issues or can be abused, so maybe it's a good idea to summon @rphmeier for discussion here.

The fix seems very subtle to me. I didn't think this too deep, but an alternative would be to approve it under all blocks known in CandidateEntry::block_assignments once it is approved on either of them.

@sandreim convinced me offline that this ^^^ is the proper fix, will come back with a new version, marking the PR as draft until I get that done.

[sandreim]

Just discussing this with @eskimor . This might reduce security slightly, since attackers get more chances(as amount of forks) to have all the bad guys in tranche0 and approve something invalid. CC @burdges

[alexggh]

Just discussing this with @eskimor . This might reduce security slightly, since attackers get more chances(as amount of forks) to have all the bad guys in tranche0 and approve something invalid. CC @burdges

You mean if we mark the candidate approved in other blocks when it got approved in one of the fork, yeah that's right we would increase the chances of an attacker.
We actually need to call everything in advance_approval_state which would use the assignments on R1 with the approvals we received on all forks(R1 + R1') and mark the candidate approved on R1 only if all the assignments on R1 had been properly covered. So, that is actually what this PR in this form does.

[alexggh]

Ended up making this fix more explicit by scheduling a waking up on forked relay blocks, only if there is no wake up scheduled and if the validator in question already has an assignment on it.

Additionally, improved the unittest to test the exact scenario I described here: #4149 (comment).

@sandreim @ordian let me know what you think. Marking it as ready for re-review.

[sandreim]

Thanks @alexggh this much better now 👍🏼

[ordian]

Looking good to me. Nice work!

[burdges]

I've no idea how I missed this one, so..

We do assignments using private VRFs of the relay chain VRF when the candidate gets included, so candidates have different assignments based upon when they're included.

We should not give adversaries more control over the selection of approval checkers, but there are roughly two axies here:

1. enough adversarial assignments - controls when adversaries can launch attacks, but adversaries can already control this by simply waiting, so no new concerns here.
2. few honest assignments - controls when adversaries could win in their attacks, so adversaries should not gain new influence here.

Abstractly, once assignments exist then they should be executed, possibly creating remote disputes. In principle, if relay block R includes parablock X then finality for R should ideally wait upon all assignments for X on all relay chain forks, but how much does this matter?

We've always worried about 2 in the context of an adversary making two forks, one which includes the core and one which abandons the core, with which the adversary kills the including fork if approval chekers look unfavorable. In this, the abandonment fork could safely finalize early, but then the assignments should still occur on the inclusion fork, and create a remote dispute. As the adversay loses temporarily, we ensure they exaust all the dots before winning.

In theory, adversaries could include the same candidate in many relay chain forks, and then advance and finalize only the fork where we assign too few honest checkers. We never really covered this case in the security analysis, but does the adversary have enough checkers?

As an off the cuff approximation, an adversary needs core collisions in multi-sets given by their own tranche zero assignments. We have p = num_samples / num_cores odds of a fixed core being inside a fixed validator's tranche zero, so then the number of adversarial validators like this one has the Binomial distribution B(n=num_validators/3, p) and num_cores ways that could happen. As k = needed_approvals-1 > n p we have Pr(X ≥ k) = F(n-k; n, 1-p) < exp[ -2n (p - k/n)^2 ].

We'll take p = E/num_validators >?< needed_approvals/num_validators using the usual approximations discussed in #640, so then p - k/n >?< (2/3) (k/n). If >?< is = then Pr(X ≥ k) < exp[ - 8/9 n (k/n)^2 ].

I remembered much smaller numbers there, so maybe I've screwed up the above, but this suggests adversaries have simple attack windows like 1% of the time, but the multi-attack windows still strink rapidly in size.