Integrate new Aura / Parachain Consensus Logic in Parachain-Template / Polkadot-Parachain

[rphmeier]

Closes #2476

This is a catch-all PR which:

• Updates the basic and lookahead collators to explicitly produce Futures which are Send
• Migrates node-template and polkadot-parachain to use some of the new consensus infrastructure, deprecating some old pieces
• Migrates node-template to use the new basic collator
• Adds a function to the aura-ext::FixedVelocityConsensusHook which can be used to implement the AuraUnincludedSegmentApi

[BradleyOlson64]

Are the futures in the run() functions of lookahead.rs and basic.rs examples of the "consensus futures" you mention here?

Perhaps we would call said run() functions instead of old_consensus::start_collator() in parachain-template/node/src/service.rs to fully activate async backing aura?

[rphmeier]

Yes, exactly.

The first step is to use the basic.rs future, which is intended to have the exact same behavior as current Aura. Then, when asynchronous backing is being enabled, the lookahead.rs future would be swapped in.

[BradleyOlson64]

Didn't find any issues this time.

Now that the new basic aura collator is plugged in, I can take a first pass at showing those changes in the user update guide.

[slumber]

Isn't it responsibility of can_build_upon?

I'm having hard time understanding "graceful" handling in this todo.

[rphmeier]

We don't use can_build_upon in this collator, as it's intended to be fully backwards compatible, so doesn't depend on AuraUnincludedSegmentApi. Unfortunately, we can't "half-depend" on it (i.e. use it when it's present and not when it's not) because of https://github.com/paritytech/substrate/issues/13331 .

This does lead to panics when the runtime does have an unincluded segment of max size 1 and the collation-generation subsystem requests blocks to be built when there is a candidate pending.

Honestly, this change is unnecessary because of paritytech/polkadot#7514 so I will just remove it.

[rphmeier]

We could add a dependency on AuraUnincludedSegmentApi but it then requires the user to implement that in their most recent version of the native runtime, which I'm not sure we should require. Maybe useful to do that.

[slumber]

I guess this should use max_ancestry_lookback which already hardcodes 2.

[rphmeier]

This is actually more of a velocity hardcoding than ancestry lookback. It will need to be adjusted when we start dealing with dynamic velocities, but saving that for a later date as velocity won't exceed 1 per relay-block for a while.

The specific behavior we're dealing with is that most parachains hardcode an Aura slot duration of 12 seconds, so they can actually author at 2 different relay parents. These chains should set velocity=2 in their runtime as they expect 2 blocks to be included per slot. That allows them to author 3 blocks per slot when the unincluded segment has space, and this change just spreads that out and showed better results in testing.

Generalizing a bit - any slot duration which is a multiple of the relay chain slot duration probably doesn't want to try authoring all the blocks in their slot at the front, but rather spread them out.

It's definitely not ideal or particularly elegant, but couldn't think of a better solution at the time.

The alternative is to keep track in aura-ext how many blocks are authored per relay-chain slot not per parachain slot, and change can_build_upon to `can_build_upon(included_hash, relay_chain_slot) . But then slot durations become kind of meaningless in a relay-chain setting, except for rotating keys.

WDYT?

[slumber]

Yes, my initial comment is not correct, first I was confused where does hardcoded 2 come from, then noticed the below function and assumed it's the same.

We had this discussion about filling backlog in the first PR, and I guess looping while can_build is ok for the start. When I was thinking about this, I imagined parachains would somehow customize this rate of block production, like

enum BlockProductionRate {
    /// Only author every N seconds.
    Fixed(Duration),
    /// No fixed block time, attempt to author whenever possible,
    Asap,
    // ... and more
}

[rphmeier]

Let's revisit this in a new issue. The design space is pretty open, though I think the best the runtime can do is give "hints" and not expect them to be obeyed.