approval-voting: implement some TODOs and remove v2 assignment core_indices and samples

[sandreim]

This is a child PR of #6782.

Implementing some TODOs and removing redundant information as suggested in review discussion: #6782 (comment)

Changes:

• Separate transcript for v1/v2 assignments
• remove sample and core_indices from v2 certificate.
• update check_assignment_cert to return the assigned cores as outputed by VRF.
• impl TODO: bitfield instead of Vec<CandidateIndex>
• impl TODO: bitfield instead of Vec<CoreIndex>

[burdges]

We've this security implication: If we permit validators to claim less than all their assignments then the list of what they actually claim should really be signed by the extra transcript going into the VRF, or else signed by an external signature scheme, but even having an external signature scheme sounds wasteful.

At a higher level..

Do we need validators to be able to claim less than all their assignments? It's unclear. We do not have any mental model by which a validator could do this right now, and do not understand how doing this impacts security, but nothing prevents us inventing this in future, but maybe we should push validators onto common hardware instead. Also, validators could simply skip some whole blocks. We've never thought about the security implications of skipping whole blocks either of course.

tl;dr Do we leave a bitfield for functionality which we've no idea how to safely use anyways, but which morally sounds like load balancing somehow?

[sandreim]

We've this security implication: If we permit validators to claim less than all their assignments then the list of what they actually claim should really be signed by the extra transcript going into the VRF, or else signed by an external signature scheme, but even having an external signature scheme sounds wasteful.

At a higher level..

Do we need validators to be able to claim less than all their assignments? It's unclear. We do not have any mental model by which a validator could do this right now, and do not understand how doing this impacts security, but nothing prevents us inventing this in future, but maybe we should push validators onto common hardware instead. Also, validators could simply skip some whole blocks. We've never thought about the security implications of skipping whole blocks either of course.

This is a good point on which I spent some time thinking. Are we ok with the simplifying assumption that for now validators actually can claim a subset of the VRF output cores ? AFAIU this is not worse than current situation when they can just not send out any assignments and skip whole blocks. We should think of ways to fix this when we implement incentives for approval checker work.

tl;dr Do we leave a bitfield for functionality which we've no idea how to safely use anyways, but which morally sounds like load balancing somehow?

Yeah, load balancing is something that we will likely need, however I am inclined to not introduce such a bitfield yet as maybe not showing up because of load is also an option that would basically achieve the same thing. I am a little bit worried TBH, since there is a chance that due to load some candidates might not get needed_approvals votes stalling finality.

[burdges]

We have this bitfield/list now as claimed_core_indices: Vec<CoreIndex> since @rphmeier did it this way the first time (and you wrote it into #6782 too), but do we want to permit them to take fewer assignments?

I have no intuition if skipping some assignments is worse than skipping whole blocks, or visa versa. It's likely easier to analyze whole blocks somehow, but not necessarily better for security. It's possible code elegance trumps other concerns now anyways. lol

If we permit taking only some assignments, then instead of the bitfield/list they have now we should let them give a number accepted_assignments, and then they take the assignments numbered 0..accepted_assignments, which gets determined by the first 4*accepted_assignments bytes of the VRF output. An adversary having less control maybe helpful somewhere, or this maybe simplifies our analysis, but adversary won't typically care about being no-show on an irrelevant core.

there is a chance that due to load some candidates might not get needed_approvals votes stalling finality.

All candidates have a delay assignment from every validator in some tranche. We want tranche zero to fill up almost everything almost all the time, but some guys form delay assignments works fine.

[sandreim]

We have this bitfield/list now as claimed_core_indices: Vec<CoreIndex> since @rphmeier did it this way the first time (and you wrote it into #6782 too), but do we want to permit them to take fewer assignments?

Until we implement approval checker incentivisation, it is no worse than what is currently possible.

I have no intuition if skipping some assignments is worse than skipping whole blocks, or visa versa. It's likely easier to analyze whole blocks somehow, but not necessarily better for security. It's possible code elegance trumps other concerns now anyways. lol

IMO, the worse case scenario would be that at least one candidate would remain unapproved and finality stalls, skipping a whole block has the same effect.

It's possible code elegance trumps other concerns now anyways. lol

Might make sense for anything that is not production code :P

If we permit taking only some assignments, then instead of the bitfield/list they have now we should let them give a number accepted_assignments, and then they take the assignments numbered 0..accepted_assignments, which gets determined by the first 4*accepted_assignments bytes of the VRF output. An adversary having less control maybe helpful somewhere, or this maybe simplifies our analysis, but adversary won't typically care about being no-show on an irrelevant core.

I think this will be necessary probably for incentivisation, but I would rather not extend the initial scope of this change set.

[burdges]

I'll try to do a proper write up of incentivization but just fyi some notes exist at https://github.com/w3f/research-security-issues/issues/30#issuecomment-768593265 so message me on element if you do not have access.

[sandreim]

Seems that I need to update docs.

[ordian]

some questions

[ordian]

does it refer to a single candidate or a few?

[sandreim]

Refers to all candidates in assignment_bitfield.

[ordian]

I was referring to "the candidate was" part :) Should be "the candidates were"?

[sandreim]

yeah, I totally missed that :)

[ordian]

Looks reasonable to be merged into parent PR. Will take another look at the latter once it's ready.

[ordian]

I was referring to "the candidate was" part :) Should be "the candidates were"?