paritytech / substrate

Substrate: The platform for blockchain innovators
Apache License 2.0
8.39k stars 2.65k forks source link

pallet-aura: add feature-flagged explicit slot duration type #14649

Closed rphmeier closed 1 year ago

rphmeier commented 1 year ago

This is feature-flagged for backwards compatibility - runtimes which are intending to upgrade to asynchronous backing should make use of this.

The main motivation of this change is that computing the slot duration with the MinimumPeriod doesn't play nicely with asynchronous backing and authoring multiple blocks per slot. MinimumPeriod for Cumulus chains doesn't really make a whole lot of sense, as the only clock that they can be compared to is the relay-chain slot number. We will recommend that parachains run with MinimumPeriod = 0, so this change is necessary to keep Aura working in that situation.

I added this with the experimental feature flag with LTS in mind.

rphmeier commented 1 year ago

I'm guessing this should be included in the User Update Guide?

Yeah, specifically for the parts where asynchronous backing is actually enabled and more than one block needs to be made per slot.

rphmeier commented 1 year ago

Can someone in @paritytech/sdk-node comment on whether it's better to just do the breaking change or to add the feature flag as proposed here?

davxy commented 1 year ago

Can someone in @paritytech/sdk-node comment on whether it's better to just do the breaking change or to add the feature flag as proposed here?

IMO the feature flag is a good way to gently propose a change. However should also be considered that this breaking change is not "silent". Maybe an explicit comment about the backward compatible default type would be useful and a matter of a very small change for the user...

Even better would be to use associated_types_default. But is unstable :-/

rphmeier commented 1 year ago

bot merge

paritytech-processbot[bot] commented 1 year ago

Waiting for commit status.

paritytech-processbot[bot] commented 1 year ago

Merge cancelled due to error. Error: Statuses failed for 9558faa765361ac912005beb13674db54352e009

rphmeier commented 1 year ago

bot merge

paritytech-processbot[bot] commented 1 year ago

Waiting for commit status.