From cee99c316e8079a28f7d1e2573f9d2bf2e1c0d0a Mon Sep 17 00:00:00 2001
From: Eval EXEC <execvy@gmail.com>
Date: Mon, 4 Sep 2023 15:48:21 +0800
Subject: [PATCH] wip

---
 chain/src/chain.rs                     | 22 +++++++++++++++-------
 shared/src/types/mod.rs                |  1 +
 sync/src/synchronizer/block_process.rs |  2 +-
 sync/src/synchronizer/mod.rs           |  6 +++---
 sync/src/types/mod.rs                  |  4 ++--
 5 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/chain/src/chain.rs b/chain/src/chain.rs
index cb16c5da0dd..3a021a22a4b 100644
--- a/chain/src/chain.rs
+++ b/chain/src/chain.rs
@@ -88,7 +88,7 @@ impl ChainController {
     pub fn process_block(
         &self,
         block: Arc<BlockView>,
-    ) -> (Result<bool, Error>, Vec<VerifyFailedBlockInfo>) {
+    ) -> Result<Vec<VerifyFailedBlockInfo>, Error> {
         self.internal_process_block(block, Switch::NONE)
     }
 
@@ -99,7 +99,7 @@ impl ChainController {
         &self,
         block: Arc<BlockView>,
         switch: Switch,
-    ) -> (Result<bool, Error>, Vec<VerifyFailedBlockInfo>) {
+    ) -> Result<Vec<VerifyFailedBlockInfo>, Error> {
         Request::call(&self.process_block_sender, (block, switch)).unwrap_or_else(|| {
             Err(InternalErrorKind::System
                 .other("Chain service has gone")
@@ -668,21 +668,28 @@ impl ChainService {
     pub fn process_block_v2(
         &self,
         block: Arc<BlockView>,
+        peer_id: PeerId,
         switch: Switch,
-    ) -> (Result<bool, Error>, Vec<VerifyFailedBlockInfo>) {
+    ) -> Vec<VerifyFailedBlockInfo> {
         let block_number = block.number();
         let block_hash = block.hash();
         if block_number < 1 {
             warn!("receive 0 number block: 0-{}", block_hash);
         }
 
-        let failed_blocks_peer_ids: Vec<VerifyFailedBlockInfo> =
+        let mut failed_blocks_peer_ids: Vec<VerifyFailedBlockInfo> =
             self.verify_failed_blocks_rx.iter().collect();
 
         if !switch.disable_non_contextual() {
             let result = self.non_contextual_verify(&block);
             match result {
-                Err(err) => return (Err(err), failed_blocks_peer_ids),
+                Err(err) => {
+                    failed_blocks_peer_ids.push(VerifyFailedBlockInfo {
+                        block_hash,
+                        peer_id,
+                    });
+                    return failed_blocks_peer_ids;
+                }
                 _ => {}
             }
         }
@@ -694,15 +701,16 @@ impl ChainService {
             }
         }
         debug!(
-            "processing block: {}-{}, orphan_len: {}, (tip:unverified_tip):({}:{})",
+            "processing block: {}-{}, orphan_len: {}, (tip:unverified_tip):({}:{}), and return failed_blocks_peer_ids: {:?}",
             block_number,
             block_hash,
             self.orphan_blocks_broker.len(),
             self.shared.snapshot().tip_number(),
             self.shared.get_unverified_tip().number(),
+            failed_blocks_peer_ids,
         );
 
-        (Ok(false), failed_blocks_peer_ids)
+        failed_blocks_peer_ids
     }
 
     fn accept_block(&self, block: Arc<BlockView>) -> Result<Option<(HeaderView, U256)>, Error> {
diff --git a/shared/src/types/mod.rs b/shared/src/types/mod.rs
index f0083e6596a..a1f38faa857 100644
--- a/shared/src/types/mod.rs
+++ b/shared/src/types/mod.rs
@@ -306,6 +306,7 @@ fn get_skip_height(height: BlockNumber) -> BlockNumber {
 
 pub const SHRINK_THRESHOLD: usize = 300;
 
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct VerifyFailedBlockInfo {
     pub block_hash: Byte32,
     pub peer_id: PeerId,
diff --git a/sync/src/synchronizer/block_process.rs b/sync/src/synchronizer/block_process.rs
index 3c58c54a4fb..8fd9d75da40 100644
--- a/sync/src/synchronizer/block_process.rs
+++ b/sync/src/synchronizer/block_process.rs
@@ -32,7 +32,7 @@ impl<'a> BlockProcess<'a> {
         let shared = self.synchronizer.shared();
 
         if shared.new_block_received(&block) {
-            let (this_block_verify_result, maliformed_peers) =
+            let (this_block_verify_result, malformed_peers) =
                 self.synchronizer.process_new_block(block.clone());
 
             if let Err(err) = this_block_verify_result {
diff --git a/sync/src/synchronizer/mod.rs b/sync/src/synchronizer/mod.rs
index 571b9f810f7..154c4fb5ead 100644
--- a/sync/src/synchronizer/mod.rs
+++ b/sync/src/synchronizer/mod.rs
@@ -353,14 +353,14 @@ impl Synchronizer {
     pub fn process_new_block(
         &self,
         block: core::BlockView,
-    ) -> (Result<bool, CKBError>, Vec<VerifyFailedBlockInfo>) {
+    ) -> Result<Option<Vec<VerifyFailedBlockInfo>>, CKBError> {
         let block_hash = block.hash();
         let status = self.shared.active_chain().get_block_status(&block_hash);
         // NOTE: Filtering `BLOCK_STORED` but not `BLOCK_RECEIVED`, is for avoiding
         // stopping synchronization even when orphan_pool maintains dirty items by bugs.
         if status.contains(BlockStatus::BLOCK_PARTIAL_STORED) {
             error!("block {} already partial stored", block_hash);
-            (Ok(false), Vec::new())
+            Ok(Some(Vec::new()))
         } else if status.contains(BlockStatus::HEADER_VALID) {
             self.shared.insert_new_block(&self.chain, Arc::new(block))
         } else {
@@ -369,7 +369,7 @@ impl Synchronizer {
                 status, block_hash,
             );
             // TODO which error should we return?
-            (Ok(false), Vec::new())
+            (Ok(Some(Vec::new())))
         }
     }
 
diff --git a/sync/src/types/mod.rs b/sync/src/types/mod.rs
index db2b9263337..003758911dc 100644
--- a/sync/src/types/mod.rs
+++ b/sync/src/types/mod.rs
@@ -1086,7 +1086,7 @@ impl SyncShared {
         &self,
         chain: &ChainController,
         block: Arc<core::BlockView>,
-    ) -> (Result<bool, CKBError>, Vec<VerifyFailedBlockInfo>) {
+    ) -> Result<Vec<VerifyFailedBlockInfo>, CKBError> {
         // Insert the given block into orphan_block_pool if its parent is not found
         // if !self.is_stored(&block.parent_hash()) {
         //     debug!(
@@ -1167,7 +1167,7 @@ impl SyncShared {
         &self,
         chain: &ChainController,
         block: Arc<core::BlockView>,
-    ) -> (Result<bool, CKBError>, Vec<VerifyFailedBlockInfo>) {
+    ) -> Result<Vec<VerifyFailedBlockInfo>, CKBError> {
         let ret = {
             let mut assume_valid_target = self.state.assume_valid_target();
             if let Some(ref target) = *assume_valid_target {