From d35e6f6e89ea074812acb7c668f2cbc1ca4835fb Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 23 Jan 2024 16:55:13 +0100 Subject: [PATCH 01/15] Zombienet --- .../functional/0011-beefy-and-mmr.toml | 21 +++++++++++++++++++ .../functional/0011-beefy-and-mmr.zndsl | 14 +++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 polkadot/zombienet_tests/functional/0011-beefy-and-mmr.toml create mode 100644 polkadot/zombienet_tests/functional/0011-beefy-and-mmr.zndsl diff --git a/polkadot/zombienet_tests/functional/0011-beefy-and-mmr.toml b/polkadot/zombienet_tests/functional/0011-beefy-and-mmr.toml new file mode 100644 index 000000000000..6c9b54c0950a --- /dev/null +++ b/polkadot/zombienet_tests/functional/0011-beefy-and-mmr.toml @@ -0,0 +1,21 @@ +[settings] +timeout = 1000 + +[relaychain] +default_command = "/home/serban/workplace/sources/polkadot-sdk/target/release/polkadot" +chain = "rococo-local" + +[[relaychain.node_groups]] +name = "stable" +count = 3 +args = ["--log=beefy=debug", "--enable-offchain-indexing=true"] + +[[relaychain.node_groups]] +name = "unstable-1" +count = 5 +args = ["--log=beefy=debug", "--enable-offchain-indexing=true"] + +[[relaychain.node_groups]] +name = "unstable-2" +count = 3 +args = ["--log=beefy=debug", "--enable-offchain-indexing=true"] diff --git a/polkadot/zombienet_tests/functional/0011-beefy-and-mmr.zndsl b/polkadot/zombienet_tests/functional/0011-beefy-and-mmr.zndsl new file mode 100644 index 000000000000..8f0a39bf2894 --- /dev/null +++ b/polkadot/zombienet_tests/functional/0011-beefy-and-mmr.zndsl @@ -0,0 +1,14 @@ +Description: Test BEEFY voting and finality, test MMR proofs. Assumes Rococo sessions of 1 minute. +Network: ./0011-beefy-and-mmr.toml +Creds: config + +stable: reports finalised height is at least 9 within 120 seconds + +unstable-1: pause +unstable-2: pause + +sleep 10 seconds + +unstable-1: restart + +sleep 300 seconds From 16cb5f38b5c5b27d01b055e18f875c515e16ce4d Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 24 Jan 2024 12:22:29 +0100 Subject: [PATCH 02/15] cosmetics --- substrate/client/consensus/beefy/src/worker.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index 26f940f05f18..780598203c5b 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -236,12 +236,10 @@ impl VoterOracle { /// Return `Some(number)` if we should be voting on block `number`, /// return `None` if there is no block we should vote on. pub fn voting_target(&self) -> Option> { - let rounds = if let Some(r) = self.sessions.front() { - r - } else { + let rounds = self.sessions.front().or_else(|| { debug!(target: LOG_TARGET, "🥩 No voting round started"); return None - }; + })?; let best_grandpa = *self.best_grandpa_block_header.number(); let best_beefy = self.best_beefy_block; @@ -1028,7 +1026,7 @@ where /// Calculate next block number to vote on. /// -/// Return `None` if there is no voteable target yet. +/// Return `None` if there is no votable target yet. fn vote_target(best_grandpa: N, best_beefy: N, session_start: N, min_delta: u32) -> Option where N: AtLeast32Bit + Copy + Debug, From 274571808daa3e5c9e95ebc96bccd75ab86485ec Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 24 Jan 2024 20:44:02 +0100 Subject: [PATCH 03/15] Unify errors across BEEFY crate --- .../client/consensus/beefy/src/aux_schema.rs | 20 +++++++------- substrate/client/consensus/beefy/src/lib.rs | 26 ++++++++++++------- substrate/client/consensus/beefy/src/tests.rs | 3 ++- 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/substrate/client/consensus/beefy/src/aux_schema.rs b/substrate/client/consensus/beefy/src/aux_schema.rs index 409eb30d09ab..944a00f8372f 100644 --- a/substrate/client/consensus/beefy/src/aux_schema.rs +++ b/substrate/client/consensus/beefy/src/aux_schema.rs @@ -18,11 +18,10 @@ //! Schema for BEEFY state persisted in the aux-db. -use crate::{worker::PersistedState, LOG_TARGET}; +use crate::{error::Error, worker::PersistedState, LOG_TARGET}; use codec::{Decode, Encode}; use log::{info, trace}; use sc_client_api::{backend::AuxStore, Backend}; -use sp_blockchain::{Error as ClientError, Result as ClientResult}; use sp_runtime::traits::Block as BlockT; const VERSION_KEY: &[u8] = b"beefy_auxschema_version"; @@ -30,31 +29,33 @@ const WORKER_STATE_KEY: &[u8] = b"beefy_voter_state"; const CURRENT_VERSION: u32 = 4; -pub(crate) fn write_current_version(backend: &BE) -> ClientResult<()> { +pub(crate) fn write_current_version(backend: &BE) -> Result<(), Error> { info!(target: LOG_TARGET, "🥩 write aux schema version {:?}", CURRENT_VERSION); AuxStore::insert_aux(backend, &[(VERSION_KEY, CURRENT_VERSION.encode().as_slice())], &[]) + .map_err(|e| Error::Backend(e.to_string())) } /// Write voter state. pub(crate) fn write_voter_state( backend: &BE, state: &PersistedState, -) -> ClientResult<()> { +) -> Result<(), Error> { trace!(target: LOG_TARGET, "🥩 persisting {:?}", state); AuxStore::insert_aux(backend, &[(WORKER_STATE_KEY, state.encode().as_slice())], &[]) + .map_err(|e| Error::Backend(e.to_string())) } -fn load_decode(backend: &BE, key: &[u8]) -> ClientResult> { - match backend.get_aux(key)? { +fn load_decode(backend: &BE, key: &[u8]) -> Result, Error> { + match backend.get_aux(key).map_err(|e| Error::Backend(e.to_string()))? { None => Ok(None), Some(t) => T::decode(&mut &t[..]) - .map_err(|e| ClientError::Backend(format!("BEEFY DB is corrupted: {}", e))) + .map_err(|e| Error::Backend(format!("BEEFY DB is corrupted: {}", e))) .map(Some), } } /// Load or initialize persistent data from backend. -pub(crate) fn load_persistent(backend: &BE) -> ClientResult>> +pub(crate) fn load_persistent(backend: &BE) -> Result>, Error> where B: BlockT, BE: Backend, @@ -65,8 +66,7 @@ where None => (), Some(1) | Some(2) | Some(3) => (), // versions 1, 2 & 3 are obsolete and should be ignored Some(4) => return load_decode::<_, PersistedState>(backend, WORKER_STATE_KEY), - other => - return Err(ClientError::Backend(format!("Unsupported BEEFY DB version: {:?}", other))), + other => return Err(Error::Backend(format!("Unsupported BEEFY DB version: {:?}", other))), } // No persistent state found in DB. diff --git a/substrate/client/consensus/beefy/src/lib.rs b/substrate/client/consensus/beefy/src/lib.rs index 2e2e22288e3b..cc6e32241f27 100644 --- a/substrate/client/consensus/beefy/src/lib.rs +++ b/substrate/client/consensus/beefy/src/lib.rs @@ -27,6 +27,7 @@ use crate::{ outgoing_requests_engine::OnDemandJustificationsEngine, BeefyJustifsRequestHandler, }, }, + error::Error, import::BeefyBlockImport, metrics::register_metrics, round::Rounds, @@ -374,7 +375,7 @@ async fn load_or_init_voter_state( beefy_genesis: NumberFor, best_grandpa: ::Header, min_block_delta: u32, -) -> ClientResult> +) -> Result, Error> where B: Block, BE: Backend, @@ -415,7 +416,7 @@ async fn wait_for_parent_header( blockchain: &BC, current: ::Header, delay: Duration, -) -> ClientResult<::Header> +) -> Result<::Header, Error> where B: Block, BC: BlockchainBackend, @@ -423,10 +424,13 @@ where 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)) + return Err(Error::Backend(msg)); } loop { - match blockchain.header(*current.parent_hash())? { + match blockchain + .header(*current.parent_hash()) + .map_err(|e| Error::Backend(e.to_string()))? + { Some(parent) => return Ok(parent), None => { info!( @@ -451,7 +455,7 @@ async fn initialize_voter_state( beefy_genesis: NumberFor, best_grandpa: ::Header, min_block_delta: u32, -) -> ClientResult> +) -> Result, Error> where B: Block, BE: Backend, @@ -466,7 +470,7 @@ where .ok() .flatten() .filter(|genesis| *genesis == beefy_genesis) - .ok_or_else(|| ClientError::Backend("BEEFY pallet expected to be active.".into()))?; + .ok_or_else(|| Error::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 mut sessions = VecDeque::new(); @@ -499,7 +503,7 @@ where min_block_delta, beefy_genesis, ) - .ok_or_else(|| ClientError::Backend("Invalid BEEFY chain".into()))?; + .ok_or_else(|| Error::Backend("Invalid BEEFY chain".into()))?; break state } @@ -522,7 +526,7 @@ where min_block_delta, beefy_genesis, ) - .ok_or_else(|| ClientError::Backend("Invalid BEEFY chain".into()))? + .ok_or_else(|| Error::Backend("Invalid BEEFY chain".into()))? } if let Some(active) = worker::find_authorities_change::(&header) { @@ -595,7 +599,7 @@ async fn expect_validator_set( runtime: &R, backend: &BE, at_header: &B::Header, -) -> ClientResult> +) -> Result, Error> where B: Block, BE: Backend, @@ -618,7 +622,9 @@ where // Move up the chain. Ultimately we'll get it from chain genesis state, or error out // there. None => - header = wait_for_parent_header(blockchain, header, HEADER_SYNC_DELAY).await?, + header = wait_for_parent_header(blockchain, header, HEADER_SYNC_DELAY) + .await + .map_err(|e| Error::Backend(e.to_string()))?, } } } diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index 170654325642..54eb14987603 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -28,6 +28,7 @@ use crate::{ }, request_response::{on_demand_justifications_protocol_config, BeefyJustifsRequestHandler}, }, + error::Error, gossip_protocol_name, justification::*, load_or_init_voter_state, wait_for_runtime_pallet, BeefyRPCLinks, BeefyVoterLinks, KnownPeers, @@ -363,7 +364,7 @@ async fn voter_init_setup( net: &mut BeefyTestNet, finality: &mut futures::stream::Fuse>, api: &TestApi, -) -> sp_blockchain::Result> { +) -> Result, Error> { let backend = net.peer(0).client().as_backend(); let known_peers = Arc::new(Mutex::new(KnownPeers::new())); let (gossip_validator, _) = GossipValidator::new(known_peers); From 4d1e4e5e2d031bafc8443c04c3639735a60772fb Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Thu, 25 Jan 2024 11:02:16 +0100 Subject: [PATCH 04/15] Make metrics macros more generic --- .../incoming_requests_handler.rs | 4 +- .../outgoing_requests_engine.rs | 10 ++--- .../client/consensus/beefy/src/import.rs | 4 +- .../client/consensus/beefy/src/metrics.rs | 12 +++--- .../client/consensus/beefy/src/worker.rs | 42 ++++++++++--------- 5 files changed, 38 insertions(+), 34 deletions(-) diff --git a/substrate/client/consensus/beefy/src/communication/request_response/incoming_requests_handler.rs b/substrate/client/consensus/beefy/src/communication/request_response/incoming_requests_handler.rs index 71c5c49b3690..d856e9748a10 100644 --- a/substrate/client/consensus/beefy/src/communication/request_response/incoming_requests_handler.rs +++ b/substrate/client/consensus/beefy/src/communication/request_response/incoming_requests_handler.rs @@ -201,7 +201,7 @@ where let peer = request.peer; match self.handle_request(request) { Ok(()) => { - metric_inc!(self, beefy_successful_justification_responses); + metric_inc!(self.metrics, beefy_successful_justification_responses); debug!( target: BEEFY_SYNC_LOG_TARGET, "🥩 Handled BEEFY justification request from {:?}.", peer @@ -209,7 +209,7 @@ where }, Err(e) => { // peer reputation changes already applied in `self.handle_request()` - metric_inc!(self, beefy_failed_justification_responses); + metric_inc!(self.metrics, beefy_failed_justification_responses); debug!( target: BEEFY_SYNC_LOG_TARGET, "🥩 Failed to handle BEEFY justification request from {:?}: {}", peer, e, diff --git a/substrate/client/consensus/beefy/src/communication/request_response/outgoing_requests_engine.rs b/substrate/client/consensus/beefy/src/communication/request_response/outgoing_requests_engine.rs index 7121410ea109..992b9fa08c09 100644 --- a/substrate/client/consensus/beefy/src/communication/request_response/outgoing_requests_engine.rs +++ b/substrate/client/consensus/beefy/src/communication/request_response/outgoing_requests_engine.rs @@ -148,7 +148,7 @@ impl OnDemandJustificationsEngine { if let Some(peer) = self.try_next_peer() { self.request_from_peer(peer, RequestInfo { block, active_set }); } else { - metric_inc!(self, beefy_on_demand_justification_no_peer_to_request_from); + metric_inc!(self.metrics, beefy_on_demand_justification_no_peer_to_request_from); debug!( target: BEEFY_SYNC_LOG_TARGET, "🥩 no good peers to request justif #{:?} from", block @@ -194,13 +194,13 @@ impl OnDemandJustificationsEngine { ); match e { RequestFailure::Refused => { - metric_inc!(self, beefy_on_demand_justification_peer_refused); + metric_inc!(self.metrics, beefy_on_demand_justification_peer_refused); let peer_report = PeerReport { who: *peer, cost_benefit: cost::REFUSAL_RESPONSE }; Error::InvalidResponse(peer_report) }, _ => { - metric_inc!(self, beefy_on_demand_justification_peer_error); + metric_inc!(self.metrics, beefy_on_demand_justification_peer_error); Error::ResponseError }, } @@ -212,7 +212,7 @@ impl OnDemandJustificationsEngine { &req_info.active_set, ) .map_err(|(err, signatures_checked)| { - metric_inc!(self, beefy_on_demand_justification_invalid_proof); + metric_inc!(self.metrics, beefy_on_demand_justification_invalid_proof); debug!( target: BEEFY_SYNC_LOG_TARGET, "🥩 for on demand justification #{:?}, peer {:?} responded with invalid proof: {:?}", @@ -261,7 +261,7 @@ impl OnDemandJustificationsEngine { } }, Ok(proof) => { - metric_inc!(self, beefy_on_demand_justification_good_proof); + metric_inc!(self.metrics, beefy_on_demand_justification_good_proof); debug!( target: BEEFY_SYNC_LOG_TARGET, "🥩 received valid on-demand justif #{:?} from {:?}", block, peer diff --git a/substrate/client/consensus/beefy/src/import.rs b/substrate/client/consensus/beefy/src/import.rs index 6eced17b58ff..fc19ecc30142 100644 --- a/substrate/client/consensus/beefy/src/import.rs +++ b/substrate/client/consensus/beefy/src/import.rs @@ -165,7 +165,7 @@ where self.justification_sender .notify(|| Ok::<_, ()>(proof)) .expect("the closure always returns Ok; qed."); - metric_inc!(self, beefy_good_justification_imports); + metric_inc!(self.metrics, beefy_good_justification_imports); }, Err(err) => { debug!( @@ -174,7 +174,7 @@ where number, err, ); - metric_inc!(self, beefy_bad_justification_imports); + metric_inc!(self.metrics, beefy_bad_justification_imports); }, } }, diff --git a/substrate/client/consensus/beefy/src/metrics.rs b/substrate/client/consensus/beefy/src/metrics.rs index 031748bdceab..ef3928d79faa 100644 --- a/substrate/client/consensus/beefy/src/metrics.rs +++ b/substrate/client/consensus/beefy/src/metrics.rs @@ -305,10 +305,10 @@ pub(crate) fn register_metrics( // if expr does not derive `Display`. #[macro_export] macro_rules! metric_set { - ($self:ident, $m:ident, $v:expr) => {{ + ($metrics:expr, $m:ident, $v:expr) => {{ let val: u64 = format!("{}", $v).parse().unwrap(); - if let Some(metrics) = $self.metrics.as_ref() { + if let Some(metrics) = $metrics.as_ref() { metrics.$m.set(val); } }}; @@ -316,8 +316,8 @@ macro_rules! metric_set { #[macro_export] macro_rules! metric_inc { - ($self:ident, $m:ident) => {{ - if let Some(metrics) = $self.metrics.as_ref() { + ($metrics:expr, $m:ident) => {{ + if let Some(metrics) = $metrics.as_ref() { metrics.$m.inc(); } }}; @@ -325,8 +325,8 @@ macro_rules! metric_inc { #[macro_export] macro_rules! metric_get { - ($self:ident, $m:ident) => {{ - $self.metrics.as_ref().map(|metrics| metrics.$m.clone()) + ($metrics:expr, $m:ident) => {{ + $metrics.as_ref().map(|metrics| metrics.$m.clone()) }}; } diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index 780598203c5b..e356317b9845 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -392,7 +392,7 @@ where if store.intersection(&active).count() == 0 { let msg = "no authority public key found in store".to_string(); debug!(target: LOG_TARGET, "🥩 for block {:?} {}", block, msg); - metric_inc!(self, beefy_no_authority_found_in_store); + metric_inc!(self.metrics, beefy_no_authority_found_in_store); Err(Error::Keystore(msg)) } else { Ok(()) @@ -416,7 +416,7 @@ where validator_set.id(), active_session.validator_set_id(), ); - metric_inc!(self, beefy_lagging_sessions); + metric_inc!(self.metrics, beefy_lagging_sessions); } } @@ -429,7 +429,7 @@ where self.persisted_state .voting_oracle .add_session(Rounds::new(new_session_start, validator_set)); - metric_set!(self, beefy_validator_set_id, id); + metric_set!(self.metrics, beefy_validator_set_id, id); info!( target: LOG_TARGET, "🥩 New Rounds for validator set id: {:?} with session_start {:?}", @@ -517,7 +517,7 @@ where true, ); }, - RoundAction::Drop => metric_inc!(self, beefy_stale_votes), + RoundAction::Drop => metric_inc!(self.metrics, beefy_stale_votes), RoundAction::Enqueue => error!(target: LOG_TARGET, "🥩 unexpected vote: {:?}.", vote), }; Ok(()) @@ -537,23 +537,23 @@ where match self.voting_oracle().triage_round(block_num)? { RoundAction::Process => { debug!(target: LOG_TARGET, "🥩 Process justification for round: {:?}.", block_num); - metric_inc!(self, beefy_imported_justifications); + metric_inc!(self.metrics, beefy_imported_justifications); self.finalize(justification)? }, RoundAction::Enqueue => { debug!(target: LOG_TARGET, "🥩 Buffer justification for round: {:?}.", block_num); if self.pending_justifications.len() < MAX_BUFFERED_JUSTIFICATIONS { self.pending_justifications.entry(block_num).or_insert(justification); - metric_inc!(self, beefy_buffered_justifications); + metric_inc!(self.metrics, beefy_buffered_justifications); } else { - metric_inc!(self, beefy_buffered_justifications_dropped); + metric_inc!(self.metrics, beefy_buffered_justifications_dropped); warn!( target: LOG_TARGET, "🥩 Buffer justification dropped for round: {:?}.", block_num ); } }, - RoundAction::Drop => metric_inc!(self, beefy_stale_justifications), + RoundAction::Drop => metric_inc!(self.metrics, beefy_stale_justifications), }; Ok(()) } @@ -575,7 +575,7 @@ where // We created the `finality_proof` and know to be valid. // New state is persisted after finalization. self.finalize(finality_proof.clone())?; - metric_inc!(self, beefy_good_votes_processed); + metric_inc!(self.metrics, beefy_good_votes_processed); return Ok(Some(finality_proof)) }, VoteImportResult::Ok => { @@ -589,14 +589,14 @@ where crate::aux_schema::write_voter_state(&*self.backend, &self.persisted_state) .map_err(|e| Error::Backend(e.to_string()))?; } - metric_inc!(self, beefy_good_votes_processed); + metric_inc!(self.metrics, beefy_good_votes_processed); }, VoteImportResult::Equivocation(proof) => { - metric_inc!(self, beefy_equivocation_votes); + metric_inc!(self.metrics, beefy_equivocation_votes); self.report_equivocation(proof)?; }, - VoteImportResult::Invalid => metric_inc!(self, beefy_invalid_votes), - VoteImportResult::Stale => metric_inc!(self, beefy_stale_votes), + VoteImportResult::Invalid => metric_inc!(self.metrics, beefy_invalid_votes), + VoteImportResult::Stale => metric_inc!(self.metrics, beefy_stale_votes), }; Ok(None) } @@ -626,7 +626,7 @@ where crate::aux_schema::write_voter_state(&*self.backend, &self.persisted_state) .map_err(|e| Error::Backend(e.to_string()))?; - metric_set!(self, beefy_best_block, block_num); + metric_set!(self.metrics, beefy_best_block, block_num); self.comms.on_demand_justifications.cancel_requests_older_than(block_num); @@ -677,12 +677,16 @@ where for (num, justification) in justifs_to_process.into_iter() { debug!(target: LOG_TARGET, "🥩 Handle buffered justification for: {:?}.", num); - metric_inc!(self, beefy_imported_justifications); + metric_inc!(self.metrics, beefy_imported_justifications); if let Err(err) = self.finalize(justification) { error!(target: LOG_TARGET, "🥩 Error finalizing block: {}", err); } } - metric_set!(self, beefy_buffered_justifications, self.pending_justifications.len()); + metric_set!( + self.metrics, + beefy_buffered_justifications, + self.pending_justifications.len() + ); } Ok(()) } @@ -691,7 +695,7 @@ where fn try_to_vote(&mut self) -> Result<(), Error> { // Vote if there's now a new vote target. if let Some(target) = self.voting_oracle().voting_target() { - metric_set!(self, beefy_should_vote_on, target); + metric_set!(self.metrics, beefy_should_vote_on, target); if target > self.persisted_state.best_voted { self.do_vote(target)?; } @@ -781,7 +785,7 @@ where .gossip_engine .gossip_message(proofs_topic::(), encoded_proof, true); } else { - metric_inc!(self, beefy_votes_sent); + metric_inc!(self.metrics, beefy_votes_sent); debug!(target: LOG_TARGET, "🥩 Sent vote message: {:?}", vote); let encoded_vote = GossipMessage::::Vote(vote).encode(); self.comms.gossip_engine.gossip_message(votes_topic::(), encoded_vote, false); @@ -789,7 +793,7 @@ where // Persist state after vote to avoid double voting in case of voter restarts. self.persisted_state.best_voted = target_number; - metric_set!(self, beefy_best_voted, target_number); + metric_set!(self.metrics, beefy_best_voted, target_number); crate::aux_schema::write_voter_state(&*self.backend, &self.persisted_state) .map_err(|e| Error::Backend(e.to_string())) } From 839042bd526c0aff9317e82f262a44442daaf951 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Thu, 25 Jan 2024 13:24:15 +0100 Subject: [PATCH 05/15] Split BeefyWorker --- substrate/client/consensus/beefy/src/lib.rs | 13 +- .../client/consensus/beefy/src/worker.rs | 187 +++++++++++------- 2 files changed, 123 insertions(+), 77 deletions(-) diff --git a/substrate/client/consensus/beefy/src/lib.rs b/substrate/client/consensus/beefy/src/lib.rs index cc6e32241f27..f46b4109e5d0 100644 --- a/substrate/client/consensus/beefy/src/lib.rs +++ b/substrate/client/consensus/beefy/src/lib.rs @@ -310,6 +310,14 @@ pub async fn start_beefy_gadget( }, }; + let worker_base = worker::BeefyWorkerBase { + backend: backend.clone(), + runtime: runtime.clone(), + key_store: key_store.clone().into(), + metrics: metrics.clone(), + _phantom: Default::default(), + }; + let persisted_state = match load_or_init_voter_state( &*backend, &*runtime, @@ -335,14 +343,11 @@ pub async fn start_beefy_gadget( } let worker = worker::BeefyWorker { - backend: backend.clone(), + base: worker_base, payload_provider: payload_provider.clone(), - runtime: runtime.clone(), sync: sync.clone(), - key_store: key_store.clone().into(), comms: beefy_comms, links: links.clone(), - metrics: metrics.clone(), pending_justifications: BTreeMap::new(), persisted_state, }; diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index e356317b9845..7fa321af3373 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -53,6 +53,7 @@ use sp_runtime::{ use std::{ collections::{BTreeMap, BTreeSet, VecDeque}, fmt::Debug, + marker::PhantomData, sync::Arc, }; @@ -325,52 +326,25 @@ pub(crate) struct BeefyComms { pub on_demand_justifications: OnDemandJustificationsEngine, } -/// A BEEFY worker plays the BEEFY protocol -pub(crate) struct BeefyWorker { +pub(crate) struct BeefyWorkerBase { // utilities pub backend: Arc, - pub payload_provider: P, pub runtime: Arc, - pub sync: Arc, pub key_store: BeefyKeystore, - // communication (created once, but returned and reused if worker is restarted/reinitialized) - pub comms: BeefyComms, - - // channels - /// Links between the block importer, the background voter and the RPC layer. - pub links: BeefyVoterLinks, - - // voter state /// BEEFY client metrics. pub metrics: Option, - /// Buffer holding justifications for future processing. - pub pending_justifications: BTreeMap, BeefyVersionedFinalityProof>, - /// Persisted voter state. - pub persisted_state: PersistedState, + + pub _phantom: PhantomData, } -impl BeefyWorker +impl BeefyWorkerBase where B: Block + Codec, BE: Backend, - P: PayloadProvider, - S: SyncOracle, R: ProvideRuntimeApi, R::Api: BeefyApi, { - fn best_grandpa_block(&self) -> NumberFor { - *self.persisted_state.voting_oracle.best_grandpa_block_header.number() - } - - fn voting_oracle(&self) -> &VoterOracle { - &self.persisted_state.voting_oracle - } - - fn active_rounds(&mut self) -> Result<&Rounds, Error> { - self.persisted_state.voting_oracle.active_rounds() - } - /// Verify `active` validator set for `block` against the key store /// /// We want to make sure that we have _at least one_ key in our keystore that @@ -402,13 +376,14 @@ where /// Handle session changes by starting new voting round for mandatory blocks. fn init_session_at( &mut self, + persisted_state: &mut PersistedState, validator_set: ValidatorSet, new_session_start: NumberFor, ) { debug!(target: LOG_TARGET, "🥩 New active validator set: {:?}", validator_set); // BEEFY should finalize a mandatory block during each session. - if let Ok(active_session) = self.active_rounds() { + if let Ok(active_session) = persisted_state.voting_oracle.active_rounds() { if !active_session.mandatory_done() { debug!( target: LOG_TARGET, @@ -426,7 +401,7 @@ where } let id = validator_set.id(); - self.persisted_state + persisted_state .voting_oracle .add_session(Rounds::new(new_session_start, validator_set)); metric_set!(self.metrics, beefy_validator_set_id, id); @@ -437,6 +412,61 @@ where new_session_start ); } +} + +/// A BEEFY worker plays the BEEFY protocol +pub(crate) struct BeefyWorker { + pub base: BeefyWorkerBase, + + // utils + pub payload_provider: P, + pub sync: Arc, + + // communication (created once, but returned and reused if worker is restarted/reinitialized) + pub comms: BeefyComms, + + // channels + /// Links between the block importer, the background voter and the RPC layer. + pub links: BeefyVoterLinks, + + // voter state + /// Buffer holding justifications for future processing. + pub pending_justifications: BTreeMap, BeefyVersionedFinalityProof>, + /// Persisted voter state. + pub persisted_state: PersistedState, +} + +impl BeefyWorker +where + B: Block + Codec, + BE: Backend, + P: PayloadProvider, + S: SyncOracle, + R: ProvideRuntimeApi, + R::Api: BeefyApi, +{ + fn best_grandpa_block(&self) -> NumberFor { + *self.persisted_state.voting_oracle.best_grandpa_block_header.number() + } + + fn voting_oracle(&self) -> &VoterOracle { + &self.persisted_state.voting_oracle + } + + #[cfg(test)] + fn active_rounds(&mut self) -> Result<&Rounds, Error> { + self.persisted_state.voting_oracle.active_rounds() + } + + /// Handle session changes by starting new voting round for mandatory blocks. + fn init_session_at( + &mut self, + validator_set: ValidatorSet, + new_session_start: NumberFor, + ) { + self.base + .init_session_at(&mut self.persisted_state, validator_set, new_session_start); + } fn handle_finality_notification( &mut self, @@ -450,7 +480,8 @@ where ); let header = ¬ification.header; - self.runtime + self.base + .runtime .runtime_api() .beefy_genesis(header.hash()) .ok() @@ -464,7 +495,7 @@ where self.persisted_state.set_best_grandpa(header.clone()); // Check all (newly) finalized blocks for new session(s). - let backend = self.backend.clone(); + let backend = self.base.backend.clone(); for header in notification .tree_route .iter() @@ -483,7 +514,7 @@ where } if new_session_added { - crate::aux_schema::write_voter_state(&*self.backend, &self.persisted_state) + crate::aux_schema::write_voter_state(&*self.base.backend, &self.persisted_state) .map_err(|e| Error::Backend(e.to_string()))?; } @@ -517,7 +548,7 @@ where true, ); }, - RoundAction::Drop => metric_inc!(self.metrics, beefy_stale_votes), + RoundAction::Drop => metric_inc!(self.base.metrics, beefy_stale_votes), RoundAction::Enqueue => error!(target: LOG_TARGET, "🥩 unexpected vote: {:?}.", vote), }; Ok(()) @@ -537,23 +568,23 @@ where match self.voting_oracle().triage_round(block_num)? { RoundAction::Process => { debug!(target: LOG_TARGET, "🥩 Process justification for round: {:?}.", block_num); - metric_inc!(self.metrics, beefy_imported_justifications); + metric_inc!(self.base.metrics, beefy_imported_justifications); self.finalize(justification)? }, RoundAction::Enqueue => { debug!(target: LOG_TARGET, "🥩 Buffer justification for round: {:?}.", block_num); if self.pending_justifications.len() < MAX_BUFFERED_JUSTIFICATIONS { self.pending_justifications.entry(block_num).or_insert(justification); - metric_inc!(self.metrics, beefy_buffered_justifications); + metric_inc!(self.base.metrics, beefy_buffered_justifications); } else { - metric_inc!(self.metrics, beefy_buffered_justifications_dropped); + metric_inc!(self.base.metrics, beefy_buffered_justifications_dropped); warn!( target: LOG_TARGET, "🥩 Buffer justification dropped for round: {:?}.", block_num ); } }, - RoundAction::Drop => metric_inc!(self.metrics, beefy_stale_justifications), + RoundAction::Drop => metric_inc!(self.base.metrics, beefy_stale_justifications), }; Ok(()) } @@ -575,7 +606,7 @@ where // We created the `finality_proof` and know to be valid. // New state is persisted after finalization. self.finalize(finality_proof.clone())?; - metric_inc!(self.metrics, beefy_good_votes_processed); + metric_inc!(self.base.metrics, beefy_good_votes_processed); return Ok(Some(finality_proof)) }, VoteImportResult::Ok => { @@ -586,17 +617,20 @@ where .map(|(mandatory_num, _)| mandatory_num == block_number) .unwrap_or(false) { - crate::aux_schema::write_voter_state(&*self.backend, &self.persisted_state) - .map_err(|e| Error::Backend(e.to_string()))?; + crate::aux_schema::write_voter_state( + &*self.base.backend, + &self.persisted_state, + ) + .map_err(|e| Error::Backend(e.to_string()))?; } - metric_inc!(self.metrics, beefy_good_votes_processed); + metric_inc!(self.base.metrics, beefy_good_votes_processed); }, VoteImportResult::Equivocation(proof) => { - metric_inc!(self.metrics, beefy_equivocation_votes); + metric_inc!(self.base.metrics, beefy_equivocation_votes); self.report_equivocation(proof)?; }, - VoteImportResult::Invalid => metric_inc!(self.metrics, beefy_invalid_votes), - VoteImportResult::Stale => metric_inc!(self.metrics, beefy_stale_votes), + VoteImportResult::Invalid => metric_inc!(self.base.metrics, beefy_invalid_votes), + VoteImportResult::Stale => metric_inc!(self.base.metrics, beefy_stale_votes), }; Ok(None) } @@ -623,14 +657,15 @@ where // Set new best BEEFY block number. self.persisted_state.set_best_beefy(block_num); - crate::aux_schema::write_voter_state(&*self.backend, &self.persisted_state) + crate::aux_schema::write_voter_state(&*self.base.backend, &self.persisted_state) .map_err(|e| Error::Backend(e.to_string()))?; - metric_set!(self.metrics, beefy_best_block, block_num); + metric_set!(self.base.metrics, beefy_best_block, block_num); self.comms.on_demand_justifications.cancel_requests_older_than(block_num); if let Err(e) = self + .base .backend .blockchain() .expect_block_hash_from_id(&BlockId::Number(block_num)) @@ -640,7 +675,8 @@ where .notify(|| Ok::<_, ()>(hash)) .expect("forwards closure result; the closure always returns Ok; qed."); - self.backend + self.base + .backend .append_justification(hash, (BEEFY_ENGINE_ID, finality_proof.encode())) }) { debug!( @@ -677,13 +713,13 @@ where for (num, justification) in justifs_to_process.into_iter() { debug!(target: LOG_TARGET, "🥩 Handle buffered justification for: {:?}.", num); - metric_inc!(self.metrics, beefy_imported_justifications); + metric_inc!(self.base.metrics, beefy_imported_justifications); if let Err(err) = self.finalize(justification) { error!(target: LOG_TARGET, "🥩 Error finalizing block: {}", err); } } metric_set!( - self.metrics, + self.base.metrics, beefy_buffered_justifications, self.pending_justifications.len() ); @@ -695,7 +731,7 @@ where fn try_to_vote(&mut self) -> Result<(), Error> { // Vote if there's now a new vote target. if let Some(target) = self.voting_oracle().voting_target() { - metric_set!(self.metrics, beefy_should_vote_on, target); + metric_set!(self.base.metrics, beefy_should_vote_on, target); if target > self.persisted_state.best_voted { self.do_vote(target)?; } @@ -715,6 +751,7 @@ where self.persisted_state.voting_oracle.best_grandpa_block_header.clone() } else { let hash = self + .base .backend .blockchain() .expect_block_hash_from_id(&BlockId::Number(target_number)) @@ -726,7 +763,7 @@ where Error::Backend(err_msg) })?; - self.backend.blockchain().expect_header(hash).map_err(|err| { + self.base.backend.blockchain().expect_header(hash).map_err(|err| { let err_msg = format!( "Couldn't get header for block #{:?} ({:?}) (error: {:?}), skipping vote..", target_number, hash, err @@ -746,7 +783,7 @@ where let rounds = self.persisted_state.voting_oracle.active_rounds_mut()?; let (validators, validator_set_id) = (rounds.validators(), rounds.validator_set_id()); - let authority_id = if let Some(id) = self.key_store.authority_id(validators) { + let authority_id = if let Some(id) = self.base.key_store.authority_id(validators) { debug!(target: LOG_TARGET, "🥩 Local authority id: {:?}", id); id } else { @@ -760,7 +797,7 @@ where let commitment = Commitment { payload, block_number: target_number, validator_set_id }; let encoded_commitment = commitment.encode(); - let signature = match self.key_store.sign(&authority_id, &encoded_commitment) { + let signature = match self.base.key_store.sign(&authority_id, &encoded_commitment) { Ok(sig) => sig, Err(err) => { warn!(target: LOG_TARGET, "🥩 Error signing commitment: {:?}", err); @@ -785,7 +822,7 @@ where .gossip_engine .gossip_message(proofs_topic::(), encoded_proof, true); } else { - metric_inc!(self.metrics, beefy_votes_sent); + metric_inc!(self.base.metrics, beefy_votes_sent); debug!(target: LOG_TARGET, "🥩 Sent vote message: {:?}", vote); let encoded_vote = GossipMessage::::Vote(vote).encode(); self.comms.gossip_engine.gossip_message(votes_topic::(), encoded_vote, false); @@ -793,8 +830,8 @@ where // Persist state after vote to avoid double voting in case of voter restarts. self.persisted_state.best_voted = target_number; - metric_set!(self.metrics, beefy_best_voted, target_number); - crate::aux_schema::write_voter_state(&*self.backend, &self.persisted_state) + metric_set!(self.base.metrics, beefy_best_voted, target_number); + crate::aux_schema::write_voter_state(&*self.base.backend, &self.persisted_state) .map_err(|e| Error::Backend(e.to_string())) } @@ -968,7 +1005,7 @@ where if !check_equivocation_proof::<_, _, BeefySignatureHasher>(&proof) { debug!(target: LOG_TARGET, "🥩 Skip report for bad equivocation {:?}", proof); return Ok(()) - } else if let Some(local_id) = self.key_store.authority_id(validators) { + } else if let Some(local_id) = self.base.key_store.authority_id(validators) { if offender_id == local_id { debug!(target: LOG_TARGET, "🥩 Skip equivocation report for own equivocation"); return Ok(()) @@ -977,6 +1014,7 @@ where let number = *proof.round_number(); let hash = self + .base .backend .blockchain() .expect_block_hash_from_id(&BlockId::Number(number)) @@ -987,7 +1025,7 @@ where ); Error::Backend(err_msg) })?; - let runtime_api = self.runtime.runtime_api(); + let runtime_api = self.base.runtime.runtime_api(); // generate key ownership proof at that block let key_owner_proof = match runtime_api .generate_key_ownership_proof(hash, validator_set_id, offender_id) @@ -1004,7 +1042,7 @@ where }; // submit equivocation report at **best** block - let best_block_hash = self.backend.blockchain().info().best_hash; + let best_block_hash = self.base.backend.blockchain().info().best_hash; runtime_api .submit_report_equivocation_unsigned_extrinsic(best_block_hash, proof, key_owner_proof) .map_err(Error::RuntimeApi)?; @@ -1191,14 +1229,17 @@ pub(crate) mod tests { on_demand_justifications, }; BeefyWorker { - backend, + base: BeefyWorkerBase { + backend, + runtime: api, + key_store: Some(keystore).into(), + metrics, + _phantom: Default::default(), + }, payload_provider, - runtime: api, - key_store: Some(keystore).into(), + sync: Arc::new(sync), links, comms, - metrics, - sync: Arc::new(sync), pending_justifications: BTreeMap::new(), persisted_state, } @@ -1472,19 +1513,19 @@ pub(crate) mod tests { let mut worker = create_beefy_worker(net.peer(0), &keys[0], 1, validator_set.clone()); // keystore doesn't contain other keys than validators' - assert_eq!(worker.verify_validator_set(&1, &validator_set), Ok(())); + assert_eq!(worker.base.verify_validator_set(&1, &validator_set), Ok(())); // unknown `Bob` key let keys = &[Keyring::Bob]; let validator_set = ValidatorSet::new(make_beefy_ids(keys), 0).unwrap(); let err_msg = "no authority public key found in store".to_string(); let expected = Err(Error::Keystore(err_msg)); - assert_eq!(worker.verify_validator_set(&1, &validator_set), expected); + assert_eq!(worker.base.verify_validator_set(&1, &validator_set), expected); // worker has no keystore - worker.key_store = None.into(); + worker.base.key_store = None.into(); let expected_err = Err(Error::Keystore("no Keystore".into())); - assert_eq!(worker.verify_validator_set(&1, &validator_set), expected_err); + assert_eq!(worker.base.verify_validator_set(&1, &validator_set), expected_err); } #[tokio::test] @@ -1636,7 +1677,7 @@ pub(crate) mod tests { let mut net = BeefyTestNet::new(1); let mut worker = create_beefy_worker(net.peer(0), &keys[0], 1, validator_set.clone()); - worker.runtime = api_alice.clone(); + worker.base.runtime = api_alice.clone(); // let there be a block with num = 1: let _ = net.peer(0).push_blocks(1, false); From 3dc09b69af7c6e32ad6fb2d5bd08b630019ca4a2 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Thu, 25 Jan 2024 19:02:18 +0100 Subject: [PATCH 06/15] Move state init/load logic to worker --- substrate/client/consensus/beefy/src/lib.rs | 160 +----------------- substrate/client/consensus/beefy/src/tests.rs | 37 +++- .../client/consensus/beefy/src/worker.rs | 140 ++++++++++++++- 3 files changed, 171 insertions(+), 166 deletions(-) diff --git a/substrate/client/consensus/beefy/src/lib.rs b/substrate/client/consensus/beefy/src/lib.rs index f46b4109e5d0..ca9c1138f39e 100644 --- a/substrate/client/consensus/beefy/src/lib.rs +++ b/substrate/client/consensus/beefy/src/lib.rs @@ -30,8 +30,6 @@ use crate::{ error::Error, import::BeefyBlockImport, metrics::register_metrics, - round::Rounds, - worker::PersistedState, }; use futures::{stream::Fuse, StreamExt}; use log::{debug, error, info, warn}; @@ -48,17 +46,11 @@ use sp_blockchain::{ use sp_consensus::{Error as ConsensusError, SyncOracle}; use sp_consensus_beefy::{ ecdsa_crypto::AuthorityId, BeefyApi, MmrRootHash, PayloadProvider, ValidatorSet, - BEEFY_ENGINE_ID, }; use sp_keystore::KeystorePtr; use sp_mmr_primitives::MmrApi; use sp_runtime::traits::{Block, Header as HeaderT, NumberFor, Zero}; -use std::{ - collections::{BTreeMap, VecDeque}, - marker::PhantomData, - sync::Arc, - time::Duration, -}; +use std::{collections::BTreeMap, marker::PhantomData, sync::Arc, time::Duration}; mod aux_schema; mod error; @@ -318,14 +310,9 @@ pub async fn start_beefy_gadget( _phantom: Default::default(), }; - let persisted_state = match load_or_init_voter_state( - &*backend, - &*runtime, - beefy_genesis, - best_grandpa, - min_block_delta, - ) - .await + let persisted_state = match worker_base + .load_or_init_state(beefy_genesis, best_grandpa, min_block_delta) + .await { Ok(state) => state, Err(e) => { @@ -374,43 +361,6 @@ pub async fn start_beefy_gadget( } } -async fn load_or_init_voter_state( - backend: &BE, - runtime: &R, - beefy_genesis: NumberFor, - best_grandpa: ::Header, - min_block_delta: u32, -) -> Result, Error> -where - B: Block, - BE: Backend, - R: ProvideRuntimeApi, - R::Api: BeefyApi, -{ - // Initialize voter state from AUX DB if compatible. - if let Some(mut state) = crate::aux_schema::load_persistent(backend)? - // Verify state pallet genesis matches runtime. - .filter(|state| state.pallet_genesis() == beefy_genesis) - { - // 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. @@ -450,108 +400,6 @@ where } } -// 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. -async fn initialize_voter_state( - backend: &BE, - runtime: &R, - beefy_genesis: NumberFor, - best_grandpa: ::Header, - min_block_delta: u32, -) -> Result, Error> -where - B: Block, - BE: Backend, - R: ProvideRuntimeApi, - R::Api: BeefyApi, -{ - let blockchain = backend.blockchain(); - - let beefy_genesis = runtime - .runtime_api() - .beefy_genesis(best_grandpa.hash()) - .ok() - .flatten() - .filter(|genesis| *genesis == beefy_genesis) - .ok_or_else(|| Error::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 mut sessions = VecDeque::new(); - let mut header = best_grandpa.clone(); - let state = loop { - if let Some(true) = blockchain - .justifications(header.hash()) - .ok() - .flatten() - .map(|justifs| justifs.get(BEEFY_ENGINE_ID).is_some()) - { - info!( - target: LOG_TARGET, - "🥩 Initialize BEEFY voter at last BEEFY finalized block: {:?}.", - *header.number() - ); - 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).await?; - let mut rounds = Rounds::new(best_beefy, active_set); - // Mark the round as already finalized. - rounds.conclude(best_beefy); - sessions.push_front(rounds); - } - let state = PersistedState::checked_new( - best_grandpa, - best_beefy, - sessions, - min_block_delta, - beefy_genesis, - ) - .ok_or_else(|| Error::Backend("Invalid BEEFY chain".into()))?; - break state - } - - if *header.number() == beefy_genesis { - // We've reached BEEFY genesis, initialize voter here. - 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. \ - Starting voting rounds at block {:?}, genesis validator set {:?}.", - beefy_genesis, - genesis_set, - ); - - sessions.push_front(Rounds::new(beefy_genesis, genesis_set)); - break PersistedState::checked_new( - best_grandpa, - Zero::zero(), - sessions, - min_block_delta, - beefy_genesis, - ) - .ok_or_else(|| Error::Backend("Invalid BEEFY chain".into()))? - } - - if let Some(active) = worker::find_authorities_change::(&header) { - info!( - target: LOG_TARGET, - "🥩 Marking block {:?} as BEEFY Mandatory.", - *header.number() - ); - sessions.push_front(Rounds::new(*header.number(), active)); - } - - // Move up the chain. - header = wait_for_parent_header(blockchain, header, HEADER_SYNC_DELAY).await?; - }; - - aux_schema::write_current_version(backend)?; - aux_schema::write_voter_state(backend, &state)?; - Ok(state) -} - /// Wait for BEEFY runtime pallet to be available, return active validator set. /// Should be called only once during worker initialization. async fn wait_for_runtime_pallet( diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index 54eb14987603..14d47a6c851f 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -31,8 +31,9 @@ use crate::{ error::Error, gossip_protocol_name, justification::*, - load_or_init_voter_state, wait_for_runtime_pallet, BeefyRPCLinks, BeefyVoterLinks, KnownPeers, - PersistedState, + wait_for_runtime_pallet, + worker::{BeefyWorkerBase, PersistedState}, + BeefyRPCLinks, BeefyVoterLinks, KnownPeers, }; use futures::{future, stream::FuturesUnordered, Future, FutureExt, StreamExt}; use parking_lot::Mutex; @@ -379,7 +380,14 @@ 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).await + let worker_base = BeefyWorkerBase { + backend, + runtime: Arc::new(api.clone()), + key_store: None.into(), + metrics: None, + _phantom: Default::default(), + }; + worker_base.load_or_init_state(beefy_genesis, best_grandpa, 1).await } // Spawns beefy voters. Returns a future to spawn on the runtime. @@ -1073,9 +1081,15 @@ 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) - .await - .unwrap(); + let worker_base = BeefyWorkerBase { + backend: backend.clone(), + runtime: Arc::new(api), + key_store: None.into(), + metrics: None, + _phantom: Default::default(), + }; + let persisted_state = + worker_base.load_or_init_state(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) @@ -1108,10 +1122,15 @@ async fn should_initialize_voter_at_custom_genesis() { // the network state persists and uses the old `GossipEngine` initialized for `peer(0)` let (beefy_genesis, best_grandpa) = wait_for_runtime_pallet(&api, &mut gossip_engine, &mut finality).await.unwrap(); + let worker_base = BeefyWorkerBase { + backend: backend.clone(), + runtime: Arc::new(api), + key_store: None.into(), + metrics: None, + _phantom: Default::default(), + }; let new_persisted_state = - load_or_init_voter_state(&*backend, &api, beefy_genesis, best_grandpa, 1) - .await - .unwrap(); + worker_base.load_or_init_state(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(); diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index 7fa321af3373..9cf99ecb6ed5 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -17,18 +17,20 @@ // along with this program. If not, see . use crate::{ + aux_schema, communication::{ gossip::{proofs_topic, votes_topic, GossipFilterCfg, GossipMessage, GossipValidator}, peers::PeerReport, request_response::outgoing_requests_engine::{OnDemandJustificationsEngine, ResponseInfo}, }, error::Error, + expect_validator_set, justification::BeefyVersionedFinalityProof, keystore::{BeefyKeystore, BeefySignatureHasher}, metric_inc, metric_set, metrics::VoterMetrics, round::{Rounds, VoteImportResult}, - BeefyVoterLinks, LOG_TARGET, + wait_for_parent_header, BeefyVoterLinks, HEADER_SYNC_DELAY, LOG_TARGET, }; use codec::{Codec, Decode, DecodeAll, Encode}; use futures::{stream::Fuse, FutureExt, StreamExt}; @@ -38,6 +40,7 @@ use sc_network_gossip::GossipEngine; use sc_utils::{mpsc::TracingUnboundedReceiver, notification::NotificationReceiver}; use sp_api::ProvideRuntimeApi; use sp_arithmetic::traits::{AtLeast32Bit, Saturating}; +use sp_blockchain::Backend as BlockchainBackend; use sp_consensus::SyncOracle; use sp_consensus_beefy::{ check_equivocation_proof, @@ -345,6 +348,141 @@ where R: ProvideRuntimeApi, R::Api: BeefyApi, { + // 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. + async fn init_state( + &self, + beefy_genesis: NumberFor, + best_grandpa: ::Header, + min_block_delta: u32, + ) -> Result, Error> { + let blockchain = self.backend.blockchain(); + + let beefy_genesis = self + .runtime + .runtime_api() + .beefy_genesis(best_grandpa.hash()) + .ok() + .flatten() + .filter(|genesis| { + // test + *genesis == beefy_genesis + }) + .ok_or_else(|| Error::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 mut sessions = VecDeque::new(); + let mut header = best_grandpa.clone(); + let state = loop { + if let Some(true) = blockchain + .justifications(header.hash()) + .ok() + .flatten() + .map(|justifs| justifs.get(BEEFY_ENGINE_ID).is_some()) + { + info!( + target: LOG_TARGET, + "🥩 Initialize BEEFY voter at last BEEFY finalized block: {:?}.", + *header.number() + ); + 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(self.runtime.as_ref(), self.backend.as_ref(), &header) + .await?; + let mut rounds = Rounds::new(best_beefy, active_set); + // Mark the round as already finalized. + rounds.conclude(best_beefy); + sessions.push_front(rounds); + } + let state = PersistedState::checked_new( + best_grandpa, + best_beefy, + sessions, + min_block_delta, + beefy_genesis, + ) + .ok_or_else(|| Error::Backend("Invalid BEEFY chain".into()))?; + break state + } + + if *header.number() == beefy_genesis { + // We've reached BEEFY genesis, initialize voter here. + let genesis_set = + expect_validator_set(self.runtime.as_ref(), self.backend.as_ref(), &header) + .await?; + info!( + target: LOG_TARGET, + "🥩 Loading BEEFY voter state from genesis on what appears to be first startup. \ + Starting voting rounds at block {:?}, genesis validator set {:?}.", + beefy_genesis, + genesis_set, + ); + + sessions.push_front(Rounds::new(beefy_genesis, genesis_set)); + break PersistedState::checked_new( + best_grandpa, + Zero::zero(), + sessions, + min_block_delta, + beefy_genesis, + ) + .ok_or_else(|| Error::Backend("Invalid BEEFY chain".into()))? + } + + if let Some(active) = find_authorities_change::(&header) { + info!( + target: LOG_TARGET, + "🥩 Marking block {:?} as BEEFY Mandatory.", + *header.number() + ); + sessions.push_front(Rounds::new(*header.number(), active)); + } + + // Move up the chain. + header = wait_for_parent_header(blockchain, header, HEADER_SYNC_DELAY).await?; + }; + + aux_schema::write_current_version(self.backend.as_ref())?; + aux_schema::write_voter_state(self.backend.as_ref(), &state)?; + Ok(state) + } + + pub async fn load_or_init_state( + &self, + beefy_genesis: NumberFor, + best_grandpa: ::Header, + min_block_delta: u32, + ) -> Result, Error> { + // Initialize voter state from AUX DB if compatible. + if let Some(mut state) = crate::aux_schema::load_persistent(self.backend.as_ref())? + // Verify state pallet genesis matches runtime. + .filter(|state| state.pallet_genesis() == beefy_genesis) + { + // 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(self.backend.blockchain(), header, HEADER_SYNC_DELAY) + .await?; + } + return Ok(state) + } + + // No valid voter-state persisted, re-initialize from pallet genesis. + self.init_state(beefy_genesis, best_grandpa, min_block_delta).await + } + /// Verify `active` validator set for `block` against the key store /// /// We want to make sure that we have _at least one_ key in our keystore that From b7f7bd3ea383eae09a184ce4033f70868de28fb2 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 24 Jan 2024 11:52:47 +0100 Subject: [PATCH 07/15] Avoid missing BEEFY voting sessions during node restart --- substrate/client/consensus/beefy/src/lib.rs | 2 +- substrate/client/consensus/beefy/src/tests.rs | 6 +++--- substrate/client/consensus/beefy/src/worker.rs | 16 +++++++++++++++- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/substrate/client/consensus/beefy/src/lib.rs b/substrate/client/consensus/beefy/src/lib.rs index ca9c1138f39e..1f10d8099d83 100644 --- a/substrate/client/consensus/beefy/src/lib.rs +++ b/substrate/client/consensus/beefy/src/lib.rs @@ -302,7 +302,7 @@ pub async fn start_beefy_gadget( }, }; - let worker_base = worker::BeefyWorkerBase { + let mut worker_base = worker::BeefyWorkerBase { backend: backend.clone(), runtime: runtime.clone(), key_store: key_store.clone().into(), diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index 14d47a6c851f..d0cdf8bfe36b 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -380,7 +380,7 @@ async fn voter_init_setup( ); let (beefy_genesis, best_grandpa) = wait_for_runtime_pallet(api, &mut gossip_engine, finality).await.unwrap(); - let worker_base = BeefyWorkerBase { + let mut worker_base = BeefyWorkerBase { backend, runtime: Arc::new(api.clone()), key_store: None.into(), @@ -1081,7 +1081,7 @@ 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 worker_base = BeefyWorkerBase { + let mut worker_base = BeefyWorkerBase { backend: backend.clone(), runtime: Arc::new(api), key_store: None.into(), @@ -1122,7 +1122,7 @@ async fn should_initialize_voter_at_custom_genesis() { // the network state persists and uses the old `GossipEngine` initialized for `peer(0)` let (beefy_genesis, best_grandpa) = wait_for_runtime_pallet(&api, &mut gossip_engine, &mut finality).await.unwrap(); - let worker_base = BeefyWorkerBase { + let mut worker_base = BeefyWorkerBase { backend: backend.clone(), runtime: Arc::new(api), key_store: None.into(), diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index 9cf99ecb6ed5..a2441a96f93b 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -453,7 +453,7 @@ where } pub async fn load_or_init_state( - &self, + &mut self, beefy_genesis: NumberFor, best_grandpa: ::Header, min_block_delta: u32, @@ -470,12 +470,26 @@ where 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 new_sessions = vec![]; let mut header = best_grandpa.clone(); while *header.number() > state.best_beefy() { + if let Some(active) = find_authorities_change::(&header) { + new_sessions.push((active, *header.number())); + } header = wait_for_parent_header(self.backend.blockchain(), header, HEADER_SYNC_DELAY) .await?; } + + // Make sure we didn't miss any sessions during node restart. + for (validator_set, new_session_start) in new_sessions.drain(..).rev() { + info!( + target: LOG_TARGET, + "🥩 Handling missed BEEFY session after node restart: {:?}.", + new_session_start + ); + self.init_session_at(&mut state, validator_set, new_session_start); + } return Ok(state) } From b3e7fe24702bdac662ed6e3ce38c3c565957af24 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Fri, 26 Jan 2024 11:08:05 +0100 Subject: [PATCH 08/15] Revert "Zombienet" This reverts commit b89531c56092308de216533384267a49a65e5a66. --- .../functional/0011-beefy-and-mmr.toml | 21 ------------------- .../functional/0011-beefy-and-mmr.zndsl | 14 ------------- 2 files changed, 35 deletions(-) delete mode 100644 polkadot/zombienet_tests/functional/0011-beefy-and-mmr.toml delete mode 100644 polkadot/zombienet_tests/functional/0011-beefy-and-mmr.zndsl diff --git a/polkadot/zombienet_tests/functional/0011-beefy-and-mmr.toml b/polkadot/zombienet_tests/functional/0011-beefy-and-mmr.toml deleted file mode 100644 index 6c9b54c0950a..000000000000 --- a/polkadot/zombienet_tests/functional/0011-beefy-and-mmr.toml +++ /dev/null @@ -1,21 +0,0 @@ -[settings] -timeout = 1000 - -[relaychain] -default_command = "/home/serban/workplace/sources/polkadot-sdk/target/release/polkadot" -chain = "rococo-local" - -[[relaychain.node_groups]] -name = "stable" -count = 3 -args = ["--log=beefy=debug", "--enable-offchain-indexing=true"] - -[[relaychain.node_groups]] -name = "unstable-1" -count = 5 -args = ["--log=beefy=debug", "--enable-offchain-indexing=true"] - -[[relaychain.node_groups]] -name = "unstable-2" -count = 3 -args = ["--log=beefy=debug", "--enable-offchain-indexing=true"] diff --git a/polkadot/zombienet_tests/functional/0011-beefy-and-mmr.zndsl b/polkadot/zombienet_tests/functional/0011-beefy-and-mmr.zndsl deleted file mode 100644 index 8f0a39bf2894..000000000000 --- a/polkadot/zombienet_tests/functional/0011-beefy-and-mmr.zndsl +++ /dev/null @@ -1,14 +0,0 @@ -Description: Test BEEFY voting and finality, test MMR proofs. Assumes Rococo sessions of 1 minute. -Network: ./0011-beefy-and-mmr.toml -Creds: config - -stable: reports finalised height is at least 9 within 120 seconds - -unstable-1: pause -unstable-2: pause - -sleep 10 seconds - -unstable-1: restart - -sleep 300 seconds From 7ac8282288100dde4f2b36f2d1a36cdab1bd0899 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Fri, 26 Jan 2024 11:31:53 +0100 Subject: [PATCH 09/15] Fix toml files --- Cargo.toml | 2 +- polkadot/node/subsystem-bench/Cargo.toml | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1edc64217fdf..fd8708a3dadd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -159,7 +159,6 @@ members = [ "polkadot/node/gum/proc-macro", "polkadot/node/jaeger", "polkadot/node/malus", - "polkadot/node/subsystem-bench", "polkadot/node/metrics", "polkadot/node/network/approval-distribution", "polkadot/node/network/availability-distribution", @@ -176,6 +175,7 @@ members = [ "polkadot/node/service", "polkadot/node/subsystem", "polkadot/node/subsystem-bench", + "polkadot/node/subsystem-bench", "polkadot/node/subsystem-test-helpers", "polkadot/node/subsystem-types", "polkadot/node/subsystem-util", diff --git a/polkadot/node/subsystem-bench/Cargo.toml b/polkadot/node/subsystem-bench/Cargo.toml index 40702411d8b2..11f53c6e8d1f 100644 --- a/polkadot/node/subsystem-bench/Cargo.toml +++ b/polkadot/node/subsystem-bench/Cargo.toml @@ -22,13 +22,13 @@ polkadot-node-subsystem-types = { path = "../subsystem-types" } polkadot-node-primitives = { path = "../primitives" } polkadot-primitives = { path = "../../primitives" } polkadot-node-network-protocol = { path = "../network/protocol" } -polkadot-availability-recovery = { path = "../network/availability-recovery", features=["subsystem-benchmarks"]} -polkadot-availability-distribution = { path = "../network/availability-distribution"} -polkadot-node-core-av-store = { path = "../core/av-store"} -polkadot-node-core-chain-api = { path = "../core/chain-api"} -polkadot-availability-bitfield-distribution = { path = "../network/bitfield-distribution"} +polkadot-availability-recovery = { path = "../network/availability-recovery", features = ["subsystem-benchmarks"] } +polkadot-availability-distribution = { path = "../network/availability-distribution" } +polkadot-node-core-av-store = { path = "../core/av-store" } +polkadot-node-core-chain-api = { path = "../core/chain-api" } +polkadot-availability-bitfield-distribution = { path = "../network/bitfield-distribution" } color-eyre = { version = "0.6.1", default-features = false } -polkadot-overseer = { path = "../overseer" } +polkadot-overseer = { path = "../overseer" } colored = "2.0.4" assert_matches = "1.5" async-trait = "0.1.57" @@ -45,10 +45,10 @@ env_logger = "0.9.0" rand = "0.8.5" # `rand` only supports uniform distribution, we need normal distribution for latency. rand_distr = "0.4.3" -bitvec="1.0.1" +bitvec = "1.0.1" kvdb-memorydb = "0.13.0" -parity-scale-codec = { version = "3.6.1", features = ["std", "derive"] } +parity-scale-codec = { version = "3.6.1", features = ["derive", "std"] } tokio = "1.24.2" clap-num = "1.0.2" polkadot-node-subsystem-test-helpers = { path = "../subsystem-test-helpers" } From f73aa1b014e6aa41cb201a36d0d372520429fc2b Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 30 Jan 2024 17:03:20 +0100 Subject: [PATCH 10/15] Fix code review comments --- .../client/consensus/beefy/src/worker.rs | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index a2441a96f93b..e67e3e0f76ad 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -180,6 +180,13 @@ impl VoterOracle { } } + // Check if an observed session can be added to the Oracle. + fn can_add_session(&self, session_start: NumberFor) -> bool { + let latest_known_session_start = + self.sessions.back().map(|session| session.session_start()); + Some(session_start) > latest_known_session_start + } + /// Add new observed session to the Oracle. pub fn add_session(&mut self, rounds: Rounds) { self.sessions.push_back(rounds); @@ -242,7 +249,7 @@ impl VoterOracle { pub fn voting_target(&self) -> Option> { let rounds = self.sessions.front().or_else(|| { debug!(target: LOG_TARGET, "🥩 No voting round started"); - return None + None })?; let best_grandpa = *self.best_grandpa_block_header.number(); let best_beefy = self.best_beefy_block; @@ -367,10 +374,7 @@ where .beefy_genesis(best_grandpa.hash()) .ok() .flatten() - .filter(|genesis| { - // test - *genesis == beefy_genesis - }) + .filter(|genesis| *genesis == beefy_genesis) .ok_or_else(|| Error::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. @@ -473,8 +477,10 @@ where let mut new_sessions = vec![]; let mut header = best_grandpa.clone(); while *header.number() > state.best_beefy() { - if let Some(active) = find_authorities_change::(&header) { - new_sessions.push((active, *header.number())); + if state.voting_oracle.can_add_session(*header.number()) { + if let Some(active) = find_authorities_change::(&header) { + new_sessions.push((active, *header.number())); + } } header = wait_for_parent_header(self.backend.blockchain(), header, HEADER_SYNC_DELAY) @@ -566,7 +572,7 @@ where } } -/// A BEEFY worker plays the BEEFY protocol +/// A BEEFY worker/voter that follows the BEEFY protocol pub(crate) struct BeefyWorker { pub base: BeefyWorkerBase, From 3bf0805e2676a69271e5e1a931c235a7e641e447 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 31 Jan 2024 19:09:26 +0100 Subject: [PATCH 11/15] Revert "Fix toml files" This reverts commit 7ac8282288100dde4f2b36f2d1a36cdab1bd0899. --- Cargo.toml | 2 +- polkadot/node/subsystem-bench/Cargo.toml | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fd8708a3dadd..1edc64217fdf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -159,6 +159,7 @@ members = [ "polkadot/node/gum/proc-macro", "polkadot/node/jaeger", "polkadot/node/malus", + "polkadot/node/subsystem-bench", "polkadot/node/metrics", "polkadot/node/network/approval-distribution", "polkadot/node/network/availability-distribution", @@ -175,7 +176,6 @@ members = [ "polkadot/node/service", "polkadot/node/subsystem", "polkadot/node/subsystem-bench", - "polkadot/node/subsystem-bench", "polkadot/node/subsystem-test-helpers", "polkadot/node/subsystem-types", "polkadot/node/subsystem-util", diff --git a/polkadot/node/subsystem-bench/Cargo.toml b/polkadot/node/subsystem-bench/Cargo.toml index f7866f993631..263f0bd036e9 100644 --- a/polkadot/node/subsystem-bench/Cargo.toml +++ b/polkadot/node/subsystem-bench/Cargo.toml @@ -22,13 +22,13 @@ polkadot-node-subsystem-types = { path = "../subsystem-types" } polkadot-node-primitives = { path = "../primitives" } polkadot-primitives = { path = "../../primitives" } polkadot-node-network-protocol = { path = "../network/protocol" } -polkadot-availability-recovery = { path = "../network/availability-recovery", features = ["subsystem-benchmarks"] } -polkadot-availability-distribution = { path = "../network/availability-distribution" } -polkadot-node-core-av-store = { path = "../core/av-store" } -polkadot-node-core-chain-api = { path = "../core/chain-api" } -polkadot-availability-bitfield-distribution = { path = "../network/bitfield-distribution" } +polkadot-availability-recovery = { path = "../network/availability-recovery", features=["subsystem-benchmarks"]} +polkadot-availability-distribution = { path = "../network/availability-distribution"} +polkadot-node-core-av-store = { path = "../core/av-store"} +polkadot-node-core-chain-api = { path = "../core/chain-api"} +polkadot-availability-bitfield-distribution = { path = "../network/bitfield-distribution"} color-eyre = { version = "0.6.1", default-features = false } -polkadot-overseer = { path = "../overseer" } +polkadot-overseer = { path = "../overseer" } colored = "2.0.4" assert_matches = "1.5" async-trait = "0.1.57" @@ -45,10 +45,10 @@ env_logger = "0.9.0" rand = "0.8.5" # `rand` only supports uniform distribution, we need normal distribution for latency. rand_distr = "0.4.3" -bitvec = "1.0.1" +bitvec="1.0.1" kvdb-memorydb = "0.13.0" -parity-scale-codec = { version = "3.6.1", features = ["derive", "std"] } +parity-scale-codec = { version = "3.6.1", features = ["std", "derive"] } tokio = "1.24.2" clap-num = "1.0.2" polkadot-node-subsystem-test-helpers = { path = "../subsystem-test-helpers" } From 11d19467a6686bae86972010efaacca8bf4f76ef Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 31 Jan 2024 19:11:09 +0100 Subject: [PATCH 12/15] Revert "Revert "Fix toml files"" This reverts commit 3bf0805e2676a69271e5e1a931c235a7e641e447. --- Cargo.toml | 2 +- polkadot/node/subsystem-bench/Cargo.toml | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1edc64217fdf..fd8708a3dadd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -159,7 +159,6 @@ members = [ "polkadot/node/gum/proc-macro", "polkadot/node/jaeger", "polkadot/node/malus", - "polkadot/node/subsystem-bench", "polkadot/node/metrics", "polkadot/node/network/approval-distribution", "polkadot/node/network/availability-distribution", @@ -176,6 +175,7 @@ members = [ "polkadot/node/service", "polkadot/node/subsystem", "polkadot/node/subsystem-bench", + "polkadot/node/subsystem-bench", "polkadot/node/subsystem-test-helpers", "polkadot/node/subsystem-types", "polkadot/node/subsystem-util", diff --git a/polkadot/node/subsystem-bench/Cargo.toml b/polkadot/node/subsystem-bench/Cargo.toml index 263f0bd036e9..f7866f993631 100644 --- a/polkadot/node/subsystem-bench/Cargo.toml +++ b/polkadot/node/subsystem-bench/Cargo.toml @@ -22,13 +22,13 @@ polkadot-node-subsystem-types = { path = "../subsystem-types" } polkadot-node-primitives = { path = "../primitives" } polkadot-primitives = { path = "../../primitives" } polkadot-node-network-protocol = { path = "../network/protocol" } -polkadot-availability-recovery = { path = "../network/availability-recovery", features=["subsystem-benchmarks"]} -polkadot-availability-distribution = { path = "../network/availability-distribution"} -polkadot-node-core-av-store = { path = "../core/av-store"} -polkadot-node-core-chain-api = { path = "../core/chain-api"} -polkadot-availability-bitfield-distribution = { path = "../network/bitfield-distribution"} +polkadot-availability-recovery = { path = "../network/availability-recovery", features = ["subsystem-benchmarks"] } +polkadot-availability-distribution = { path = "../network/availability-distribution" } +polkadot-node-core-av-store = { path = "../core/av-store" } +polkadot-node-core-chain-api = { path = "../core/chain-api" } +polkadot-availability-bitfield-distribution = { path = "../network/bitfield-distribution" } color-eyre = { version = "0.6.1", default-features = false } -polkadot-overseer = { path = "../overseer" } +polkadot-overseer = { path = "../overseer" } colored = "2.0.4" assert_matches = "1.5" async-trait = "0.1.57" @@ -45,10 +45,10 @@ env_logger = "0.9.0" rand = "0.8.5" # `rand` only supports uniform distribution, we need normal distribution for latency. rand_distr = "0.4.3" -bitvec="1.0.1" +bitvec = "1.0.1" kvdb-memorydb = "0.13.0" -parity-scale-codec = { version = "3.6.1", features = ["std", "derive"] } +parity-scale-codec = { version = "3.6.1", features = ["derive", "std"] } tokio = "1.24.2" clap-num = "1.0.2" polkadot-node-subsystem-test-helpers = { path = "../subsystem-test-helpers" } From de26869b3534c07de0ed2f8761f12a48de476df5 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 31 Jan 2024 19:12:19 +0100 Subject: [PATCH 13/15] Fix build --- Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index fd8708a3dadd..20cc16039fe4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -175,7 +175,6 @@ members = [ "polkadot/node/service", "polkadot/node/subsystem", "polkadot/node/subsystem-bench", - "polkadot/node/subsystem-bench", "polkadot/node/subsystem-test-helpers", "polkadot/node/subsystem-types", "polkadot/node/subsystem-util", From 0d079431b47e00c13f20cfe844709e2755d4653f Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Thu, 1 Feb 2024 10:32:01 +0100 Subject: [PATCH 14/15] Add test --- substrate/client/consensus/beefy/src/tests.rs | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index d0cdf8bfe36b..d821a2db1f85 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -1305,6 +1305,104 @@ async fn should_initialize_voter_at_custom_genesis_when_state_unavailable() { assert_eq!(state, persisted_state); } +#[tokio::test] +async fn should_catch_up_when_loading_saved_voter_state() { + 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(); + + // push 15 blocks with `AuthorityChange` digests every 10 blocks + let hashes = net.generate_blocks_and_sync(30, 10, &validator_set, false).await; + let mut finality = net.peer(0).client().as_client().finality_notification_stream().fuse(); + // finalize 13 without justifications + net.peer(0).client().as_client().finalize_block(hashes[13], None).unwrap(); + + let api = TestApi::with_validator_set(&validator_set); + + // load persistent state - nothing in DB, should init at genesis + // + // NOTE: code from `voter_init_setup()` is moved here because the new network event system + // doesn't allow creating a new `GossipEngine` as the notification handle is consumed by the + // first `GossipEngine` + let known_peers = Arc::new(Mutex::new(KnownPeers::new())); + let (gossip_validator, _) = GossipValidator::new(known_peers); + let gossip_validator = Arc::new(gossip_validator); + let mut gossip_engine = sc_network_gossip::GossipEngine::new( + net.peer(0).network_service().clone(), + net.peer(0).sync_service().clone(), + net.peer(0).take_notification_service(&beefy_gossip_proto_name()).unwrap(), + "/beefy/whatever", + gossip_validator, + None, + ); + let (beefy_genesis, best_grandpa) = + wait_for_runtime_pallet(&api, &mut gossip_engine, &mut finality).await.unwrap(); + let mut worker_base = BeefyWorkerBase { + backend: backend.clone(), + runtime: Arc::new(api.clone()), + key_store: None.into(), + metrics: None, + _phantom: Default::default(), + }; + let persisted_state = + worker_base.load_or_init_state(beefy_genesis, best_grandpa, 1).await.unwrap(); + + // Test initialization at session boundary. + // verify voter initialized with two sessions starting at blocks 1 and 10 + let sessions = persisted_state.voting_oracle().sessions(); + assert_eq!(sessions.len(), 2); + assert_eq!(sessions[0].session_start(), 1); + assert_eq!(sessions[1].session_start(), 10); + let rounds = persisted_state.active_round().unwrap(); + assert_eq!(rounds.session_start(), 1); + assert_eq!(rounds.validator_set_id(), validator_set.id()); + + // verify next vote target is mandatory block 1 + 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)); + + // verify state also saved to db + assert!(verify_persisted_version(&*backend)); + let state = load_persistent(&*backend).unwrap().unwrap(); + assert_eq!(state, persisted_state); + + // now let's consider that the node goes offline, and then it restarts after a while + + // finalize 25 without justifications + net.peer(0).client().as_client().finalize_block(hashes[25], None).unwrap(); + // load persistent state - state preset in DB + // the network state persists and uses the old `GossipEngine` initialized for `peer(0)` + let (beefy_genesis, best_grandpa) = + wait_for_runtime_pallet(&api, &mut gossip_engine, &mut finality).await.unwrap(); + let mut worker_base = BeefyWorkerBase { + backend: backend.clone(), + runtime: Arc::new(api), + key_store: None.into(), + metrics: None, + _phantom: Default::default(), + }; + let persisted_state = + worker_base.load_or_init_state(beefy_genesis, best_grandpa, 1).await.unwrap(); + + // Verify voter initialized with old sessions plus a new one starting at block 20. + // There shouldn't be any duplicates. + let sessions = persisted_state.voting_oracle().sessions(); + assert_eq!(sessions.len(), 3); + assert_eq!(sessions[0].session_start(), 1); + assert_eq!(sessions[1].session_start(), 10); + assert_eq!(sessions[2].session_start(), 20); + let rounds = persisted_state.active_round().unwrap(); + assert_eq!(rounds.session_start(), 1); + assert_eq!(rounds.validator_set_id(), validator_set.id()); + + // verify next vote target is mandatory block 1 + assert_eq!(persisted_state.best_beefy(), 0); + assert_eq!(persisted_state.best_grandpa_number(), 25); + assert_eq!(persisted_state.voting_oracle().voting_target(), Some(1)); +} + #[tokio::test] async fn beefy_finalizing_after_pallet_genesis() { sp_tracing::try_init_simple(); From 746b1ea866e5eff71616680d1e98898d1062800d Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Thu, 1 Feb 2024 11:22:34 +0100 Subject: [PATCH 15/15] Fix comment Co-authored-by: Adrian Catangiu --- substrate/client/consensus/beefy/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index d821a2db1f85..7e61e877c1dd 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -1312,7 +1312,7 @@ async fn should_catch_up_when_loading_saved_voter_state() { let mut net = BeefyTestNet::new(1); let backend = net.peer(0).client().as_backend(); - // push 15 blocks with `AuthorityChange` digests every 10 blocks + // push 30 blocks with `AuthorityChange` digests every 10 blocks let hashes = net.generate_blocks_and_sync(30, 10, &validator_set, false).await; let mut finality = net.peer(0).client().as_client().finality_notification_stream().fuse(); // finalize 13 without justifications