polkadot icon indicating copy to clipboard operation
polkadot copied to clipboard

Importing votes from approval voting on raised dispute is racy

Open eskimor opened this issue 3 years ago • 1 comments

For performance reason we changed the implementation to no longer import approval votes eagerly, instead we only import them once a dispute is raised. Unfortunately approval-voting and approval-distribution will abandon any votes as soon as a fork can no longer be finalized (because another fork with a larger block height already has been finalized). Therefore for a candidate that only has been included on one fork, we have no guarantees that by the time the dispute-coordinator fetches votes, they will still be available.

In theory it would therefore be possible for malicious approval checkers to avoid a slash if that race can be exploited. This is not super critical as the validators who are supposed to have skin in the game (the backing nodes) will still get slashed, so gambler's ruin still holds in all cases, still it would be good to maximize slashes on malicious nodes no matter what.

Proposed solutions:

  1. Either revert the change to eagerly import approval votes into the dispute-coordinator, but make sure it is efficient and avoid the quadratic complexity it currently has.
  2. Change requirements on approval-voting and make it preserve votes a bit longer than necessary.

I don't have a strong opinion on either of those. 2) will likely be a bit more efficient as votes are already kept, only the requirements on how long to keep them change a bit. "How long?" is a subtle question though, anything too short might still get attacked to avoid slashes. 1) is likely a bit cleaner, but imposes more overhead and requires some engineering to make fast.

With a little thought other more elegant solutions might also come to mind.

eskimor avatar Aug 08 '22 10:08 eskimor

We must keep all approval votes slightly longer so that disputes can access them? We'd stop gossiping them once the fork can no longer be finalized, but then restart gossiping them once the dispute comes in?

If memory is a concern then we could persist votes to disk when the fork dies I think. What do votes contain? An authority id and candidate hash should be 32 bytes or less each, the signature should be 64 bytes, so no reason for them to exceed 150 bytes even if they need some other hash, and they could be persisted in just less 96 bytes each We do not need the assignment anymore once we've given up on ever finalizing this block.

burdges avatar Aug 08 '22 11:08 burdges