diff --git a/polkadot/cli/src/command.rs b/polkadot/cli/src/command.rs index 018400fbcf..9290ec584e 100644 --- a/polkadot/cli/src/command.rs +++ b/polkadot/cli/src/command.rs @@ -17,7 +17,7 @@ use crate::cli::{Cli, Subcommand, NODE_VERSION}; use frame_benchmarking_cli::{BenchmarkCmd, ExtrinsicFactory, SUBSTRATE_REFERENCE_HARDWARE}; use futures::future::TryFutureExt; -use log::{info, warn}; +use log::info; use sc_cli::SubstrateCli; use service::{ self, @@ -196,22 +196,7 @@ where let chain_spec = &runner.config().chain_spec; // By default, enable BEEFY on all networks, unless explicitly disabled through CLI. - let mut enable_beefy = !cli.run.no_beefy; - // BEEFY doesn't (yet) support warp sync: - // Until we implement https://github.com/paritytech/substrate/issues/14756 - // - disallow warp sync for validators, - // - disable BEEFY when warp sync for non-validators. - if enable_beefy && runner.config().network.sync_mode.is_warp() { - if runner.config().role.is_authority() { - return Err(Error::Other( - "Warp sync not supported for validator nodes running BEEFY.".into(), - )) - } else { - // disable BEEFY for non-validator nodes that are warp syncing - warn!("🥩 BEEFY not supported when warp syncing. Disabling BEEFY."); - enable_beefy = false; - } - } + let enable_beefy = !cli.run.no_beefy; set_default_ss58_version(chain_spec); diff --git a/prdoc/pr_2689.prdoc b/prdoc/pr_2689.prdoc new file mode 100644 index 0000000000..847c3e8026 --- /dev/null +++ b/prdoc/pr_2689.prdoc @@ -0,0 +1,13 @@ +# Schema: Parity PR Documentation Schema (prdoc) +# See doc at https://github.com/paritytech/prdoc + +title: BEEFY: Support compatibility with Warp Sync - Allow Warp Sync for Validators + +doc: + - audience: Node Operator + description: | + BEEFY can now sync itself even when using Warp Sync to sync the node. This removes the limitation of not + being able to run BEEFY when warp syncing. Validators are now again able to warp sync. + +crates: + - name: sc-consensus-beefy diff --git a/substrate/client/consensus/beefy/Cargo.toml b/substrate/client/consensus/beefy/Cargo.toml index 1736929e9b..1570dc744e 100644 --- a/substrate/client/consensus/beefy/Cargo.toml +++ b/substrate/client/consensus/beefy/Cargo.toml @@ -39,11 +39,12 @@ sp-core = { path = "../../../primitives/core" } sp-keystore = { path = "../../../primitives/keystore" } sp-mmr-primitives = { path = "../../../primitives/merkle-mountain-range" } sp-runtime = { path = "../../../primitives/runtime" } +tokio = "1.22.0" + [dev-dependencies] serde = "1.0.193" tempfile = "3.1.0" -tokio = "1.22.0" sc-block-builder = { path = "../../block-builder" } sc-network-test = { path = "../../network/test" } sp-consensus-grandpa = { path = "../../../primitives/consensus/grandpa" } diff --git a/substrate/client/consensus/beefy/src/communication/gossip.rs b/substrate/client/consensus/beefy/src/communication/gossip.rs index 342cd0511a..645a10b2a1 100644 --- a/substrate/client/consensus/beefy/src/communication/gossip.rs +++ b/substrate/client/consensus/beefy/src/communication/gossip.rs @@ -260,7 +260,11 @@ where /// /// Only votes for `set_id` and rounds `start <= round <= end` will be accepted. pub(crate) fn update_filter(&self, filter: GossipFilterCfg) { - debug!(target: LOG_TARGET, "🥩 New gossip filter {:?}", filter); + debug!( + target: LOG_TARGET, + "🥩 New gossip filter: start {:?}, end {:?}, validator set id {:?}", + filter.start, filter.end, filter.validator_set.id() + ); self.gossip_filter.write().update(filter); } diff --git a/substrate/client/consensus/beefy/src/import.rs b/substrate/client/consensus/beefy/src/import.rs index 5b2abb20ac..5bbf07fba7 100644 --- a/substrate/client/consensus/beefy/src/import.rs +++ b/substrate/client/consensus/beefy/src/import.rs @@ -142,6 +142,16 @@ where // Run inner block import. let inner_import_result = self.inner.import_block(block).await?; + match self.backend.state_at(hash) { + Ok(_) => {}, + Err(_) => { + // The block is imported as part of some chain sync. + // The voter doesn't need to process it now. + // It will be detected and processed as part of the voter state init. + return Ok(inner_import_result); + }, + } + match (beefy_encoded, &inner_import_result) { (Some(encoded), ImportResult::Imported(_)) => { match self.decode_and_verify(&encoded, number, hash) { diff --git a/substrate/client/consensus/beefy/src/lib.rs b/substrate/client/consensus/beefy/src/lib.rs index e6224cbf3e..51e82b6a81 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::{debug, error, info}; +use log::{debug, error, info, warn}; use parking_lot::Mutex; use prometheus::Registry; use sc_client_api::{Backend, BlockBackend, BlockchainEvents, FinalityNotifications, Finalizer}; @@ -56,6 +56,7 @@ use std::{ collections::{BTreeMap, VecDeque}, marker::PhantomData, sync::Arc, + time::Duration, }; mod aux_schema; @@ -78,6 +79,8 @@ mod tests; const LOG_TARGET: &str = "beefy"; +const HEADER_SYNC_DELAY: Duration = Duration::from_secs(60); + /// A convenience BEEFY client trait that defines all the type bounds a BEEFY client /// has to satisfy. Ideally that should actually be a trait alias. Unfortunately as /// of today, Rust does not allow a type alias to be used as a trait bound. Tracking @@ -292,21 +295,29 @@ pub async fn start_beefy_gadget( // select recoverable errors. loop { // Wait for BEEFY pallet to be active before starting voter. - let persisted_state = match wait_for_runtime_pallet( + let (beefy_genesis, best_grandpa) = match wait_for_runtime_pallet( &*runtime, &mut beefy_comms.gossip_engine, &mut finality_notifications, ) .await - .and_then(|(beefy_genesis, best_grandpa)| { - load_or_init_voter_state( - &*backend, - &*runtime, - beefy_genesis, - best_grandpa, - min_block_delta, - ) - }) { + { + Ok(res) => res, + Err(e) => { + error!(target: LOG_TARGET, "Error: {:?}. Terminating.", e); + return + }, + }; + + let persisted_state = match load_or_init_voter_state( + &*backend, + &*runtime, + beefy_genesis, + best_grandpa, + min_block_delta, + ) + .await + { Ok(state) => state, Err(e) => { error!(target: LOG_TARGET, "Error: {:?}. Terminating.", e); @@ -357,7 +368,7 @@ pub async fn start_beefy_gadget( } } -fn load_or_init_voter_state( +async fn load_or_init_voter_state( backend: &BE, runtime: &R, beefy_genesis: NumberFor, @@ -371,28 +382,70 @@ where R::Api: BeefyApi, { // Initialize voter state from AUX DB if compatible. - crate::aux_schema::load_persistent(backend)? + if let Some(mut state) = crate::aux_schema::load_persistent(backend)? // Verify state pallet genesis matches runtime. .filter(|state| state.pallet_genesis() == beefy_genesis) - .and_then(|mut state| { - // Overwrite persisted state with current best GRANDPA block. - state.set_best_grandpa(best_grandpa.clone()); - // Overwrite persisted data with newly provided `min_block_delta`. - state.set_min_block_delta(min_block_delta); - info!(target: LOG_TARGET, "🥩 Loading BEEFY voter state from db: {:?}.", state); - Some(Ok(state)) - }) - // No valid voter-state persisted, re-initialize from pallet genesis. - .unwrap_or_else(|| { - initialize_voter_state(backend, runtime, beefy_genesis, best_grandpa, min_block_delta) - }) + { + // Overwrite persisted state with current best GRANDPA block. + state.set_best_grandpa(best_grandpa.clone()); + // Overwrite persisted data with newly provided `min_block_delta`. + state.set_min_block_delta(min_block_delta); + info!(target: LOG_TARGET, "🥩 Loading BEEFY voter state from db: {:?}.", state); + + // Make sure that all the headers that we need have been synced. + let mut header = best_grandpa.clone(); + while *header.number() > state.best_beefy() { + header = + wait_for_parent_header(backend.blockchain(), header, HEADER_SYNC_DELAY).await?; + } + return Ok(state); + } + + // No valid voter-state persisted, re-initialize from pallet genesis. + initialize_voter_state(backend, runtime, beefy_genesis, best_grandpa, min_block_delta).await +} + +/// Waits until the parent header of `current` is available and returns it. +/// +/// When the node uses GRANDPA warp sync it initially downloads only the mandatory GRANDPA headers. +/// The rest of the headers (gap sync) are lazily downloaded later. But the BEEFY voter also needs +/// the headers in range `[beefy_genesis..=best_grandpa]` to be available. This helper method +/// enables us to wait until these headers have been synced. +async fn wait_for_parent_header( + blockchain: &BC, + current: ::Header, + delay: Duration, +) -> ClientResult<::Header> +where + B: Block, + BC: BlockchainBackend, +{ + if *current.number() == Zero::zero() { + let msg = format!("header {} is Genesis, there is no parent for it", current.hash()); + warn!(target: LOG_TARGET, "{}", msg); + return Err(ClientError::UnknownBlock(msg)) + } + loop { + match blockchain.header(*current.parent_hash())? { + Some(parent) => return Ok(parent), + None => { + info!( + target: LOG_TARGET, + "🥩 Parent of header number {} not found. \ + BEEFY gadget waiting for header sync to finish ...", + current.number() + ); + tokio::time::sleep(delay).await; + }, + } + } } // If no persisted state present, walk back the chain from first GRANDPA notification to either: // - latest BEEFY finalized block, or if none found on the way, // - BEEFY pallet genesis; // Enqueue any BEEFY mandatory blocks (session boundaries) found on the way, for voter to finalize. -fn initialize_voter_state( +async fn initialize_voter_state( backend: &BE, runtime: &R, beefy_genesis: NumberFor, @@ -405,6 +458,8 @@ where R: ProvideRuntimeApi, R::Api: BeefyApi, { + let blockchain = backend.blockchain(); + let beefy_genesis = runtime .runtime_api() .beefy_genesis(best_grandpa.hash()) @@ -414,7 +469,6 @@ where .ok_or_else(|| ClientError::Backend("BEEFY pallet expected to be active.".into()))?; // Walk back the imported blocks and initialize voter either, at the last block with // a BEEFY justification, or at pallet genesis block; voter will resume from there. - let blockchain = backend.blockchain(); let mut sessions = VecDeque::new(); let mut header = best_grandpa.clone(); let state = loop { @@ -432,7 +486,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)?; + let active_set = expect_validator_set(runtime, backend, &header).await?; let mut rounds = Rounds::new(best_beefy, active_set); // Mark the round as already finalized. rounds.conclude(best_beefy); @@ -451,7 +505,7 @@ where if *header.number() == beefy_genesis { // We've reached BEEFY genesis, initialize voter here. - let genesis_set = expect_validator_set(runtime, backend, &header)?; + let genesis_set = expect_validator_set(runtime, backend, &header).await?; info!( target: LOG_TARGET, "🥩 Loading BEEFY voter state from genesis on what appears to be first startup. \ @@ -481,7 +535,7 @@ where } // Move up the chain. - header = blockchain.expect_header(*header.parent_hash())?; + header = wait_for_parent_header(blockchain, header, HEADER_SYNC_DELAY).await?; }; aux_schema::write_current_version(backend)?; @@ -532,7 +586,12 @@ where Err(ClientError::Backend(err_msg)) } -fn expect_validator_set( +/// Provides validator set active `at_header`. It tries to get it from state, otherwise falls +/// back to walk up the chain looking the validator set enactment in header digests. +/// +/// Note: function will `async::sleep()` when walking back the chain if some needed header hasn't +/// been synced yet (as it happens when warp syncing when headers are synced in the background). +async fn expect_validator_set( runtime: &R, backend: &BE, at_header: &B::Header, @@ -550,15 +609,16 @@ where debug!(target: LOG_TARGET, "🥩 Trying to find validator set active at header: {:?}", at_header); let mut header = at_header.clone(); loop { + debug!(target: LOG_TARGET, "🥩 Looking for auth set change at block number: {:?}", *header.number()); if let Ok(Some(active)) = runtime.runtime_api().validator_set(header.hash()) { return Ok(active) } else { - debug!(target: LOG_TARGET, "🥩 Looking for auth set change at block number: {:?}", *header.number()); match worker::find_authorities_change::(&header) { Some(active) => return Ok(active), // Move up the chain. Ultimately we'll get it from chain genesis state, or error out - // here. - None => header = blockchain.expect_header(*header.parent_hash())?, + // there. + None => + header = wait_for_parent_header(blockchain, header, HEADER_SYNC_DELAY).await?, } } } diff --git a/substrate/client/consensus/beefy/src/round.rs b/substrate/client/consensus/beefy/src/round.rs index 6f400ce478..47414c60fd 100644 --- a/substrate/client/consensus/beefy/src/round.rs +++ b/substrate/client/consensus/beefy/src/round.rs @@ -19,7 +19,7 @@ use crate::LOG_TARGET; use codec::{Decode, Encode}; -use log::debug; +use log::{debug, info}; use sp_consensus_beefy::{ ecdsa_crypto::{AuthorityId, Signature}, Commitment, EquivocationProof, SignedCommitment, ValidatorSet, ValidatorSetId, VoteMessage, @@ -194,7 +194,11 @@ where self.previous_votes.retain(|&(_, number), _| number > round_num); self.mandatory_done = self.mandatory_done || round_num == self.session_start; self.best_done = self.best_done.max(Some(round_num)); - debug!(target: LOG_TARGET, "🥩 Concluded round #{}", round_num); + if round_num == self.session_start { + info!(target: LOG_TARGET, "🥩 Concluded mandatory round #{}", round_num); + } else { + debug!(target: LOG_TARGET, "🥩 Concluded optional round #{}", round_num); + } } } diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index 3f800166e2..1706543256 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -378,7 +378,7 @@ async fn voter_init_setup( ); let (beefy_genesis, best_grandpa) = wait_for_runtime_pallet(api, &mut gossip_engine, finality).await.unwrap(); - load_or_init_voter_state(&*backend, api, beefy_genesis, best_grandpa, 1) + load_or_init_voter_state(&*backend, api, beefy_genesis, best_grandpa, 1).await } // Spawns beefy voters. Returns a future to spawn on the runtime. @@ -1026,7 +1026,7 @@ async fn should_initialize_voter_at_genesis() { assert_eq!(rounds.validator_set_id(), validator_set.id()); // verify next vote target is mandatory block 1 - assert_eq!(persisted_state.best_beefy_block(), 0); + assert_eq!(persisted_state.best_beefy(), 0); assert_eq!(persisted_state.best_grandpa_number(), 13); assert_eq!(persisted_state.voting_oracle().voting_target(), Some(1)); @@ -1072,8 +1072,9 @@ async fn should_initialize_voter_at_custom_genesis() { ); let (beefy_genesis, best_grandpa) = wait_for_runtime_pallet(&api, &mut gossip_engine, &mut finality).await.unwrap(); - let persisted_state = - load_or_init_voter_state(&*backend, &api, beefy_genesis, best_grandpa, 1).unwrap(); + let persisted_state = load_or_init_voter_state(&*backend, &api, beefy_genesis, best_grandpa, 1) + .await + .unwrap(); // Test initialization at session boundary. // verify voter initialized with single session starting at block `custom_pallet_genesis` (7) @@ -1085,7 +1086,7 @@ async fn should_initialize_voter_at_custom_genesis() { assert_eq!(rounds.validator_set_id(), validator_set.id()); // verify next vote target is mandatory block 7 - assert_eq!(persisted_state.best_beefy_block(), 0); + assert_eq!(persisted_state.best_beefy(), 0); assert_eq!(persisted_state.best_grandpa_number(), 8); assert_eq!(persisted_state.voting_oracle().voting_target(), Some(custom_pallet_genesis)); @@ -1107,7 +1108,9 @@ async fn should_initialize_voter_at_custom_genesis() { let (beefy_genesis, best_grandpa) = wait_for_runtime_pallet(&api, &mut gossip_engine, &mut finality).await.unwrap(); let new_persisted_state = - load_or_init_voter_state(&*backend, &api, beefy_genesis, best_grandpa, 1).unwrap(); + load_or_init_voter_state(&*backend, &api, beefy_genesis, best_grandpa, 1) + .await + .unwrap(); // verify voter initialized with single session starting at block `new_pallet_genesis` (10) let sessions = new_persisted_state.voting_oracle().sessions(); @@ -1118,7 +1121,7 @@ async fn should_initialize_voter_at_custom_genesis() { assert_eq!(rounds.validator_set_id(), new_validator_set.id()); // verify next vote target is mandatory block 10 - assert_eq!(new_persisted_state.best_beefy_block(), 0); + assert_eq!(new_persisted_state.best_beefy(), 0); assert_eq!(new_persisted_state.best_grandpa_number(), 10); assert_eq!(new_persisted_state.voting_oracle().voting_target(), Some(new_pallet_genesis)); @@ -1171,7 +1174,7 @@ async fn should_initialize_voter_when_last_final_is_session_boundary() { assert_eq!(rounds.validator_set_id(), validator_set.id()); // verify block 10 is correctly marked as finalized - assert_eq!(persisted_state.best_beefy_block(), 10); + assert_eq!(persisted_state.best_beefy(), 10); assert_eq!(persisted_state.best_grandpa_number(), 13); // verify next vote target is diff-power-of-two block 12 assert_eq!(persisted_state.voting_oracle().voting_target(), Some(12)); @@ -1224,7 +1227,7 @@ async fn should_initialize_voter_at_latest_finalized() { assert_eq!(rounds.validator_set_id(), validator_set.id()); // verify next vote target is 13 - assert_eq!(persisted_state.best_beefy_block(), 12); + assert_eq!(persisted_state.best_beefy(), 12); assert_eq!(persisted_state.best_grandpa_number(), 13); assert_eq!(persisted_state.voting_oracle().voting_target(), Some(13)); @@ -1272,7 +1275,7 @@ async fn should_initialize_voter_at_custom_genesis_when_state_unavailable() { 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_beefy(), 0); assert_eq!(persisted_state.best_grandpa_number(), 30); assert_eq!(persisted_state.voting_oracle().voting_target(), Some(custom_pallet_genesis)); diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index da73a0d17d..26f940f05f 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -298,6 +298,10 @@ impl PersistedState { self.voting_oracle.min_block_delta = min_block_delta.max(1); } + pub fn best_beefy(&self) -> NumberFor { + self.voting_oracle.best_beefy_block + } + pub(crate) fn set_best_beefy(&mut self, best_beefy: NumberFor) { self.voting_oracle.best_beefy_block = best_beefy; } @@ -1094,10 +1098,6 @@ pub(crate) mod tests { self.voting_oracle.active_rounds() } - pub fn best_beefy_block(&self) -> NumberFor { - self.voting_oracle.best_beefy_block - } - pub fn best_grandpa_number(&self) -> NumberFor { *self.voting_oracle.best_grandpa_block_header.number() } @@ -1511,7 +1511,7 @@ pub(crate) mod tests { }; // no 'best beefy block' or finality proofs - assert_eq!(worker.persisted_state.best_beefy_block(), 0); + assert_eq!(worker.persisted_state.best_beefy(), 0); poll_fn(move |cx| { assert_eq!(best_block_stream.poll_next_unpin(cx), Poll::Pending); assert_eq!(finality_proof.poll_next_unpin(cx), Poll::Pending); @@ -1534,7 +1534,7 @@ pub(crate) mod tests { // try to finalize block #1 worker.finalize(justif.clone()).unwrap(); // verify block finalized - assert_eq!(worker.persisted_state.best_beefy_block(), 1); + assert_eq!(worker.persisted_state.best_beefy(), 1); poll_fn(move |cx| { // expect Some(hash-of-block-1) match best_block_stream.poll_next_unpin(cx) { @@ -1571,7 +1571,7 @@ pub(crate) mod tests { // new session starting at #2 is in front assert_eq!(worker.active_rounds().unwrap().session_start(), 2); // verify block finalized - assert_eq!(worker.persisted_state.best_beefy_block(), 2); + assert_eq!(worker.persisted_state.best_beefy(), 2); poll_fn(move |cx| { match best_block_stream.poll_next_unpin(cx) { // expect Some(hash-of-block-2)