Various Guide Improvements

[rphmeier]

Includes:

• types chapter & candidate types

[coriolinus]

There are some nits, but it looks generally good. The most important comment I have right now is that internal links in the rest of the document should be updated:

$ rg -o 'type-definitions\.html' roadmap/implementors-guide/
Tue Jun 16 11:57:30 CEST 2020
roadmap/implementors-guide/src/runtime/configuration.md
5:type-definitions.html

roadmap/implementors-guide/src/runtime/inclusioninherent.md
19:type-definitions.html
19:type-definitions.html

roadmap/implementors-guide/src/runtime/scheduler.md
67:type-definitions.html
156:type-definitions.html

roadmap/implementors-guide/src/node/overseer.md
27:type-definitions.html
27:type-definitions.html

roadmap/implementors-guide/src/node/backing/statement-distribution.md
38:type-definitions.html

roadmap/implementors-guide/src/node/backing/candidate-backing.md
5:type-definitions.html
13:type-definitions.html
15:type-definitions.html
23:type-definitions.html
24:type-definitions.html
42:type-definitions.html

roadmap/implementors-guide/src/node/backing/candidate-selection.md
9:type-definitions.html
20:type-definitions.html

roadmap/implementors-guide/src/node/utility/provisioner.md
13:type-definitions.html
17:type-definitions.html
35:type-definitions.html

roadmap/implementors-guide/src/node/utility/candidate-validation.md
9:type-definitions.html

[coriolinus]

Would it be worth embedding an AbridgedCandidateReceipt instead of embedding the fields directly?

[rphmeier]

I didn't do this in the past for some reason, but can't remember why. Seems fine to me.

I'd also be fine with renaming CandidateReceipt to FullCandidateReceipt or FreeStanding* or Executable* or something, since the abridged version is by far more common.

[rphmeier]

The only argument I can think of against it is that embedding the fields has a flatter structure which makes instantiation for tests easier. However, in the past we didn't have a Default implementation for these types but do now, which makes this less of a concern.

[coriolinus]

I'd 👍 a simultaneous rename of CandidateReceipt -> FullCandidateReceipt, AbridgedCandidateReceipt -> CandidateReceipt, and embedding the latter into the former.

[coriolinus]

Oh, just saw that you already embedded it. Cool!

[rphmeier]

Let's do the rename in a follow-up

[coriolinus]

Looks good! A few nits/questions, but nothing which needs explicit followup, I think.

[coriolinus]

I like what you've written about the nature of the signed payloads, here and elsewhere. It's outside the scope of this PR, but I think that it might make our life easier if we extracted that behavior entirely with something like this:

struct Signed<T> {
    payload: T,
    validator_index: ValidatorIndex,
    signature: ValidatorSignature,
}

impl<T: Encode> Signed<T> {
    fn sign(payload: T, context: SigningContext, index: ValidatorIndex, key: PrivateKey) -> Signed<T> { ... }
    fn validate(&self, context: SigningContext, key: PublicKey) -> bool { ... }
}

[coriolinus]

Extracted into #1282 so that we don't lose this idea.

[coriolinus]

The descriptive text does not correspond to the enum here.

[drahnr]

we are using Hash in quite a few places here, and all have different namings

/// The block header-hash.
/// The relay-chain block hash
/// relay-parent hash

are these all equiv? Can this hash be used interminably?

[rphmeier]

Yeah, it's all the same hash type - blake2-256. Let's add this in a follow-on?

[drahnr]

does this imply guarantees that no further messages can be received from PeerId? Or does the consuming end need to take special care of that?

[rphmeier]

Yes, it does. And no messages should be sent before PeerConnected is sent.

[drahnr]

LGTM!