From 9f1beb9ae344392f269a3728c650bf792bdf85ac Mon Sep 17 00:00:00 2001 From: Cezary Olborski Date: Wed, 4 Mar 2026 09:40:13 +0800 Subject: [PATCH 1/5] fix: Reading state from discarded chain --- client/consensus/qpow/src/chain_management.rs | 48 +++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/client/consensus/qpow/src/chain_management.rs b/client/consensus/qpow/src/chain_management.rs index fd14cfda..de896660 100644 --- a/client/consensus/qpow/src/chain_management.rs +++ b/client/consensus/qpow/src/chain_management.rs @@ -179,6 +179,14 @@ where Ok(total_work) } + /// Returns true if the error indicates that block state was pruned/discarded. + /// Such blocks cannot be evaluated (e.g. get_total_work) when using ArchiveCanonical + /// pruning - non-canonical fork blocks have their state removed. + fn is_state_pruned_error(err: &sp_consensus::Error) -> bool { + let msg = format!("{:?}", err); + msg.contains("State already discarded") || msg.contains("UnknownBlock") + } + /// Method to find best chain when there's no current best header async fn find_best_chain( &self, @@ -211,7 +219,18 @@ where let header_number = *header.number(); log::debug!("Found header for leaf at height #{}", header_number); - let chain_work = self.calculate_chain_work(&header)?; + let chain_work = match self.calculate_chain_work(&header) { + Ok(work) => work, + Err(ref e) if Self::is_state_pruned_error(e) => { + log::debug!( + "Skipping leaf #{} ({:?}) - block state was pruned", + header_number, + leaf_hash + ); + continue; + }, + Err(e) => return Err(e), + }; log::debug!("Chain work for leaf #{}: {}", header_number, chain_work); if chain_work > best_work { @@ -586,7 +605,17 @@ where ); let mut best_header = current_best.clone(); - let mut best_work = self.calculate_chain_work(¤t_best)?; + let mut best_work = match self.calculate_chain_work(¤t_best) { + Ok(work) => work, + Err(ref e) if Self::is_state_pruned_error(e) => { + log::warn!( + target: "qpow", + "🍴️ Current best block state was pruned. Falling back to evaluating all leaves." + ); + return self.find_best_chain(leaves).await; + }, + Err(e) => return Err(e), + }; log::debug!( target: "qpow", "🍴️ Current best chain: {:?} with work: {:?}", @@ -639,7 +668,20 @@ where let header_number = *header.number(); log::debug!(target: "qpow", "🍴️ Found header for leaf at height #{}", header_number); - let chain_work = self.calculate_chain_work(&header)?; + let chain_work = match self.calculate_chain_work(&header) { + Ok(work) => work, + Err(ref e) if Self::is_state_pruned_error(e) => { + log::warn!( + target: "qpow", + "🍴️ Skipping leaf #{} ({:?}) - block state was pruned (non-canonical fork). Adding to ignored chains.", + header_number, + leaf_hash + ); + let _ = self.add_ignored_chain(*leaf_hash); + continue; + }, + Err(e) => return Err(e), + }; log::debug!(target: "qpow", "🍴️ Chain work for leaf #{}: {}", header_number, chain_work); let max_reorg_depth = self From 8873256321a79cb243042a12fc12fc7ae12e9b3c Mon Sep 17 00:00:00 2001 From: Cezary Olborski Date: Wed, 4 Mar 2026 12:52:15 +0800 Subject: [PATCH 2/5] fix: Errors enumerated --- client/consensus/qpow/src/chain_management.rs | 273 ++++++++++++------ client/consensus/qpow/src/lib.rs | 2 +- 2 files changed, 188 insertions(+), 87 deletions(-) diff --git a/client/consensus/qpow/src/chain_management.rs b/client/consensus/qpow/src/chain_management.rs index de896660..c44f8153 100644 --- a/client/consensus/qpow/src/chain_management.rs +++ b/client/consensus/qpow/src/chain_management.rs @@ -2,15 +2,78 @@ use futures::StreamExt; use primitive_types::{H256, U512}; use sc_client_api::{AuxStore, BlockBackend, BlockchainEvents, Finalizer}; use sc_service::TaskManager; -use sp_api::ProvideRuntimeApi; +use sp_api::{ApiError, ProvideRuntimeApi}; use sp_blockchain::{Backend, HeaderBackend}; -use sp_consensus::SelectChain; +use sp_consensus::{Error as ConsensusError, SelectChain}; use sp_consensus_qpow::QPoWApi; use sp_runtime::traits::{Block as BlockT, Header, One, Zero}; -use std::{marker::PhantomData, sync::Arc}; +use std::{fmt, marker::PhantomData, sync::Arc}; const IGNORED_CHAINS_PREFIX: &[u8] = b"QPow:IgnoredChains:"; +/// Errors from chain management operations (best chain selection, finalization, etc.) +#[derive(Debug)] +pub enum ChainManagementError { + /// Blockchain/header lookup failed + ChainLookup(String), + /// Block state was unavailable (e.g. pruned) + StateUnavailable(String), + /// No valid chain could be selected from the leaves + NoValidChain, + /// No common ancestor found between chains + NoCommonAncestor, + /// Failed to add chain to ignored list + FailedToAddIgnoredChain(String), + /// Failed to fetch blockchain leaves + FailedToFetchLeaves(String), + /// Finalization failed + FinalizationFailed(String), +} + +impl fmt::Display for ChainManagementError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::ChainLookup(msg) => write!(f, "Chain lookup error: {}", msg), + Self::StateUnavailable(msg) => write!(f, "State unavailable: {}", msg), + Self::NoValidChain => write!(f, "No valid chain found"), + Self::NoCommonAncestor => write!(f, "No common ancestor found"), + Self::FailedToAddIgnoredChain(msg) => write!(f, "Failed to add ignored chain: {}", msg), + Self::FailedToFetchLeaves(msg) => write!(f, "Failed to fetch leaves: {}", msg), + Self::FinalizationFailed(msg) => write!(f, "Finalization failed: {}", msg), + } + } +} + +impl std::error::Error for ChainManagementError {} + +impl From for ConsensusError { + fn from(err: ChainManagementError) -> Self { + match err { + ChainManagementError::ChainLookup(msg) => ConsensusError::ChainLookup(msg), + ChainManagementError::StateUnavailable(msg) => ConsensusError::StateUnavailable(msg), + ChainManagementError::NoValidChain => + ConsensusError::ChainLookup("No valid chain found".into()), + ChainManagementError::NoCommonAncestor => + ConsensusError::ChainLookup("No common ancestor found".into()), + other => ConsensusError::Other(Box::new(other)), + } + } +} + +/// Returns true if the error indicates that block state was pruned/discarded. +/// Uses structural matching on ApiError::UnknownBlock only. +fn is_state_pruned_error_raw(err: &(dyn std::error::Error + 'static)) -> bool { + if let Some(api_err) = err.downcast_ref::() { + if matches!(api_err, ApiError::UnknownBlock(_)) { + return true; + } + } + if let Some(source) = err.source() { + return is_state_pruned_error_raw(source); + } + false +} + pub struct HeaviestChain where B: BlockT, @@ -51,7 +114,7 @@ where } /// Finalizes blocks that are `max_reorg_depth - 1` blocks behind the current best block - pub fn finalize_canonical_at_depth(&self) -> Result<(), sp_consensus::Error> + pub fn finalize_canonical_at_depth(&self) -> Result<(), ConsensusError> where C: Finalizer, { @@ -71,11 +134,11 @@ where .header(best_hash) .map_err(|e| { log::error!("Failed to get header for best hash: {:?}, error: {:?}", best_hash, e); - sp_consensus::Error::Other(format!("Blockchain error: {:?}", e).into()) + ChainManagementError::ChainLookup(format!("Blockchain error: {:?}", e)) })? .ok_or_else(|| { log::error!("Missing header for best hash: {:?}", best_hash); - sp_consensus::Error::Other("Missing current best header".into()) + ChainManagementError::ChainLookup("Missing current best header".into()) })?; let best_number = *best_header.number(); @@ -110,13 +173,14 @@ where .hash(finalize_number) .map_err(|e| { log::error!("Failed to get hash for block #{}: {:?}", finalize_number, e); - sp_consensus::Error::Other( - format!("Failed to get hash at #{}: {:?}", finalize_number, e).into(), - ) + ChainManagementError::ChainLookup(format!( + "Failed to get hash at #{}: {:?}", + finalize_number, e + )) })? .ok_or_else(|| { log::error!("No block found at #{} for finalization", finalize_number); - sp_consensus::Error::Other(format!("No block found at #{}", finalize_number).into()) + ChainManagementError::ChainLookup(format!("No block found at #{}", finalize_number)) })?; log::debug!("✓ Found hash for finalization target: {:?}", finalize_hash); @@ -133,9 +197,10 @@ where finalize_hash, e ); - sp_consensus::Error::Other( - format!("Failed to finalize block #{}: {:?}", finalize_number, e).into(), - ) + ChainManagementError::FinalizationFailed(format!( + "Failed to finalize block #{}: {:?}", + finalize_number, e + )) })?; // Check if finalization was successful @@ -154,48 +219,58 @@ where Ok(()) } - fn calculate_chain_work(&self, chain_head: &B::Header) -> Result { - // calculate cumulative work of a chain - + /// Returns Some(work) on success, None when block state was pruned (non-canonical fork), + /// or Err for other failures. Prefer structural matching on ApiError::UnknownBlock; + /// string matching is a fallback for nested/wrapped errors. + fn try_calculate_chain_work( + &self, + chain_head: &B::Header, + ) -> Result, ConsensusError> { let current_hash = chain_head.hash(); let current_number = *chain_head.number(); - let total_work = self.client.runtime_api().get_total_work(current_hash).map_err(|e| { - log::error!( - "Failed to get total work for chain with head #{}: {:?}", - current_number, - e - ); - sp_consensus::Error::Other(format!("Failed to get total difficulty {:?}", e).into()) - })?; - - log::info!( - "⛏️ Total chain work: {:?} for chain with head at #{:?} hash: {:?}", - total_work, - current_number, - current_hash - ); - - Ok(total_work) - } - - /// Returns true if the error indicates that block state was pruned/discarded. - /// Such blocks cannot be evaluated (e.g. get_total_work) when using ArchiveCanonical - /// pruning - non-canonical fork blocks have their state removed. - fn is_state_pruned_error(err: &sp_consensus::Error) -> bool { - let msg = format!("{:?}", err); - msg.contains("State already discarded") || msg.contains("UnknownBlock") + match self.client.runtime_api().get_total_work(current_hash) { + Ok(total_work) => { + log::info!( + "⛏️ Total chain work: {:?} for chain with head at #{:?} hash: {:?}", + total_work, + current_number, + current_hash + ); + Ok(Some(total_work)) + }, + Err(e) => { + let is_pruned = is_state_pruned_error_raw(&e); + if is_pruned { + log::error!( + "Failed to get total work for chain with head #{}: {:?}", + current_number, + e + ); + Ok(None) + } else { + log::error!( + "Failed to get total work for chain with head #{}: {:?}", + current_number, + e + ); + Err(ChainManagementError::StateUnavailable(format!( + "Failed to get total difficulty: {:?}", + e + )) + .into()) + } + }, + } } /// Method to find best chain when there's no current best header - async fn find_best_chain( - &self, - leaves: Vec, - ) -> Result { + async fn find_best_chain(&self, leaves: Vec) -> Result { log::debug!("Finding best chain among {} leaves when no current best exists", leaves.len()); let mut best_header = None; let mut best_work = U512::zero(); + let mut skipped_pruned = 0u32; for (idx, leaf_hash) in leaves.iter().enumerate() { log::debug!("Checking leaf [{}/{}]: {:?}", idx + 1, leaves.len(), leaf_hash); @@ -209,27 +284,34 @@ where leaf_hash, e ); - sp_consensus::Error::Other(format!("Blockchain error: {:?}", e).into()) + ChainManagementError::ChainLookup(format!("Blockchain error: {:?}", e)) })? .ok_or_else(|| { log::error!("Missing header for leaf {:?}", leaf_hash); - sp_consensus::Error::Other(format!("Missing header for {:?}", leaf_hash).into()) + ChainManagementError::ChainLookup(format!("Missing header for {:?}", leaf_hash)) })?; let header_number = *header.number(); log::debug!("Found header for leaf at height #{}", header_number); - let chain_work = match self.calculate_chain_work(&header) { - Ok(work) => work, - Err(ref e) if Self::is_state_pruned_error(e) => { + let chain_work = match self.try_calculate_chain_work(&header)? { + Some(work) => work, + None => { log::debug!( - "Skipping leaf #{} ({:?}) - block state was pruned", + "Skipping leaf #{} ({:?}) - block state was pruned. Adding to ignored chains.", header_number, leaf_hash ); + skipped_pruned += 1; + if let Err(e) = self.add_ignored_chain(*leaf_hash) { + log::warn!( + "Failed to add pruned leaf {:?} to ignored chains: {:?}", + leaf_hash, + e + ); + } continue; }, - Err(e) => return Err(e), }; log::debug!("Chain work for leaf #{}: {}", header_number, chain_work); @@ -260,11 +342,16 @@ where header.hash(), best_work ); + } else if skipped_pruned > 0 && skipped_pruned == leaves.len() as u32 { + log::warn!( + "No valid chain found: all {} leaves had pruned state (non-canonical forks)", + leaves.len() + ); } else { log::error!("No valid chain found among the leaves"); } - best_header.ok_or(sp_consensus::Error::Other("No Valid Chain Found".into())) + best_header.ok_or(ChainManagementError::NoValidChain.into()) } /// Method to find Re-Org depth and fork-point @@ -272,7 +359,7 @@ where &self, current_best: &B::Header, competing_header: &B::Header, - ) -> Result<(B::Hash, u32), sp_consensus::Error> { + ) -> Result<(B::Hash, u32), ConsensusError> { let current_best_hash = current_best.hash(); let competing_hash = competing_header.hash(); let current_height = *current_best.number(); @@ -334,11 +421,11 @@ where current_height, e ); - sp_consensus::Error::Other(format!("Blockchain error: {:?}", e).into()) + ChainManagementError::ChainLookup(format!("Blockchain error: {:?}", e)) })? .ok_or_else(|| { log::error!("Missing header at #{} ({:?})", current_height, current_best_hash); - sp_consensus::Error::Other("Missing header".into()) + ChainManagementError::ChainLookup("Missing header".into()) })? .parent_hash(); @@ -377,7 +464,7 @@ where competing_height, e ); - sp_consensus::Error::Other(format!("Blockchain error: {:?}", e).into()) + ChainManagementError::ChainLookup(format!("Blockchain error: {:?}", e)) })? .ok_or_else(|| { log::error!( @@ -385,7 +472,7 @@ where competing_height, competing_hash ); - sp_consensus::Error::Other("Missing header".into()) + ChainManagementError::ChainLookup("Missing header".into()) })? .parent_hash(); @@ -406,7 +493,7 @@ where // If we reach genesis and still no match, no common ancestor if current_height.is_zero() { log::error!("Reached genesis block without finding common ancestor"); - return Err(sp_consensus::Error::Other("No common ancestor found".into())); + return Err(ChainManagementError::NoCommonAncestor.into()); } log::debug!( @@ -426,7 +513,7 @@ where current_height, e ); - sp_consensus::Error::Other(format!("Blockchain error: {:?}", e).into()) + ChainManagementError::ChainLookup(format!("Blockchain error: {:?}", e)) })? .ok_or_else(|| { log::error!( @@ -434,7 +521,7 @@ where current_height, current_best_hash ); - sp_consensus::Error::Other("Missing header".into()) + ChainManagementError::ChainLookup("Missing header".into()) })? .parent_hash(); @@ -448,7 +535,7 @@ where current_height, e ); - sp_consensus::Error::Other(format!("Blockchain error: {:?}", e).into()) + ChainManagementError::ChainLookup(format!("Blockchain error: {:?}", e)) })? .ok_or_else(|| { log::error!( @@ -456,7 +543,7 @@ where current_height, competing_hash ); - sp_consensus::Error::Other("Missing header".into()) + ChainManagementError::ChainLookup("Missing header".into()) })? .parent_hash(); @@ -485,7 +572,7 @@ where Ok((current_best_hash, reorg_depth)) } - fn is_chain_ignored(&self, hash: &B::Hash) -> Result { + fn is_chain_ignored(&self, hash: &B::Hash) -> Result { log::debug!("Checking if chain with head {:?} is ignored", hash); let key = ignored_chain_key(hash); @@ -501,14 +588,16 @@ where }, Err(e) => { log::error!("Failed to check if chain with head {:?} is ignored: {:?}", hash, e); - Err(sp_consensus::Error::Other( - format!("Failed to check ignored chain: {:?}", e).into(), + Err(ChainManagementError::FailedToAddIgnoredChain(format!( + "Failed to check ignored chain: {:?}", + e )) + .into()) }, } } - fn add_ignored_chain(&self, hash: B::Hash) -> Result<(), sp_consensus::Error> { + fn add_ignored_chain(&self, hash: B::Hash) -> Result<(), ConsensusError> { log::debug!("Adding chain with head {:?} to ignored chains", hash); let key = ignored_chain_key(&hash); @@ -522,7 +611,11 @@ where .insert_aux(&[(key.as_slice(), empty_value.as_slice())], &[]) .map_err(|e| { log::error!("Failed to add chain with head {:?} to ignored chains: {:?}", hash, e); - sp_consensus::Error::Other(format!("Failed to add ignored chain: {:?}", e).into()) + ChainManagementError::FailedToAddIgnoredChain(format!( + "Failed to add ignored chain: {:?}", + e + )) + .into() }) } } @@ -535,12 +628,12 @@ where C::Api: QPoWApi, BE: sc_client_api::Backend + 'static, { - async fn leaves(&self) -> Result, sp_consensus::Error> { + async fn leaves(&self) -> Result, ConsensusError> { log::debug!("Getting blockchain leaves"); let leaves = self.backend.blockchain().leaves().map_err(|e| { log::error!("Failed to fetch leaves: {:?}", e); - sp_consensus::Error::Other(format!("Failed to fetch leaves: {:?}", e).into()) + ChainManagementError::FailedToFetchLeaves(format!("Failed to fetch leaves: {:?}", e)) })?; log::debug!("Found {} leaves", leaves.len()); @@ -548,19 +641,19 @@ where Ok(leaves) } - async fn best_chain(&self) -> Result { + async fn best_chain(&self) -> Result { log::debug!(target: "qpow", "------ 🍴️Starting best chain selection process ------"); let leaves = self.backend.blockchain().leaves().map_err(|e| { log::error!("🍴️ Failed to fetch leaves: {:?}", e); - sp_consensus::Error::Other(format!("Failed to fetch leaves: {:?}", e).into()) + ChainManagementError::FailedToFetchLeaves(format!("Failed to fetch leaves: {:?}", e)) })?; log::debug!(target: "qpow", "🍴️ Found {} leaves to evaluate", leaves.len()); if leaves.is_empty() { log::error!("🍴️ Blockchain has no leaves"); - return Err(sp_consensus::Error::Other("Blockchain has no leaves".into())); + return Err(ChainManagementError::NoValidChain.into()); } // Get info about last finalized block @@ -579,11 +672,11 @@ where "🍴️ Blockchain error when getting header for best hash: {:?}", e ); - sp_consensus::Error::Other(format!("Blockchain error: {:?}", e).into()) + ChainManagementError::ChainLookup(format!("Blockchain error: {:?}", e)) })? .ok_or_else(|| { log::error!("🍴️ Missing header for current best hash: {:?}", hash); - sp_consensus::Error::Other("Missing current best header".into()) + ChainManagementError::ChainLookup("Missing current best header".into()) })? }, _ => { @@ -605,16 +698,18 @@ where ); let mut best_header = current_best.clone(); - let mut best_work = match self.calculate_chain_work(¤t_best) { - Ok(work) => work, - Err(ref e) if Self::is_state_pruned_error(e) => { + let mut best_work = match self.try_calculate_chain_work(¤t_best)? { + Some(work) => work, + None => { + // Emergency fallback: current best has pruned state. Evaluate all leaves without + // reorg depth constraints. Note: this bypasses max_reorg_depth for the fallback + // path, which is acceptable since we have no valid current best to compare against. log::warn!( target: "qpow", "🍴️ Current best block state was pruned. Falling back to evaluating all leaves." ); return self.find_best_chain(leaves).await; }, - Err(e) => return Err(e), }; log::debug!( target: "qpow", @@ -658,29 +753,35 @@ where .header(*leaf_hash) .map_err(|e| { log::error!("🍴️ Blockchain error when getting header for leaf: {:?}", e); - sp_consensus::Error::Other(format!("Blockchain error: {:?}", e).into()) + ChainManagementError::ChainLookup(format!("Blockchain error: {:?}", e)) })? .ok_or_else(|| { log::error!("🍴️ Missing header for leaf hash: {:?}", leaf_hash); - sp_consensus::Error::Other(format!("Missing header for {:?}", leaf_hash).into()) + ChainManagementError::ChainLookup(format!("Missing header for {:?}", leaf_hash)) })?; let header_number = *header.number(); log::debug!(target: "qpow", "🍴️ Found header for leaf at height #{}", header_number); - let chain_work = match self.calculate_chain_work(&header) { - Ok(work) => work, - Err(ref e) if Self::is_state_pruned_error(e) => { + let chain_work = match self.try_calculate_chain_work(&header)? { + Some(work) => work, + None => { log::warn!( target: "qpow", "🍴️ Skipping leaf #{} ({:?}) - block state was pruned (non-canonical fork). Adding to ignored chains.", header_number, leaf_hash ); - let _ = self.add_ignored_chain(*leaf_hash); + if let Err(e) = self.add_ignored_chain(*leaf_hash) { + log::warn!( + target: "qpow", + "🍴️ Failed to add pruned leaf {:?} to ignored chains: {:?}", + leaf_hash, + e + ); + } continue; }, - Err(e) => return Err(e), }; log::debug!(target: "qpow", "🍴️ Chain work for leaf #{}: {}", header_number, chain_work); diff --git a/client/consensus/qpow/src/lib.rs b/client/consensus/qpow/src/lib.rs index b3de0c0e..293da01a 100644 --- a/client/consensus/qpow/src/lib.rs +++ b/client/consensus/qpow/src/lib.rs @@ -1,7 +1,7 @@ mod chain_management; mod worker; -pub use chain_management::{ChainManagement, HeaviestChain}; +pub use chain_management::{ChainManagement, ChainManagementError, HeaviestChain}; use primitive_types::{H256, U512}; use sc_client_api::BlockBackend; use sp_api::ProvideRuntimeApi; From 3f3719037533794778348b6da12bdb4b0d91c72d Mon Sep 17 00:00:00 2001 From: Cezary Olborski Date: Wed, 4 Mar 2026 15:41:16 +0800 Subject: [PATCH 3/5] fix: AI review --- client/consensus/qpow/src/chain_management.rs | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/client/consensus/qpow/src/chain_management.rs b/client/consensus/qpow/src/chain_management.rs index c44f8153..052d5dd7 100644 --- a/client/consensus/qpow/src/chain_management.rs +++ b/client/consensus/qpow/src/chain_management.rs @@ -28,6 +28,8 @@ pub enum ChainManagementError { FailedToFetchLeaves(String), /// Finalization failed FinalizationFailed(String), + /// Runtime API call failed (e.g. get_total_work for reasons other than pruned state) + RuntimeApiError(String), } impl fmt::Display for ChainManagementError { @@ -40,6 +42,7 @@ impl fmt::Display for ChainManagementError { Self::FailedToAddIgnoredChain(msg) => write!(f, "Failed to add ignored chain: {}", msg), Self::FailedToFetchLeaves(msg) => write!(f, "Failed to fetch leaves: {}", msg), Self::FinalizationFailed(msg) => write!(f, "Finalization failed: {}", msg), + Self::RuntimeApiError(msg) => write!(f, "Runtime API error: {}", msg), } } } @@ -60,8 +63,9 @@ impl From for ConsensusError { } } -/// Returns true if the error indicates that block state was pruned/discarded. -/// Uses structural matching on ApiError::UnknownBlock only. +/// Returns true if the error indicates that block state was pruned/discarded or the block +/// is unknown (e.g. never imported). Uses structural matching on ApiError::UnknownBlock. +/// Note: UnknownBlock can also fire for blocks that were never imported, not just pruned. fn is_state_pruned_error_raw(err: &(dyn std::error::Error + 'static)) -> bool { if let Some(api_err) = err.downcast_ref::() { if matches!(api_err, ApiError::UnknownBlock(_)) { @@ -219,9 +223,8 @@ where Ok(()) } - /// Returns Some(work) on success, None when block state was pruned (non-canonical fork), - /// or Err for other failures. Prefer structural matching on ApiError::UnknownBlock; - /// string matching is a fallback for nested/wrapped errors. + /// Returns Some(work) on success, None when block state was pruned or block is unknown, + /// or Err for other runtime API failures. fn try_calculate_chain_work( &self, chain_head: &B::Header, @@ -242,8 +245,8 @@ where Err(e) => { let is_pruned = is_state_pruned_error_raw(&e); if is_pruned { - log::error!( - "Failed to get total work for chain with head #{}: {:?}", + log::warn!( + "Block state unavailable for chain head #{} (pruned or unknown): {:?}", current_number, e ); @@ -254,7 +257,7 @@ where current_number, e ); - Err(ChainManagementError::StateUnavailable(format!( + Err(ChainManagementError::RuntimeApiError(format!( "Failed to get total difficulty: {:?}", e )) @@ -724,6 +727,8 @@ where leaves.len() ); + let mut skipped_pruned = 0u32; + for (idx, leaf_hash) in leaves.iter().enumerate() { log::debug!( target: "qpow", @@ -766,6 +771,7 @@ where let chain_work = match self.try_calculate_chain_work(&header)? { Some(work) => work, None => { + skipped_pruned += 1; log::warn!( target: "qpow", "🍴️ Skipping leaf #{} ({:?}) - block state was pruned (non-canonical fork). Adding to ignored chains.", @@ -926,6 +932,14 @@ where } } + if skipped_pruned > 0 { + log::info!( + target: "qpow", + "🍴️ Skipped {} leaves with pruned state during best chain selection", + skipped_pruned + ); + } + if leaves.len() > 1 { log::info!( "🍴️ Evaluated {} leaves and selected best chain with head: #{} ({:?}) and work: {}", From c8a9a25e17270329d0090db03279b98b8909f4f2 Mon Sep 17 00:00:00 2001 From: Cezary Olborski Date: Wed, 4 Mar 2026 16:25:35 +0800 Subject: [PATCH 4/5] fix: Review - chain_management refactor --- client/consensus/qpow/src/chain_management.rs | 117 ++++++++---------- 1 file changed, 53 insertions(+), 64 deletions(-) diff --git a/client/consensus/qpow/src/chain_management.rs b/client/consensus/qpow/src/chain_management.rs index 052d5dd7..b15e5325 100644 --- a/client/consensus/qpow/src/chain_management.rs +++ b/client/consensus/qpow/src/chain_management.rs @@ -223,6 +223,51 @@ where Ok(()) } + /// Evaluates a leaf: fetches header, gets chain work. Returns Some((header, work)) on success, + /// None when block state was pruned (leaf is added to ignored chains), or Err on failure. + fn evaluate_leaf( + &self, + leaf_hash: B::Hash, + ) -> Result, ConsensusError> { + let header = self + .client + .header(leaf_hash) + .map_err(|e| { + log::error!( + target: "qpow", + "Blockchain error when getting header for leaf {:?}: {:?}", + leaf_hash, + e + ); + ChainManagementError::ChainLookup(format!("Blockchain error: {:?}", e)) + })? + .ok_or_else(|| { + log::error!("Missing header for leaf {:?}", leaf_hash); + ChainManagementError::ChainLookup(format!("Missing header for {:?}", leaf_hash)) + })?; + + match self.try_calculate_chain_work(&header)? { + Some(work) => Ok(Some((header, work))), + None => { + log::warn!( + target: "qpow", + "Skipping leaf #{} ({:?}) - block state was pruned. Adding to ignored chains.", + header.number(), + leaf_hash + ); + if let Err(e) = self.add_ignored_chain(leaf_hash) { + log::warn!( + target: "qpow", + "Failed to add pruned leaf {:?} to ignored chains: {:?}", + leaf_hash, + e + ); + } + Ok(None) + }, + } + } + /// Returns Some(work) on success, None when block state was pruned or block is unknown, /// or Err for other runtime API failures. fn try_calculate_chain_work( @@ -278,44 +323,15 @@ where for (idx, leaf_hash) in leaves.iter().enumerate() { log::debug!("Checking leaf [{}/{}]: {:?}", idx + 1, leaves.len(), leaf_hash); - let header = self - .client - .header(*leaf_hash) - .map_err(|e| { - log::error!( - "Blockchain error when getting header for leaf {:?}: {:?}", - leaf_hash, - e - ); - ChainManagementError::ChainLookup(format!("Blockchain error: {:?}", e)) - })? - .ok_or_else(|| { - log::error!("Missing header for leaf {:?}", leaf_hash); - ChainManagementError::ChainLookup(format!("Missing header for {:?}", leaf_hash)) - })?; - - let header_number = *header.number(); - log::debug!("Found header for leaf at height #{}", header_number); - - let chain_work = match self.try_calculate_chain_work(&header)? { - Some(work) => work, + let (header, chain_work) = match self.evaluate_leaf(*leaf_hash)? { + Some(result) => result, None => { - log::debug!( - "Skipping leaf #{} ({:?}) - block state was pruned. Adding to ignored chains.", - header_number, - leaf_hash - ); skipped_pruned += 1; - if let Err(e) = self.add_ignored_chain(*leaf_hash) { - log::warn!( - "Failed to add pruned leaf {:?} to ignored chains: {:?}", - leaf_hash, - e - ); - } continue; }, }; + + let header_number = *header.number(); log::debug!("Chain work for leaf #{}: {}", header_number, chain_work); if chain_work > best_work { @@ -753,42 +769,15 @@ where continue; } - let header = self - .client - .header(*leaf_hash) - .map_err(|e| { - log::error!("🍴️ Blockchain error when getting header for leaf: {:?}", e); - ChainManagementError::ChainLookup(format!("Blockchain error: {:?}", e)) - })? - .ok_or_else(|| { - log::error!("🍴️ Missing header for leaf hash: {:?}", leaf_hash); - ChainManagementError::ChainLookup(format!("Missing header for {:?}", leaf_hash)) - })?; - - let header_number = *header.number(); - log::debug!(target: "qpow", "🍴️ Found header for leaf at height #{}", header_number); - - let chain_work = match self.try_calculate_chain_work(&header)? { - Some(work) => work, + let (header, chain_work) = match self.evaluate_leaf(*leaf_hash)? { + Some(result) => result, None => { skipped_pruned += 1; - log::warn!( - target: "qpow", - "🍴️ Skipping leaf #{} ({:?}) - block state was pruned (non-canonical fork). Adding to ignored chains.", - header_number, - leaf_hash - ); - if let Err(e) = self.add_ignored_chain(*leaf_hash) { - log::warn!( - target: "qpow", - "🍴️ Failed to add pruned leaf {:?} to ignored chains: {:?}", - leaf_hash, - e - ); - } continue; }, }; + + let header_number = *header.number(); log::debug!(target: "qpow", "🍴️ Chain work for leaf #{}: {}", header_number, chain_work); let max_reorg_depth = self From 02ee2661d4326afec7a8988be1255354e2096a31 Mon Sep 17 00:00:00 2001 From: Cezary Olborski Date: Wed, 4 Mar 2026 18:06:45 +0800 Subject: [PATCH 5/5] fix: runtime exception + useless counter --- client/consensus/qpow/src/chain_management.rs | 54 +++++++------------ 1 file changed, 19 insertions(+), 35 deletions(-) diff --git a/client/consensus/qpow/src/chain_management.rs b/client/consensus/qpow/src/chain_management.rs index b15e5325..49a33cd1 100644 --- a/client/consensus/qpow/src/chain_management.rs +++ b/client/consensus/qpow/src/chain_management.rs @@ -148,11 +148,14 @@ where let best_number = *best_header.number(); log::debug!("Current best block number: {}", best_number); - let max_reorg_depth = self - .client - .runtime_api() - .get_max_reorg_depth(best_hash) - .expect("Failed to get max reorg depth"); + let max_reorg_depth = + self.client.runtime_api().get_max_reorg_depth(best_hash).map_err(|e| { + log::error!("Failed to get max reorg depth: {:?}", e); + ChainManagementError::RuntimeApiError(format!( + "Failed to get max reorg depth: {:?}", + e + )) + })?; // Calculate how far back to finalize let finalize_depth = max_reorg_depth.saturating_sub(1); @@ -318,17 +321,13 @@ where let mut best_header = None; let mut best_work = U512::zero(); - let mut skipped_pruned = 0u32; for (idx, leaf_hash) in leaves.iter().enumerate() { log::debug!("Checking leaf [{}/{}]: {:?}", idx + 1, leaves.len(), leaf_hash); let (header, chain_work) = match self.evaluate_leaf(*leaf_hash)? { Some(result) => result, - None => { - skipped_pruned += 1; - continue; - }, + None => continue, }; let header_number = *header.number(); @@ -361,13 +360,8 @@ where header.hash(), best_work ); - } else if skipped_pruned > 0 && skipped_pruned == leaves.len() as u32 { - log::warn!( - "No valid chain found: all {} leaves had pruned state (non-canonical forks)", - leaves.len() - ); } else { - log::error!("No valid chain found among the leaves"); + log::warn!("No valid chain found among {} leaves", leaves.len()); } best_header.ok_or(ChainManagementError::NoValidChain.into()) @@ -743,8 +737,6 @@ where leaves.len() ); - let mut skipped_pruned = 0u32; - for (idx, leaf_hash) in leaves.iter().enumerate() { log::debug!( target: "qpow", @@ -771,20 +763,20 @@ where let (header, chain_work) = match self.evaluate_leaf(*leaf_hash)? { Some(result) => result, - None => { - skipped_pruned += 1; - continue; - }, + None => continue, }; let header_number = *header.number(); log::debug!(target: "qpow", "🍴️ Chain work for leaf #{}: {}", header_number, chain_work); - let max_reorg_depth = self - .client - .runtime_api() - .get_max_reorg_depth(best_header.hash()) - .expect("Failed to get max reorg depth"); + let max_reorg_depth = + self.client.runtime_api().get_max_reorg_depth(best_header.hash()).map_err(|e| { + log::error!(target: "qpow", "Failed to get max reorg depth: {:?}", e); + ChainManagementError::RuntimeApiError(format!( + "Failed to get max reorg depth: {:?}", + e + )) + })?; log::debug!(target: "qpow", "🍴️ Max reorg depth from runtime: {}", max_reorg_depth); if chain_work >= best_work { @@ -921,14 +913,6 @@ where } } - if skipped_pruned > 0 { - log::info!( - target: "qpow", - "🍴️ Skipped {} leaves with pruned state during best chain selection", - skipped_pruned - ); - } - if leaves.len() > 1 { log::info!( "🍴️ Evaluated {} leaves and selected best chain with head: #{} ({:?}) and work: {}",