Modify unit test in ckb-sync and ckb-chain to use RemoteBlock related…

… API

chain/src/chain_controller.rs

@@ -8,6 +8,7 @@ use crate::{
 use ckb_channel::Sender;
 use ckb_error::{Error, InternalErrorKind};
 use ckb_logger::{self, error};
+use ckb_network::PeerIndex;
 use ckb_types::{
     core::{service::Request, BlockView},
     packed::Byte32,
@@ -64,7 +65,11 @@ impl ChainController {
 
     /// MinerRpc::submit_block and `ckb import` need this blocking way to process block
     pub fn blocking_process_block(&self, block: Arc<BlockView>) -> VerifyResult {
-        self.blocking_process_block_with_opt_switch(block, None)
+        self.blocking_process_block_internal(block, None, None)
+    }
+
+    pub fn blocking_process_remote_block(&self, remote_block: RemoteBlock) -> VerifyResult {
+        self.blocking_process_block_internal(remote_block.block, Some(remote_block.peer_id), None)
     }
 
     /// `IntegrationTestRpcImpl::process_block_without_verify` need this
@@ -73,12 +78,13 @@ impl ChainController {
         block: Arc<BlockView>,
         switch: Switch,
     ) -> VerifyResult {
-        self.blocking_process_block_with_opt_switch(block, Some(switch))
+        self.blocking_process_block_internal(block, None, Some(switch))
     }
 
-    pub fn blocking_process_block_with_opt_switch(
+    fn blocking_process_block_internal(
         &self,
         block: Arc<BlockView>,
+        peer_id: Option<PeerIndex>,
         switch: Option<Switch>,
     ) -> VerifyResult {
         let (verify_result_tx, verify_result_rx) = ckb_channel::oneshot::channel::<VerifyResult>();
@@ -96,7 +102,7 @@ impl ChainController {
 
         let lonely_block = LonelyBlock {
             block,
-            peer_id: None,
+            peer_id,
             switch,
             verify_callback: Some(Box::new(verify_callback)),
         };

chain/src/tests/find_fork.rs

@@ -1,10 +1,7 @@
 use crate::consume_orphan::ConsumeDescendantProcessor;
 use crate::consume_unverified::ConsumeUnverifiedBlockProcessor;
 use crate::utils::forkchanges::ForkChanges;
-use crate::{
-    start_chain_services, LonelyBlock, LonelyBlockHash, LonelyBlockHashWithCallback,
-    LonelyBlockWithCallback, VerifyFailedBlockInfo,
-};
+use crate::{start_chain_services, LonelyBlock, LonelyBlockHash, VerifyFailedBlockInfo};
 use ckb_chain_spec::consensus::{Consensus, ProposalWindow};
 use ckb_proposal_table::ProposalTable;
 use ckb_shared::types::BlockNumberAndHash;
@@ -33,23 +30,18 @@ fn process_block(
         peer_id: None,
         switch: Some(switch),
         block_number_and_hash: BlockNumberAndHash::new(blk.number(), blk.hash()),
+        verify_callback: None,
     };
 
     let lonely_block = LonelyBlock {
         peer_id: None,
         switch: Some(switch),
         block: Arc::new(blk.to_owned()),
+        verify_callback: None,
     };
 
-    consume_descendant_processor.process_descendant(LonelyBlockWithCallback {
-        verify_callback: None,
-        lonely_block,
-    });
+    consume_descendant_processor.process_descendant(lonely_block);
 
-    let lonely_block_hash = LonelyBlockHashWithCallback {
-        verify_callback: None,
-        lonely_block: lonely_block_hash,
-    };
     consume_unverified_block_processor.consume_unverified_blocks(lonely_block_hash);
 }
 
@@ -83,8 +75,7 @@ fn test_find_fork_case1() {
 
     let (verify_failed_blocks_tx, _verify_failed_blocks_rx) =
         tokio::sync::mpsc::unbounded_channel::<VerifyFailedBlockInfo>();
-    let (unverified_blocks_tx, _unverified_blocks_rx) =
-        channel::unbounded::<LonelyBlockHashWithCallback>();
+    let (unverified_blocks_tx, _unverified_blocks_rx) = channel::unbounded::<LonelyBlockHash>();
     let consume_descendant_processor = ConsumeDescendantProcessor {
         shared: shared.clone(),
         unverified_blocks_tx,
@@ -176,8 +167,7 @@ fn test_find_fork_case2() {
     let proposal_table = ProposalTable::new(consensus.tx_proposal_window());
     let (verify_failed_blocks_tx, _verify_failed_blocks_rx) =
         tokio::sync::mpsc::unbounded_channel::<VerifyFailedBlockInfo>();
-    let (unverified_blocks_tx, _unverified_blocks_rx) =
-        channel::unbounded::<LonelyBlockHashWithCallback>();
+    let (unverified_blocks_tx, _unverified_blocks_rx) = channel::unbounded::<LonelyBlockHash>();
     let consume_descendant_processor = ConsumeDescendantProcessor {
         shared: shared.clone(),
         unverified_blocks_tx,
@@ -270,8 +260,7 @@ fn test_find_fork_case3() {
     let proposal_table = ProposalTable::new(consensus.tx_proposal_window());
     let (verify_failed_blocks_tx, _verify_failed_blocks_rx) =
         tokio::sync::mpsc::unbounded_channel::<VerifyFailedBlockInfo>();
-    let (unverified_blocks_tx, _unverified_blocks_rx) =
-        channel::unbounded::<LonelyBlockHashWithCallback>();
+    let (unverified_blocks_tx, _unverified_blocks_rx) = channel::unbounded::<LonelyBlockHash>();
     let consume_descendant_processor = ConsumeDescendantProcessor {
         shared: shared.clone(),
         unverified_blocks_tx,
@@ -362,8 +351,7 @@ fn test_find_fork_case4() {
     let proposal_table = ProposalTable::new(consensus.tx_proposal_window());
     let (verify_failed_blocks_tx, _verify_failed_blocks_rx) =
         tokio::sync::mpsc::unbounded_channel::<VerifyFailedBlockInfo>();
-    let (unverified_blocks_tx, _unverified_blocks_rx) =
-        channel::unbounded::<LonelyBlockHashWithCallback>();
+    let (unverified_blocks_tx, _unverified_blocks_rx) = channel::unbounded::<LonelyBlockHash>();
     let consume_descendant_processor = ConsumeDescendantProcessor {
         shared: shared.clone(),
         unverified_blocks_tx,
@@ -455,8 +443,7 @@ fn repeatedly_switch_fork() {
     let proposal_table = ProposalTable::new(consensus.tx_proposal_window());
     let (verify_failed_blocks_tx, _verify_failed_blocks_rx) =
         tokio::sync::mpsc::unbounded_channel::<VerifyFailedBlockInfo>();
-    let (unverified_blocks_tx, _unverified_blocks_rx) =
-        channel::unbounded::<LonelyBlockHashWithCallback>();
+    let (unverified_blocks_tx, _unverified_blocks_rx) = channel::unbounded::<LonelyBlockHash>();
     let consume_descendant_processor = ConsumeDescendantProcessor {
         shared: shared.clone(),
         unverified_blocks_tx,

chain/src/tests/orphan_block_pool.rs

@@ -1,5 +1,5 @@
 #![allow(dead_code)]
-use crate::{LonelyBlock, LonelyBlockWithCallback};
+use crate::LonelyBlock;
 use ckb_chain_spec::consensus::ConsensusBuilder;
 use ckb_systemtime::unix_time_as_millis;
 use ckb_types::core::{BlockBuilder, BlockView, EpochNumberWithFraction, HeaderView};
@@ -23,13 +23,10 @@ fn gen_lonely_block(parent_header: &HeaderView) -> LonelyBlock {
         block: Arc::new(block),
         peer_id: None,
         switch: None,
+        verify_callback: None,
     }
 }
 
-fn gen_lonely_block_with_callback(parent_header: &HeaderView) -> LonelyBlockWithCallback {
-    gen_lonely_block(parent_header).without_callback()
-}
-
 #[test]
 fn test_remove_blocks_by_parent() {
     let consensus = ConsensusBuilder::default().build();
@@ -39,17 +36,22 @@ fn test_remove_blocks_by_parent() {
     let pool = OrphanBlockPool::with_capacity(200);
     for _ in 1..block_number {
         let lonely_block = gen_lonely_block(&parent);
-        let new_block_clone = lonely_block.clone().without_callback();
-        let new_block = lonely_block.without_callback();
+        let new_block_clone = lonely_block.block().clone();
+        let new_block = LonelyBlock {
+            block: new_block_clone.clone(),
+            peer_id: None,
+            switch: None,
+            verify_callback: None,
+        };
         blocks.push(new_block_clone);
 
         parent = new_block.block().header();
         pool.insert(new_block);
     }
 
     let orphan = pool.remove_blocks_by_parent(&consensus.genesis_block().hash());
-    let orphan_set: HashSet<_> = orphan.into_iter().map(|b| b.lonely_block.block).collect();
-    let blocks_set: HashSet<_> = blocks.into_iter().map(|b| b.lonely_block.block).collect();
+    let orphan_set: HashSet<_> = orphan.into_iter().map(|b| b.block).collect();
+    let blocks_set: HashSet<_> = blocks.into_iter().map(|b| b.to_owned()).collect();
     assert_eq!(orphan_set, blocks_set)
 }
 
@@ -61,10 +63,15 @@ fn test_remove_blocks_by_parent_and_get_block_should_not_deadlock() {
     let mut hashes = Vec::new();
     for _ in 1..1024 {
         let lonely_block = gen_lonely_block(&header);
-        let new_block = lonely_block.clone().without_callback();
-        let new_block_clone = lonely_block.without_callback();
+        let new_block = lonely_block.block();
+        let new_block_clone = LonelyBlock {
+            block: Arc::clone(new_block),
+            peer_id: None,
+            switch: None,
+            verify_callback: None,
+        };
         pool.insert(new_block_clone);
-        header = new_block.block().header();
+        header = new_block.header();
         hashes.push(header.hash());
     }
 
@@ -91,7 +98,12 @@ fn test_leaders() {
     let pool = OrphanBlockPool::with_capacity(20);
     for i in 0..block_number - 1 {
         let lonely_block = gen_lonely_block(&parent);
-        let new_block = lonely_block.clone().without_callback();
+        let new_block = LonelyBlock {
+            block: Arc::clone(lonely_block.block()),
+            peer_id: None,
+            switch: None,
+            verify_callback: None,
+        };
         blocks.push(lonely_block);
         parent = new_block.block().header();
         if i % 5 != 0 {
@@ -102,11 +114,21 @@ fn test_leaders() {
     assert_eq!(pool.len(), 15);
     assert_eq!(pool.leaders_len(), 4);
 
-    pool.insert(blocks[5].clone().without_callback());
+    pool.insert(LonelyBlock {
+        block: blocks[5].block().clone(),
+        peer_id: None,
+        switch: None,
+        verify_callback: None,
+    });
     assert_eq!(pool.len(), 16);
     assert_eq!(pool.leaders_len(), 3);
 
-    pool.insert(blocks[10].clone().without_callback());
+    pool.insert(LonelyBlock {
+        block: blocks[10].block().clone(),
+        peer_id: None,
+        switch: None,
+        verify_callback: None,
+    });
     assert_eq!(pool.len(), 17);
     assert_eq!(pool.leaders_len(), 2);
 
@@ -116,24 +138,34 @@ fn test_leaders() {
     assert_eq!(pool.len(), 17);
     assert_eq!(pool.leaders_len(), 2);
 
-    pool.insert(blocks[0].clone().without_callback());
+    pool.insert(LonelyBlock {
+        block: blocks[0].block().clone(),
+        peer_id: None,
+        switch: None,
+        verify_callback: None,
+    });
     assert_eq!(pool.len(), 18);
     assert_eq!(pool.leaders_len(), 2);
 
     let orphan = pool.remove_blocks_by_parent(&consensus.genesis_block().hash());
     assert_eq!(pool.len(), 3);
     assert_eq!(pool.leaders_len(), 1);
 
-    pool.insert(blocks[15].clone().without_callback());
+    pool.insert(LonelyBlock {
+        block: blocks[15].block().clone(),
+        peer_id: None,
+        switch: None,
+        verify_callback: None,
+    });
     assert_eq!(pool.len(), 4);
     assert_eq!(pool.leaders_len(), 1);
 
     let orphan_1 = pool.remove_blocks_by_parent(&blocks[14].block.hash());
 
     let orphan_set: HashSet<Arc<BlockView>> = orphan
         .into_iter()
-        .map(|b| b.lonely_block.block)
-        .chain(orphan_1.into_iter().map(|b| b.lonely_block.block))
+        .map(|b| b.block)
+        .chain(orphan_1.into_iter().map(|b| b.block))
         .collect();
     let blocks_set: HashSet<Arc<BlockView>> = blocks.into_iter().map(|b| b.block).collect();
     assert_eq!(orphan_set, blocks_set);
@@ -160,15 +192,13 @@ fn test_remove_expired_blocks() {
             .build();
 
         parent = new_block.header();
-        let lonely_block_with_callback = LonelyBlockWithCallback {
-            lonely_block: LonelyBlock {
-                block: Arc::new(new_block),
-                peer_id: None,
-                switch: None,
-            },
+        let lonely_block = LonelyBlock {
+            block: Arc::new(new_block),
+            peer_id: None,
+            switch: None,
             verify_callback: None,
         };
-        pool.insert(lonely_block_with_callback);
+        pool.insert(lonely_block);
     }
     assert_eq!(pool.leaders_len(), 1);
 

sync/src/synchronizer/mod.rs

@@ -41,6 +41,9 @@ use ckb_network::{
 use ckb_shared::types::{HeaderIndexView, VerifyFailedBlockInfo};
 use ckb_stop_handler::{new_crossbeam_exit_rx, register_thread};
 use ckb_systemtime::unix_time_as_millis;
+
+#[cfg(test)]
+use ckb_types::core;
 use ckb_types::{
     core::BlockNumber,
     packed::{self, Byte32},
@@ -393,11 +396,11 @@ impl Synchronizer {
             error!("block {} already stored", block_hash);
             Ok(false)
         } else if status.contains(BlockStatus::HEADER_VALID) {
-            self.shared.blocking_insert_new_block_with_verbose_info(
-                &self.chain,
-                Arc::new(block),
+            let remote_block = RemoteBlock {
+                block: Arc::new(block),
                 peer_id,
-            )
+            };
+            self.chain.blocking_process_remote_block(remote_block)
         } else {
             debug!(
                 "Synchronizer process_new_block unexpected status {:?} {}",

sync/src/tests/sync_shared.rs

@@ -3,7 +3,7 @@
 
 use crate::tests::util::{build_chain, inherit_block};
 use crate::SyncShared;
-use ckb_chain::{start_chain_services, store_unverified_block};
+use ckb_chain::{start_chain_services, store_unverified_block, RemoteBlock};
 use ckb_logger::info;
 use ckb_logger_service::LoggerInitGuard;
 use ckb_shared::block_status::BlockStatus;
@@ -108,8 +108,22 @@ fn test_insert_parent_unknown_block() {
     let valid_hash = valid_orphan.header().hash();
     let invalid_hash = invalid_orphan.header().hash();
     let parent_hash = parent.header().hash();
-    shared.accept_block(&chain, Arc::clone(&valid_orphan), None, None);
-    shared.accept_block(&chain, Arc::clone(&invalid_orphan), None, None);
+    shared.accept_remote_block(
+        &chain,
+        RemoteBlock {
+            block: Arc::clone(&valid_orphan),
+            peer_id: Default::default(),
+        },
+        None,
+    );
+    shared.accept_remote_block(
+        &chain,
+        RemoteBlock {
+            block: Arc::clone(&invalid_orphan),
+            peer_id: Default::default(),
+        },
+        None,
+    );
 
     let wait_for_block_status_match = |hash: &Byte32, expect_status: BlockStatus| -> bool {
         let mut status_match = false;

sync/src/types/mod.rs

@@ -1070,21 +1070,6 @@ impl SyncShared {
         chain.blocking_process_block(block)
     }
 
-    #[cfg(test)]
-    pub(crate) fn blocking_insert_new_block_with_verbose_info(
-        &self,
-        chain: &ChainController,
-        block: Arc<core::BlockView>,
-        peer_id: PeerIndex,
-    ) -> VerifyResult {
-        let lonely_block: LonelyBlock = LonelyBlock {
-            block,
-            peer_id: Some(peer_id),
-            switch: None,
-        };
-        chain.blocking_process_lonely_block(lonely_block)
-    }
-
     pub(crate) fn accept_remote_block(
         &self,
         chain: &ChainController,