Independence of Slot-based algorithms from system Timestamp
Removal from SlotInfo:
- This struct field is just redundant.
- If required it can be easily computed as
slot_info.slot * slot_info.duration.
(Edited)
Slot based algorithms such as Babe, Aura and (in the future) Sassafras should not explicitly depend on Timestamp.
This PR relaxes this requirement by removing direct timestamp inclusion from Slots crate.
If the slot value is computed using the system timestamp then this should be a user choice part of the node construction (i.e. within the bin).
Note that this also simplifies the creation of slot values for future for other consensus algorithms using a different criteria.
For example I wish to at least try (or keep the door open) to implement 2 different slot providers for Sassafras:
- one using system time (as the one used by babe right now)
- one using the median algorithm described by the babe paper (thus that relies on information that abstracts the physical system)
polkadot companion: https://github.com/paritytech/polkadot/pull/5997 cumulus companion: https://github.com/paritytech/cumulus/pull/1617
The dependency on timestamp was used to perform the check that the Timestamp set by the block author is in alignment with the slot selected by the author. e.g. to ensure that authors don't set timestamps in the future, which are not coordinated with the slot.
Does this keep that property? It'd be nice to have a test if so.
Does this keep that property? It'd be nice to have a test if so.
@rphmeier the property is enforced by exactly the same means as before.
In practice the check is performed in Babe here: https://github.com/paritytech/substrate/blob/dd1f1b5f291aa2325eddf5f26037b84cfbbe0957/client/consensus/babe/src/verification.rs#L84-L87
And in AURA here: https://github.com/paritytech/substrate/blob/dd1f1b5f291aa2325eddf5f26037b84cfbbe0957/client/consensus/aura/src/import_queue.rs#L77-L80
The check consists in ensuring that the slot used by the block author is less than slot_now.
The way slot_now is computed is opaque as before (in "practice" as a function of timestamp).
I'll check it a test exists already. Otherwise I'm going to add one.
Thanks for clarifying!
We also have these checks inside the runtime: https://github.com/paritytech/substrate/blob/master/frame/babe/src/lib.rs#L866-L869
To ensure that timestamp and slot are matching.
@andresilva your opinion on this?
bot merge