pezkuwichain/pezkuwi-subxt
1
0
Fork 0
mirror of https://github.com/pezkuwichain/pezkuwi-subxt.git synced 2026-06-12 13:31:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

Stop keeping track of epoch changes for the sync gap (#13127)

Browse Source 
* Stop keeping track of epoch changes data within the sync gap

* Fix docs

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <git@kchr.de>

* Fix typo

Co-authored-by: Bastian Köcher <git@kchr.de>
This commit is contained in:
Davide Galassi
2023-01-12 20:36:31 +01:00
committed by GitHub
parent cf7bf3d142
commit 7aa88e6a59
3 changed files with 33 additions and 454 deletions
Download Patch File Download Diff File Expand all files Collapse all files
substrate/client/consensus/babe/src/lib.rs
+29 -22
View File
@@ -111,7 +111,7 @@ use sp_api::{ApiExt, ProvideRuntimeApi};
use sp_application_crypto::AppKey;
use sp_block_builder::BlockBuilder as BlockBuilderApi;
use sp_blockchain::{
Backend as _, Error as ClientError, ForkBackend, HeaderBackend, HeaderMetadata,
Backend as _, BlockStatus, Error as ClientError, ForkBackend, HeaderBackend, HeaderMetadata,
Result as ClientResult,
};
use sp_consensus::{
@@ -1133,11 +1133,17 @@ where
let hash = block.header.hash();
let parent_hash = *block.header.parent_hash();
if block.with_state() {
// When importing whole state we don't calculate epoch descriptor, but rather
// read it from the state after import. We also skip all verifications
// because there's no parent state and we trust the sync module to verify
// that the state is correct and finalized.
let info = self.client.info();
let number = *block.header.number();
if info.block_gap.map_or(false, |(s, e)| s <= number && number <= e) || block.with_state() {
// Verification for imported blocks is skipped in two cases:
// 1. When importing blocks below the last finalized block during network initial
// synchronization.
// 2. When importing whole state we don't calculate epoch descriptor, but rather
// read it from the state after import. We also skip all verifications
// because there's no parent state and we trust the sync module to verify
// that the state is correct and finalized.
return Ok((block, Default::default()))
}
@@ -1399,18 +1405,24 @@ where
) -> Result<ImportResult, Self::Error> {
let hash = block.post_hash();
let number = *block.header.number();
let info = self.client.info();
// early exit if block already in chain, otherwise the check for
// epoch changes will error when trying to re-import an epoch change
match self.client.status(hash) {
Ok(sp_blockchain::BlockStatus::InChain) => {
// When re-importing existing block strip away intermediates.
let _ = block.remove_intermediate::<BabeIntermediate<Block>>(INTERMEDIATE_KEY);
block.fork_choice = Some(ForkChoiceStrategy::Custom(false));
return self.inner.import_block(block, new_cache).await.map_err(Into::into)
},
Ok(sp_blockchain::BlockStatus::Unknown) => {},
Err(e) => return Err(ConsensusError::ClientImport(e.to_string())),
let block_status = self
.client
.status(hash)
.map_err(|e| ConsensusError::ClientImport(e.to_string()))?;
// Skip babe logic if block already in chain or importing blocks during initial sync,
// otherwise the check for epoch changes will error because trying to re-import an
// epoch change or because of missing epoch data in the tree, respectivelly.
if info.block_gap.map_or(false, |(s, e)| s <= number && number <= e) ||
block_status == BlockStatus::InChain
{
// When re-importing existing block strip away intermediates.
// In case of initial sync intermediates should not be present...
let _ = block.remove_intermediate::<BabeIntermediate<Block>>(INTERMEDIATE_KEY);
block.fork_choice = Some(ForkChoiceStrategy::Custom(false));
return self.inner.import_block(block, new_cache).await.map_err(Into::into)
}
if block.with_state() {
@@ -1506,8 +1518,6 @@ where
)),
}
let info = self.client.info();
if let Some(next_epoch_descriptor) = next_epoch_digest {
old_epoch_changes = Some((*epoch_changes).clone());
@@ -1701,9 +1711,6 @@ where
Client: HeaderBackend<Block> + HeaderMetadata<Block, Error = sp_blockchain::Error>,
{
let info = client.info();
if info.block_gap.is_none() {
epoch_changes.clear_gap();
}
let finalized_slot = {
let finalized_header = client
substrate/client/consensus/epochs/src/lib.rs
+2 -430
View File
@@ -320,106 +320,6 @@ impl<E: Epoch> AsRef<E> for IncrementedEpoch<E> {
}
}
/// A pair of epochs for the gap block download validation.
/// Block gap is created after the warp sync is complete. Blocks
/// are imported both at the tip of the chain and at the start of the gap.
/// This holds a pair of epochs that are required to validate headers
/// at the start of the gap. Since gap download does not allow forks we don't
/// need to keep a tree of epochs.
#[derive(Clone, Encode, Decode, Debug)]
pub struct GapEpochs<Hash, Number, E: Epoch> {
current: (Hash, Number, PersistedEpoch<E>),
next: Option<(Hash, Number, E)>,
}
impl<Hash, Number, E> GapEpochs<Hash, Number, E>
where
Hash: Copy + PartialEq + std::fmt::Debug,
Number: Copy + PartialEq + std::fmt::Debug,
E: Epoch,
{
/// Check if given slot matches one of the gap epochs.
/// Returns epoch identifier if it does.
fn matches(
&self,
slot: E::Slot,
) -> Option<(Hash, Number, EpochHeader<E>, EpochIdentifierPosition)> {
match &self.current {
(_, _, PersistedEpoch::Genesis(epoch_0, _))
if slot >= epoch_0.start_slot() && slot < epoch_0.end_slot() =>
return Some((
self.current.0,
self.current.1,
epoch_0.into(),
EpochIdentifierPosition::Genesis0,
)),
(_, _, PersistedEpoch::Genesis(_, epoch_1))
if slot >= epoch_1.start_slot() && slot < epoch_1.end_slot() =>
return Some((
self.current.0,
self.current.1,
epoch_1.into(),
EpochIdentifierPosition::Genesis1,
)),
(_, _, PersistedEpoch::Regular(epoch_n))
if slot >= epoch_n.start_slot() && slot < epoch_n.end_slot() =>
return Some((
self.current.0,
self.current.1,
epoch_n.into(),
EpochIdentifierPosition::Regular,
)),
_ => {},
};
match &self.next {
Some((h, n, epoch_n)) if slot >= epoch_n.start_slot() && slot < epoch_n.end_slot() =>
Some((*h, *n, epoch_n.into(), EpochIdentifierPosition::Regular)),
_ => None,
}
}
/// Returns epoch data if it matches given identifier.
pub fn epoch(&self, id: &EpochIdentifier<Hash, Number>) -> Option<&E> {
match (&self.current, &self.next) {
((h, n, e), _) if h == &id.hash && n == &id.number => match e {
PersistedEpoch::Genesis(ref epoch_0, _)
if id.position == EpochIdentifierPosition::Genesis0 =>
Some(epoch_0),
PersistedEpoch::Genesis(_, ref epoch_1)
if id.position == EpochIdentifierPosition::Genesis1 =>
Some(epoch_1),
PersistedEpoch::Regular(ref epoch_n)
if id.position == EpochIdentifierPosition::Regular =>
Some(epoch_n),
_ => None,
},
(_, Some((h, n, e)))
if h == &id.hash &&
n == &id.number && id.position == EpochIdentifierPosition::Regular =>
Some(e),
_ => None,
}
}
/// Import a new gap epoch, potentially replacing an old epoch.
fn import(&mut self, slot: E::Slot, hash: Hash, number: Number, epoch: E) -> Result<(), E> {
match (&mut self.current, &mut self.next) {
((_, _, PersistedEpoch::Genesis(_, epoch_1)), _) if slot == epoch_1.end_slot() => {
self.next = Some((hash, number, epoch));
Ok(())
},
(_, Some((_, _, epoch_n))) if slot == epoch_n.end_slot() => {
let (cur_h, cur_n, cur_epoch) =
self.next.take().expect("Already matched as `Some`");
self.current = (cur_h, cur_n, PersistedEpoch::Regular(cur_epoch));
self.next = Some((hash, number, epoch));
Ok(())
},
_ => Err(epoch),
}
}
}
/// Tree of all epoch changes across all *seen* forks. Data stored in tree is
/// the hash and block number of the block signaling the epoch change, and the
/// epoch that was signalled at that block.
@@ -435,14 +335,10 @@ where
/// same DAG entry, pinned to a specific block #1.
///
/// Further epochs (epoch_2, ..., epoch_n) each get their own entry.
///
/// Also maintains a pair of epochs for the start of the gap,
/// as long as there's an active gap download after a warp sync.
#[derive(Clone, Encode, Decode, Debug)]
pub struct EpochChanges<Hash, Number, E: Epoch> {
inner: ForkTree<Hash, Number, PersistedEpochHeader<E>>,
epochs: BTreeMap<(Hash, Number), PersistedEpoch<E>>,
gap: Option<GapEpochs<Hash, Number, E>>,
}
// create a fake header hash which hasn't been included in the chain.
@@ -460,7 +356,7 @@ where
Number: Ord,
{
fn default() -> Self {
EpochChanges { inner: ForkTree::new(), epochs: BTreeMap::new(), gap: None }
EpochChanges { inner: ForkTree::new(), epochs: BTreeMap::new() }
}
}
@@ -480,11 +376,6 @@ where
self.inner.rebalance()
}
/// Clear gap epochs if any.
pub fn clear_gap(&mut self) {
self.gap = None;
}
/// Map the epoch changes from one storing data to a different one.
pub fn map<B, F>(self, mut f: F) -> EpochChanges<Hash, Number, B>
where
@@ -493,10 +384,6 @@ where
{
EpochChanges {
inner: self.inner.map(&mut |_, _, header: PersistedEpochHeader<E>| header.map()),
gap: self.gap.map(|GapEpochs { current: (h, n, header), next }| GapEpochs {
current: (h, n, header.map(&h, &n, &mut f)),
next: next.map(|(h, n, e)| (h, n, f(&h, &n, e))),
}),
epochs: self
.epochs
.into_iter()
@@ -536,9 +423,6 @@ where
/// Get a reference to an epoch with given identifier.
pub fn epoch(&self, id: &EpochIdentifier<Hash, Number>) -> Option<&E> {
if let Some(e) = &self.gap.as_ref().and_then(|gap| gap.epoch(id)) {
return Some(e)
}
self.epochs.get(&(id.hash, id.number)).and_then(|v| match v {
PersistedEpoch::Genesis(ref epoch_0, _)
if id.position == EpochIdentifierPosition::Genesis0 =>
@@ -665,15 +549,6 @@ where
return Ok(Some(ViableEpochDescriptor::UnimportedGenesis(slot)))
}
if let Some(gap) = &self.gap {
if let Some((hash, number, hdr, position)) = gap.matches(slot) {
return Ok(Some(ViableEpochDescriptor::Signaled(
EpochIdentifier { position, hash, number },
hdr,
)))
}
}
// find_node_where will give you the node in the fork-tree which is an ancestor
// of the `parent_hash` by default. if the last epoch was signalled at the parent_hash,
// then it won't be returned. we need to create a new fake chain head hash which
@@ -744,28 +619,9 @@ where
) -> Result<(), fork_tree::Error<D::Error>> {
let is_descendent_of =
descendent_of_builder.build_is_descendent_of(Some((hash, parent_hash)));
let slot = epoch.as_ref().start_slot();
let IncrementedEpoch(mut epoch) = epoch;
let IncrementedEpoch(epoch) = epoch;
let header = PersistedEpochHeader::<E>::from(&epoch);
if let Some(gap) = &mut self.gap {
if let PersistedEpoch::Regular(e) = epoch {
epoch = match gap.import(slot, hash, number, e) {
Ok(()) => return Ok(()),
Err(e) => PersistedEpoch::Regular(e),
}
}
} else if epoch.is_genesis() &&
!self.epochs.is_empty() &&
!self.epochs.values().any(|e| e.is_genesis())
{
// There's a genesis epoch imported when we already have an active epoch.
// This happens after the warp sync as the ancient blocks download start.
// We need to start tracking gap epochs here.
self.gap = Some(GapEpochs { current: (hash, number, epoch), next: None });
return Ok(())
}
let res = self.inner.import(hash, number, header, &is_descendent_of);
match res {
@@ -1278,288 +1134,4 @@ mod tests {
list.sort();
assert_eq!(list, vec![b"A", b"G", b"L"]);
}
/// Test that ensures that the gap is not enabled when we import multiple genesis blocks.
#[test]
fn gap_is_not_enabled_when_multiple_genesis_epochs_are_imported() {
// X
// /
// 0 - A
//
let is_descendent_of = |base: &Hash, block: &Hash| -> Result<bool, TestError> {
match (base, *block) {
(b"0", _) => Ok(true),
_ => Ok(false),
}
};
let duration = 100;
let make_genesis = |slot| Epoch { start_slot: slot, duration };
let mut epoch_changes = EpochChanges::new();
let next_descriptor = ();
// insert genesis epoch for A
{
let genesis_epoch_a_descriptor = epoch_changes
.epoch_descriptor_for_child_of(&is_descendent_of, b"0", 0, 100)
.unwrap()
.unwrap();
let incremented_epoch = epoch_changes
.viable_epoch(&genesis_epoch_a_descriptor, &make_genesis)
.unwrap()
.increment(next_descriptor);
epoch_changes
.import(&is_descendent_of, *b"A", 1, *b"0", incremented_epoch)
.unwrap();
}
// insert genesis epoch for X
{
let genesis_epoch_x_descriptor = epoch_changes
.epoch_descriptor_for_child_of(&is_descendent_of, b"0", 0, 1000)
.unwrap()
.unwrap();
let incremented_epoch = epoch_changes
.viable_epoch(&genesis_epoch_x_descriptor, &make_genesis)
.unwrap()
.increment(next_descriptor);
epoch_changes
.import(&is_descendent_of, *b"X", 1, *b"0", incremented_epoch)
.unwrap();
}
// Clearing the gap should be a no-op.
epoch_changes.clear_gap();
// Check that both epochs are available.
let epoch_a = epoch_changes
.epoch_data_for_child_of(&is_descendent_of, b"A", 1, 101, &make_genesis)
.unwrap()
.unwrap();
let epoch_x = epoch_changes
.epoch_data_for_child_of(&is_descendent_of, b"X", 1, 1001, &make_genesis)
.unwrap()
.unwrap();
assert!(epoch_a != epoch_x)
}
#[test]
fn gap_epochs_advance() {
// 0 - 1 - 2 - 3 - .... 42 - 43
let is_descendent_of = |base: &Hash, block: &Hash| -> Result<bool, TestError> {
match (base, *block) {
(b"0", _) => Ok(true),
(b"1", b) => Ok(b == *b"0"),
(b"2", b) => Ok(b == *b"1"),
(b"3", b) => Ok(b == *b"2"),
_ => Ok(false),
}
};
let duration = 100;
let make_genesis = |slot| Epoch { start_slot: slot, duration };
let mut epoch_changes = EpochChanges::new();
let next_descriptor = ();
let epoch42 = Epoch { start_slot: 42, duration: 100 };
let epoch43 = Epoch { start_slot: 43, duration: 100 };
epoch_changes.reset(*b"0", *b"1", 4200, epoch42, epoch43);
assert!(epoch_changes.gap.is_none());
// Import a new genesis epoch, this should crate the gap.
let genesis_epoch_a_descriptor = epoch_changes
.epoch_descriptor_for_child_of(&is_descendent_of, b"0", 0, 100)
.unwrap()
.unwrap();
let incremented_epoch = epoch_changes
.viable_epoch(&genesis_epoch_a_descriptor, &make_genesis)
.unwrap()
.increment(next_descriptor);
epoch_changes
.import(&is_descendent_of, *b"1", 1, *b"0", incremented_epoch)
.unwrap();
assert!(epoch_changes.gap.is_some());
let genesis_epoch = epoch_changes
.epoch_descriptor_for_child_of(&is_descendent_of, b"0", 0, 100)
.unwrap()
.unwrap();
assert_eq!(genesis_epoch, ViableEpochDescriptor::UnimportedGenesis(100));
// Import more epochs and check that gap advances.
let import_epoch_1 =
epoch_changes.viable_epoch(&genesis_epoch, &make_genesis).unwrap().increment(());
let epoch_1 = import_epoch_1.as_ref().clone();
epoch_changes
.import(&is_descendent_of, *b"1", 1, *b"0", import_epoch_1)
.unwrap();
let genesis_epoch_data = epoch_changes.epoch_data(&genesis_epoch, &make_genesis).unwrap();
let end_slot = genesis_epoch_data.end_slot();
let x = epoch_changes
.epoch_data_for_child_of(&is_descendent_of, b"1", 1, end_slot, &make_genesis)
.unwrap()
.unwrap();
assert_eq!(x, epoch_1);
assert_eq!(epoch_changes.gap.as_ref().unwrap().current.0, *b"1");
assert!(epoch_changes.gap.as_ref().unwrap().next.is_none());
let epoch_1_desriptor = epoch_changes
.epoch_descriptor_for_child_of(&is_descendent_of, b"1", 1, end_slot)
.unwrap()
.unwrap();
let epoch_1 = epoch_changes.epoch_data(&epoch_1_desriptor, &make_genesis).unwrap();
let import_epoch_2 = epoch_changes
.viable_epoch(&epoch_1_desriptor, &make_genesis)
.unwrap()
.increment(());
let epoch_2 = import_epoch_2.as_ref().clone();
epoch_changes
.import(&is_descendent_of, *b"2", 2, *b"1", import_epoch_2)
.unwrap();
let end_slot = epoch_1.end_slot();
let x = epoch_changes
.epoch_data_for_child_of(&is_descendent_of, b"2", 2, end_slot, &make_genesis)
.unwrap()
.unwrap();
assert_eq!(epoch_changes.gap.as_ref().unwrap().current.0, *b"1");
assert_eq!(epoch_changes.gap.as_ref().unwrap().next.as_ref().unwrap().0, *b"2");
assert_eq!(x, epoch_2);
let epoch_2_desriptor = epoch_changes
.epoch_descriptor_for_child_of(&is_descendent_of, b"2", 2, end_slot)
.unwrap()
.unwrap();
let import_epoch_3 = epoch_changes
.viable_epoch(&epoch_2_desriptor, &make_genesis)
.unwrap()
.increment(());
epoch_changes
.import(&is_descendent_of, *b"3", 3, *b"2", import_epoch_3)
.unwrap();
assert_eq!(epoch_changes.gap.as_ref().unwrap().current.0, *b"2");
epoch_changes.clear_gap();
assert!(epoch_changes.gap.is_none());
}
/// Test that ensures that the gap is not enabled when there's still genesis
/// epochs imported, regardless of whether there are already other further
/// epochs imported descending from such genesis epochs.
#[test]
fn gap_is_not_enabled_when_at_least_one_genesis_epoch_is_still_imported() {
// A (#1) - B (#201)
// /
// 0 - C (#1)
//
// The epoch duration is 100 slots, each of these blocks represents
// an epoch change block. block B starts a new epoch at #201 since the
// genesis epoch spans two epochs.
let is_descendent_of = |base: &Hash, block: &Hash| -> Result<bool, TestError> {
match (base, block) {
(b"0", _) => Ok(true),
(b"A", b"B") => Ok(true),
_ => Ok(false),
}
};
let duration = 100;
let make_genesis = |slot| Epoch { start_slot: slot, duration };
let mut epoch_changes = EpochChanges::new();
let next_descriptor = ();
// insert genesis epoch for A at slot 1
{
let genesis_epoch_a_descriptor = epoch_changes
.epoch_descriptor_for_child_of(&is_descendent_of, b"0", 0, 1)
.unwrap()
.unwrap();
let incremented_epoch = epoch_changes
.viable_epoch(&genesis_epoch_a_descriptor, &make_genesis)
.unwrap()
.increment(next_descriptor);
epoch_changes
.import(&is_descendent_of, *b"A", 1, *b"0", incremented_epoch)
.unwrap();
}
// insert regular epoch for B at slot 201, descending from A
{
let epoch_b_descriptor = epoch_changes
.epoch_descriptor_for_child_of(&is_descendent_of, b"A", 1, 201)
.unwrap()
.unwrap();
let incremented_epoch = epoch_changes
.viable_epoch(&epoch_b_descriptor, &make_genesis)
.unwrap()
.increment(next_descriptor);
epoch_changes
.import(&is_descendent_of, *b"B", 201, *b"A", incremented_epoch)
.unwrap();
}
// insert genesis epoch for C at slot 1000
{
let genesis_epoch_x_descriptor = epoch_changes
.epoch_descriptor_for_child_of(&is_descendent_of, b"0", 0, 1000)
.unwrap()
.unwrap();
let incremented_epoch = epoch_changes
.viable_epoch(&genesis_epoch_x_descriptor, &make_genesis)
.unwrap()
.increment(next_descriptor);
epoch_changes
.import(&is_descendent_of, *b"C", 1, *b"0", incremented_epoch)
.unwrap();
}
// Clearing the gap should be a no-op.
epoch_changes.clear_gap();
// Check that all three epochs are available.
let epoch_a = epoch_changes
.epoch_data_for_child_of(&is_descendent_of, b"A", 1, 10, &make_genesis)
.unwrap()
.unwrap();
let epoch_b = epoch_changes
.epoch_data_for_child_of(&is_descendent_of, b"B", 201, 201, &make_genesis)
.unwrap()
.unwrap();
assert!(epoch_a != epoch_b);
// the genesis epoch A will span slots [1, 200] with epoch B starting at slot 201
assert_eq!(epoch_b.start_slot(), 201);
let epoch_c = epoch_changes
.epoch_data_for_child_of(&is_descendent_of, b"C", 1, 1001, &make_genesis)
.unwrap()
.unwrap();
assert!(epoch_a != epoch_c);
}
}
substrate/client/consensus/epochs/src/migration.rs
+2 -2
View File
@@ -64,7 +64,7 @@ where
header
});
EpochChanges { inner, epochs, gap: None }
EpochChanges { inner, epochs }
}
}
@@ -75,6 +75,6 @@ where
{
/// Migrate the type into current epoch changes definition.
pub fn migrate(self) -> EpochChanges<Hash, Number, E> {
EpochChanges { inner: self.inner, epochs: self.epochs, gap: None }
EpochChanges { inner: self.inner, epochs: self.epochs }
}
}