From caa1f378d1fa075529641c63b6979f0cfdc6d72d Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Mon, 16 Oct 2023 16:40:54 +0300 Subject: [PATCH] sc-consensus-beefy: fix initialization when state is unavailable Fix situation where BEEFY initial validator set could not be determined. If state is unavailable at BEEFY genesis block to get initial validator set, get the info from header digests. For this, we need to walk back the chain starting from BEEFY genesis looking for the BEEFY digest announcing the active validator set for that respective session. This commit fixes a silly bug where walking back the chain was stopped when reaching BEEFY genesis block, which is incorrect when BEEFY genesis is not session boundary block. When BEEFY genesis is set to some random block within a session, we need to walk back to the start of the session to see the validator set announcement. Added regression test for this fix. Fixes https://github.com/paritytech/polkadot-sdk/issues/1885 --- substrate/client/consensus/beefy/src/lib.rs | 12 ++-- substrate/client/consensus/beefy/src/tests.rs | 56 +++++++++++++++++-- 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/substrate/client/consensus/beefy/src/lib.rs b/substrate/client/consensus/beefy/src/lib.rs index 72df3cab8550..89a5d51c8870 100644 --- a/substrate/client/consensus/beefy/src/lib.rs +++ b/substrate/client/consensus/beefy/src/lib.rs @@ -33,7 +33,7 @@ use crate::{ worker::PersistedState, }; use futures::{stream::Fuse, StreamExt}; -use log::{error, info}; +use log::{debug, error, info}; use parking_lot::Mutex; use prometheus::Registry; use sc_client_api::{Backend, BlockBackend, BlockchainEvents, FinalityNotifications, Finalizer}; @@ -428,7 +428,7 @@ where let best_beefy = *header.number(); // If no session boundaries detected so far, just initialize new rounds here. if sessions.is_empty() { - let active_set = expect_validator_set(runtime, backend, &header, beefy_genesis)?; + let active_set = expect_validator_set(runtime, backend, &header)?; let mut rounds = Rounds::new(best_beefy, active_set); // Mark the round as already finalized. rounds.conclude(best_beefy); @@ -447,7 +447,7 @@ where if *header.number() == beefy_genesis { // We've reached BEEFY genesis, initialize voter here. - let genesis_set = expect_validator_set(runtime, backend, &header, beefy_genesis)?; + let genesis_set = expect_validator_set(runtime, backend, &header)?; info!( target: LOG_TARGET, "🥩 Loading BEEFY voter state from genesis on what appears to be first startup. \ @@ -532,7 +532,6 @@ fn expect_validator_set( runtime: &R, backend: &BE, at_header: &B::Header, - beefy_genesis: NumberFor, ) -> ClientResult> where B: Block, @@ -540,6 +539,7 @@ where R: ProvideRuntimeApi, R::Api: BeefyApi, { + debug!(target: LOG_TARGET, "🥩 Try to find validator set active at header: {:?}", at_header); runtime .runtime_api() .validator_set(at_header.hash()) @@ -550,14 +550,14 @@ where // Digest emitted when validator set active 'at_header' was enacted. let blockchain = backend.blockchain(); let mut header = at_header.clone(); - while *header.number() >= beefy_genesis { + loop { + debug!(target: LOG_TARGET, "🥩 look for auth set change digest in header number: {:?}", *header.number()); match worker::find_authorities_change::(&header) { Some(active) => return Some(active), // Move up the chain. None => header = blockchain.expect_header(*header.parent_hash()).ok()?, } } - None }) .ok_or_else(|| ClientError::Backend("Could not find initial validator set".into())) } diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index 90b63c9cd446..902feca16398 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -247,7 +247,7 @@ impl TestNetFactory for BeefyTestNet { #[derive(Clone)] pub(crate) struct TestApi { pub beefy_genesis: u64, - pub validator_set: BeefyValidatorSet, + pub validator_set: Option, pub mmr_root_hash: MmrRootHash, pub reported_equivocations: Option, AuthorityId, Signature>>>>>, @@ -261,7 +261,7 @@ impl TestApi { ) -> Self { TestApi { beefy_genesis, - validator_set: validator_set.clone(), + validator_set: Some(validator_set.clone()), mmr_root_hash, reported_equivocations: None, } @@ -270,7 +270,7 @@ impl TestApi { pub fn with_validator_set(validator_set: &BeefyValidatorSet) -> Self { TestApi { beefy_genesis: 1, - validator_set: validator_set.clone(), + validator_set: Some(validator_set.clone()), mmr_root_hash: GOOD_MMR_ROOT, reported_equivocations: None, } @@ -300,7 +300,7 @@ sp_api::mock_impl_runtime_apis! { } fn validator_set() -> Option { - Some(self.inner.validator_set.clone()) + self.inner.validator_set.clone() } fn submit_report_equivocation_unsigned_extrinsic( @@ -1188,6 +1188,54 @@ async fn should_initialize_voter_at_latest_finalized() { assert_eq!(state, persisted_state); } +#[tokio::test] +async fn should_initialize_voter_at_custom_genesis_when_state_unavailable() { + let keys = &[BeefyKeyring::Alice]; + let validator_set = ValidatorSet::new(make_beefy_ids(keys), 0).unwrap(); + let mut net = BeefyTestNet::new(1); + let backend = net.peer(0).client().as_backend(); + // custom pallet genesis is block number 7 + let custom_pallet_genesis = 7; + let mut api = TestApi::new(custom_pallet_genesis, &validator_set, GOOD_MMR_ROOT); + // remove validator set from `TestApi`, practically simulating unavailable/pruned runtime state + api.validator_set = None; + + // push 30 blocks with `AuthorityChange` digests every 5 blocks + let hashes = net.generate_blocks_and_sync(30, 5, &validator_set, false).await; + let mut finality = net.peer(0).client().as_client().finality_notification_stream().fuse(); + // finalize 30 without justifications + net.peer(0).client().as_client().finalize_block(hashes[30], None).unwrap(); + + // load persistent state - nothing in DB, should init at genesis + let persisted_state = voter_init_setup(&mut net, &mut finality, &api).await.unwrap(); + + // Test initialization at session boundary. + // verify voter initialized with all sessions pending, first one starting at block 5 (start of + // session containing `custom_pallet_genesis`). + let sessions = persisted_state.voting_oracle().sessions(); + // should have enqueued 6 sessions (every 5 blocks from 5 to 30) + assert_eq!(sessions.len(), 6); + assert_eq!(sessions[0].session_start(), 7); + assert_eq!(sessions[1].session_start(), 10); + assert_eq!(sessions[2].session_start(), 15); + assert_eq!(sessions[3].session_start(), 20); + assert_eq!(sessions[4].session_start(), 25); + assert_eq!(sessions[5].session_start(), 30); + let rounds = persisted_state.active_round().unwrap(); + assert_eq!(rounds.session_start(), custom_pallet_genesis); + assert_eq!(rounds.validator_set_id(), validator_set.id()); + + // verify next vote target is mandatory block 7 (genesis) + assert_eq!(persisted_state.best_beefy_block(), 0); + assert_eq!(persisted_state.best_grandpa_number(), 30); + assert_eq!(persisted_state.voting_oracle().voting_target(), Some(custom_pallet_genesis)); + + // verify state also saved to db + assert!(verify_persisted_version(&*backend)); + let state = load_persistent(&*backend).unwrap().unwrap(); + assert_eq!(state, persisted_state); +} + #[tokio::test] async fn beefy_finalizing_after_pallet_genesis() { sp_tracing::try_init_simple();