From 7ac92df21e5b5b6f6eecc2cc945198f9d780a910 Mon Sep 17 00:00:00 2001 From: Eval EXEC Date: Sat, 15 Nov 2025 14:52:59 +0800 Subject: [PATCH 1/3] fix: prevent sync stuck on uncle blocks during chain reorgs Signed-off-by: Eval EXEC --- light-client-bin/src/subcmds.rs | 4 ++ .../src/protocols/light_client/mod.rs | 10 ++-- .../src/protocols/light_client/peers.rs | 22 +++++++-- light-client-lib/src/storage/db/browser.rs | 46 +++++++++++++++++++ light-client-lib/src/storage/db/native.rs | 45 ++++++++++++++++++ wasm/light-client-wasm/src/lib.rs | 4 ++ 6 files changed, 124 insertions(+), 7 deletions(-) diff --git a/light-client-bin/src/subcmds.rs b/light-client-bin/src/subcmds.rs index 339aa977..2441c9ef 100644 --- a/light-client-bin/src/subcmds.rs +++ b/light-client-bin/src/subcmds.rs @@ -40,6 +40,10 @@ impl RunConfig { .expect("build consensus should be OK"); storage.init_genesis_block(consensus.genesis_block().data()); + // Cleanup any invalid matched blocks from previous runs (e.g., uncle blocks from chain reorgs) + log::info!("Cleaning up invalid matched blocks..."); + storage.cleanup_invalid_matched_blocks(); + let pending_txs = Arc::new(RwLock::new(PendingTxs::default())); let max_outbound_peers = self.run_env.network.max_outbound_peers; let network_state = NetworkState::from_config(self.run_env.network) diff --git a/light-client-lib/src/protocols/light_client/mod.rs b/light-client-lib/src/protocols/light_client/mod.rs index a69edc50..c7bf193f 100644 --- a/light-client-lib/src/protocols/light_client/mod.rs +++ b/light-client-lib/src/protocols/light_client/mod.rs @@ -385,10 +385,14 @@ impl LightClientProtocol { debug!("fork to number: {}", to_number); let mut matched_blocks = self.peers.matched_blocks().write().await; let mut start_number_opt = None; - while let Some((start_number, _, _)) = self.storage.get_latest_matched_blocks() + while let Some((start_number, blocks_count, _)) = + self.storage.get_latest_matched_blocks() { - if start_number > to_number { - debug!("remove matched blocks start from: {}", start_number); + // Remove matched blocks if the range overlaps or is after the fork point + // The range is [start_number, start_number + blocks_count - 1] + if start_number + blocks_count > to_number { + debug!("remove matched blocks start from: {} (range covers {} blocks, overlaps fork at {})", + start_number, blocks_count, to_number); self.storage.remove_matched_blocks(start_number); } else { start_number_opt = Some(start_number); diff --git a/light-client-lib/src/protocols/light_client/peers.rs b/light-client-lib/src/protocols/light_client/peers.rs index 604e5bad..a60d67d0 100644 --- a/light-client-lib/src/protocols/light_client/peers.rs +++ b/light-client-lib/src/protocols/light_client/peers.rs @@ -1489,11 +1489,25 @@ impl Peers { matched_blocks .iter() .filter_map(|(key, value)| { - if !proof_requested_hashes.contains(key) && !value.0 { - Some(key.pack()) - } else { - None + // Skip if already in a proof request + if proof_requested_hashes.contains(key) { + return None; + } + // Skip if already proved + if value.0 { + return None; + } + // Skip if marked as missing by peers (e.g., uncle blocks) + if let Some(fetch_info) = self.fetching_headers.get(&key.pack()) { + if fetch_info.missing { + log::warn!( + "Skipping matched block {:#x} - marked as missing by peers (likely an uncle block)", + key.pack() + ); + return None; + } } + Some(key.pack()) }) .take(limit) .collect() diff --git a/light-client-lib/src/storage/db/browser.rs b/light-client-lib/src/storage/db/browser.rs index 0f0ee84e..15a2e2a9 100644 --- a/light-client-lib/src/storage/db/browser.rs +++ b/light-client-lib/src/storage/db/browser.rs @@ -1183,6 +1183,52 @@ impl Storage { self.put(key, &value) .expect("db put matched blocks should be ok"); } + + pub fn cleanup_invalid_matched_blocks(&self) { + use ckb_types::prelude::Unpack; + use log::warn; + + let tip_number: u64 = self.get_tip_header().raw().number().unpack(); + + loop { + let entry = self.get_earliest_matched_blocks(); + if entry.is_none() { + break; + } + + let (start_number, blocks_count, block_hashes) = entry.unwrap(); + let mut should_remove = false; + + for (block_hash, _) in &block_hashes { + if let Some(header) = self.get_header(block_hash) { + let stored_number: u64 = header.number(); + if stored_number < start_number || stored_number >= start_number + blocks_count + { + warn!( + "Invalid matched block {:#x} at number {} outside expected range [{}, {}), removing entry at start_number={}", + block_hash, stored_number, start_number, start_number + blocks_count, start_number + ); + should_remove = true; + break; + } + } else if start_number + 1000 < tip_number { + warn!( + "Matched block {:#x} not found in storage, entry at start_number={} is {} blocks behind tip, removing", + block_hash, start_number, tip_number - start_number + ); + should_remove = true; + break; + } + } + + if should_remove { + self.remove_matched_blocks(start_number); + } else { + break; + } + } + } + pub fn add_fetched_header(&self, hwe: &HeaderWithExtension) { let mut batch = self.batch(); let block_hash = hwe.header.calc_header_hash(); diff --git a/light-client-lib/src/storage/db/native.rs b/light-client-lib/src/storage/db/native.rs index 57e0e738..3efd2b40 100644 --- a/light-client-lib/src/storage/db/native.rs +++ b/light-client-lib/src/storage/db/native.rs @@ -717,6 +717,51 @@ impl Storage { .expect("db put matched blocks should be ok"); } + pub fn cleanup_invalid_matched_blocks(&self) { + use ckb_types::prelude::Unpack; + use log::warn; + + let tip_number: u64 = self.get_tip_header().raw().number().unpack(); + + loop { + let entry = self.get_earliest_matched_blocks(); + if entry.is_none() { + break; + } + + let (start_number, blocks_count, block_hashes) = entry.unwrap(); + let mut should_remove = false; + + for (block_hash, _) in &block_hashes { + if let Some(header) = self.get_header(block_hash) { + let stored_number: u64 = header.number(); + if stored_number < start_number || stored_number >= start_number + blocks_count + { + warn!( + "Invalid matched block {:#x} at number {} outside expected range [{}, {}), removing entry at start_number={}", + block_hash, stored_number, start_number, start_number + blocks_count, start_number + ); + should_remove = true; + break; + } + } else if start_number + 1000 < tip_number { + warn!( + "Matched block {:#x} not found in storage, entry at start_number={} is {} blocks behind tip, removing", + block_hash, start_number, tip_number - start_number + ); + should_remove = true; + break; + } + } + + if should_remove { + self.remove_matched_blocks(start_number); + } else { + break; + } + } + } + pub fn add_fetched_header(&self, hwe: &HeaderWithExtension) { let mut batch = self.batch(); let block_hash = hwe.header.calc_header_hash(); diff --git a/wasm/light-client-wasm/src/lib.rs b/wasm/light-client-wasm/src/lib.rs index 385113ae..20141391 100644 --- a/wasm/light-client-wasm/src/lib.rs +++ b/wasm/light-client-wasm/src/lib.rs @@ -154,6 +154,10 @@ pub async fn light_client( storage.init_genesis_block(genesis); + // Cleanup any invalid matched blocks from previous runs (e.g., uncle blocks from chain reorgs) + log::info!("Cleaning up invalid matched blocks..."); + storage.cleanup_invalid_matched_blocks(); + let pending_txs = Arc::new(tokio::sync::RwLock::new(PendingTxs::default())); let max_outbound_peers = config.network.max_outbound_peers; let network_secret_key = From 8615ad70c0f1ad2346e2eb2f1cf01cded5e4d955 Mon Sep 17 00:00:00 2001 From: Eval EXEC Date: Sun, 23 Nov 2025 15:20:00 +0800 Subject: [PATCH 2/3] fix: do not remove fork point block --- light-client-lib/src/protocols/light_client/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/light-client-lib/src/protocols/light_client/mod.rs b/light-client-lib/src/protocols/light_client/mod.rs index c7bf193f..b00430ef 100644 --- a/light-client-lib/src/protocols/light_client/mod.rs +++ b/light-client-lib/src/protocols/light_client/mod.rs @@ -388,10 +388,11 @@ impl LightClientProtocol { while let Some((start_number, blocks_count, _)) = self.storage.get_latest_matched_blocks() { - // Remove matched blocks if the range overlaps or is after the fork point + // Remove matched blocks if the range contains blocks after the fork point // The range is [start_number, start_number + blocks_count - 1] - if start_number + blocks_count > to_number { - debug!("remove matched blocks start from: {} (range covers {} blocks, overlaps fork at {})", + // Fork point (to_number) is the last valid block, so remove ranges containing blocks > to_number + if start_number + blocks_count > to_number + 1 { + debug!("remove matched blocks start from: {} (range covers {} blocks, contains blocks after fork at {})", start_number, blocks_count, to_number); self.storage.remove_matched_blocks(start_number); } else { From 4f3e30647d35fe677a6c0458b33c97da09b338fb Mon Sep 17 00:00:00 2001 From: Eval EXEC Date: Tue, 9 Dec 2025 15:41:50 +0800 Subject: [PATCH 3/3] Fix unit tests to match new reorg matched block removal logic Signed-off-by: Eval EXEC --- .../light_client/send_last_state_proof.rs | 63 +++++++++++++------ 1 file changed, 45 insertions(+), 18 deletions(-) diff --git a/light-client-lib/src/tests/protocols/light_client/send_last_state_proof.rs b/light-client-lib/src/tests/protocols/light_client/send_last_state_proof.rs index 18127e77..8dbb5944 100644 --- a/light-client-lib/src/tests/protocols/light_client/send_last_state_proof.rs +++ b/light-client-lib/src/tests/protocols/light_client/send_last_state_proof.rs @@ -1987,31 +1987,58 @@ async fn test_with_reorg_blocks(param: ReorgTestParameter) { min_filtered_block_number ); + // Matched blocks are removed if their range extends beyond the fork point. + // The removal condition is: start_number + blocks_count > to_number + 1 + // Since blocks_count = 4, matched blocks are kept only if: start_number <= to_number - 3 + // where to_number is the fork point (prev_last_number - rollback_blocks_count) + // + // Special case: if last_number == prev_last_number and rollback_blocks_count == 0, + // there's no reorg, so no matched blocks are removed. + let no_reorg = (rollback_blocks_count == 0) && (last_number == prev_last_number); + let earliest_matched_blocks_opt = storage.get_earliest_matched_blocks(); - if earliest_matched_number > min_filtered_block_number { - assert!(earliest_matched_blocks_opt.is_none()); - } else { + let latest_matched_blocks_opt = storage.get_latest_matched_blocks(); + + if no_reorg { + // No reorg happened, so matched blocks should still exist assert!(earliest_matched_blocks_opt.is_some()); assert_eq!( earliest_matched_blocks_opt.unwrap().0, earliest_matched_number ); - } - let latest_matched_blocks_opt = storage.get_latest_matched_blocks(); - if prev_last_number <= earliest_matched_number && last_number != prev_last_number { - assert!( - latest_matched_blocks_opt.is_none(), - "prev: {}, earliest: {}, latest: {}", - prev_last_number, - earliest_matched_number, - latest_matched_blocks_opt.unwrap().0 - ); - } else { assert!(latest_matched_blocks_opt.is_some()); - assert_eq!( - latest_matched_blocks_opt.unwrap().0, - min_filtered_block_number - ); + assert_eq!(latest_matched_blocks_opt.unwrap().0, prev_last_number); + } else { + // Reorg happened, calculate which matched blocks were removed + let to_number = min_filtered_block_number; + let blocks_count = 4u64; + let latest_kept_matched_block = if to_number >= blocks_count - 1 { + to_number - (blocks_count - 1) + } else { + 0 + }; + + if earliest_matched_number > latest_kept_matched_block { + // All matched blocks were removed + assert!(earliest_matched_blocks_opt.is_none()); + assert!( + latest_matched_blocks_opt.is_none(), + "prev: {}, earliest: {}, to_number: {}, latest_kept: {}", + prev_last_number, + earliest_matched_number, + to_number, + latest_kept_matched_block + ); + } else { + assert!(earliest_matched_blocks_opt.is_some()); + assert_eq!( + earliest_matched_blocks_opt.unwrap().0, + earliest_matched_number + ); + assert!(latest_matched_blocks_opt.is_some()); + let expected_latest = cmp::min(prev_last_number, latest_kept_matched_block); + assert_eq!(latest_matched_blocks_opt.unwrap().0, expected_latest); + } } assert_eq!( peers.matched_blocks().read().await.is_empty(),