Validator side of the collation protocol.

[rphmeier]

Each collator collates for a specific parachain. A validator manages connected collators and assigns one of them for each parachain the "primary" role, while others are "backup". The primary is intended to push a collation to the validator whenever the validator is assigned to a specific parachain, while the backups do not.

This is implemented primarily with the CollatorPool struct, which is owned by the PolkadotNetwork. The ConsensusNetwork struct implements the polkadot_consensus::Collators trait faithfully now, by forwarding to the referenced PolkadotNetwork APIs.

Follow-up work includes:

• saner key authentication protocol (Cryptographic authentication of validators' session keys in network #271)
• collator side of the network protocol
• collator peer discovery
• role reassignment based on performance

[gnunicorn]

Qs about some assumptions, minor remarks and a few lines that are quite a bit too long ;) .

[gnunicorn]

It is guaranteed that any AccountId can at most be part of one parachain collator group ever and not be part of two or more parachains in their lifetime? Like, also not switch parachains? Because we overwrite any potential entry here without updating the the corresponding ParachainCollator if it exists (which could lead to an incoherent state and cause locking, like when not removing a primary AccountId when it disconnects, because we aren't aware it is still stored there.)

[rphmeier]

#271 will ensure that we are connected to only one peer ID with a claimed AccountId at a time, and each AccountId can only collate for one parachain during the span of the connection in the current protocol.

[gnunicorn]

Not removing it from backup? That seems weird, because that would mean our backup contains disconnected AccountIds and thus the lines above could assign a disconnected AccountId as primary. Is that intentional?

[gnunicorn]

If I understand that Message right, this peer is informing us about our CollatorRole, no? And we just ignore it? Didn't it say up there in a TODO that we should get punished if we try to collate even if we aren't primary? Or is the implementation of this going to be part of the follow-up PR? Even in that case, maybe have a ToDo here, too?

[rphmeier]

that's the follow-up PR. Can add a TODO but it'll follow-up really shortly so I don't know for whose benefit that is.

[gnunicorn]

That line is really too long to read ;) (150 chars)

[gnunicorn]

on_collation above states that we must have checked the signature prior, and although this happens in the match check before, arriving here, that hasn't necessarily happened, does it? Also relying on a check to have happened in the match prior sounds like something that could easily be missed when changing this later. I'd prefer to have a more explicit guard around it, that makes sure check_signature has been called before on_collation regardless. Something like

Some((ref acc_id, ref para_id)) => {
  if para_id == &collation_para && acc_id == &collated_acc && collation.receipt.check_signature().is_ok() {
      self.collators.on_collation(acc_id.clone(), relay_parent, collation)
  } else {
    ctx.disable_peer(from)
  }
}

[rphmeier]

have rewritten to this before seeing this comment. Agree

[gnunicorn]

125 chars. please wrap ;) .

[pepyakin]

Shouldn't this take block_data_hash?

[pepyakin]

Looks good to me!

[gavofyork]

A validator ... assigns one of [the connected collators] for each parachain the "primary" role, while others are "backup". The primary is intended to push a collation to the validator whenever the validator is assigned to a specific parachain, while the backups do not.

this doesn't sound like The Plan. why the big change? validators should be listening out for candidate blocks on their current parachain generally and figuring out the best candidate with some metric (tbd). they shouldn't be picking a winning collator up front.

[rphmeier]

validators should be listening out for candidate blocks on their current parachain generally and figuring out the best candidate with some metric (tbd). they shouldn't be picking a winning collator up front.

this refers to the collations put forth by validators, not the process by which validators choose which collation to put forth into the consensus.

if each validator accepts block proposals from all connected collators it will probably use up too much network bandwidth. that notwithstanding, the validators have to choose fairly arbitrarily which collation to put forth because there is simply not enough compute-time to execute all prospective witnessed collations before making a proposal to the parachain validation group.

I'm not particularly tied to the primary/backup scheme, it was just the simplest thing to implement for PoC-2 other than accepting collations from all collators. But we can also change the primary periodically or have multiple primaries at a time.

[gnunicorn]

thanks.

[gavofyork]

so for poc-2 this is fine. but eventually (poc-5/6), i'd like to see a time-limit-based approach and as many PoV candidates evaluated as possible within it, ideally parallelised over cores. some of the game theory (such as golden tickets for collators to encourage collator-rotation) doesn't work unless validators evaluate multiple candidates.

[gavofyork]

paritytech/polkadot#301