substrate
substrate copied to clipboard
Fetch Babe configuration from runtime state
BabeApi::configuration
method semantic was not clearly defined.
Almost everywhere, within the codebase, this API method is used to fetch the protocol configuration data as defined on genesis. But the runtime apis are generally defined to act on the state at an arbitrary block.
The (upstream) BabeApi::configuration
implementation that ships with Substrate node was fetching some of the data from the current best block state and some other from a global constant containing a couple of genesis values (for c
and allowed_slots
params).
In other words the configuration
API doesn't return the genesis value nor the values at the requested block... but an unclear hybrid of the two.
This PR tries to clean-up the definition by:
- changing the
BabeApi::configuration
implementation within the substrate node to fetch the data at the requested block; - remove some over-engineering by removing the
sc_consensus_babe::Config
newtype. We're going to directly use the configuration type defined within the Babe's primitives (sp_consensus_babe::BabeConfiguration
). - rename
BabeGenesisConfiguration
toBabeConfiguration
. This type is not used only to store the genesis configuration, but it is used to store the protocol configuration in general.
The modification should not alter the current behavior since the configuration
cached by the various Babe components is almost only used as a fallback to get genesis data. In other words is used on the very first run and in this case the values returned by the Babe's configuration
Api are equal to the constant values used before this PR.
This section contains a summary of (previous and current) components holding a cached Babe configuration.
sc_consensus_babe::{BabeLink, BabeBlockImport, BabeSlotWorker}
, sc_consensus_babe_rpc::Babe
Cached configuration used only on genesis. Thus the values read from the runtime are equal to the genesis values. In other words, should be functionally equivalent to the previous implementation.
sc_consensus_manual_seal::BabeConsensusDataProvider
Cached configuration used only on genesis (as prev cases).
But it is also used to get slot_duration
and epoch_length
values in the append_block_import
implementation to set up the necessary import parameters.
Here the end-result is still "correct" just because these values are not allowed to change between epoch changes.
We should keep an eye on this if in the future these are allowed to change between epochs. (A NOTE in the code has been added just as a reminder and make this explicit)
Migration of EpochChanges
The old version config is migrated to the current one by using the configuration read during startup using the BabeApi
.
Previously the BabeApi::configuration
was constructed using some constant values (in particular the c
and allowed_slots
).
(see the BABE_GENESIS_EPOCH_CONFIG
in node runtime)
This is not technically correct since the c
and allowed_slots
may have changed during an epoch change;
Here we prefer to use the most up to date values on migration.
polkadot companion: https://github.com/paritytech/polkadot/pull/5842
Could you please prepare a companion PR for polkadot to introduce the same runtime API changes?
Could you please prepare a companion PR for polkadot to introduce the same runtime API changes?
@andresilva done
bot merge
Error: "Check reviews" status is not passing for https://github.com/paritytech/polkadot/pull/5842
bot merge