From 927ac4dd15438e65719f57d2a86eaccb824ebd80 Mon Sep 17 00:00:00 2001 From: Eval EXEC Date: Wed, 31 Jan 2024 10:43:25 +0800 Subject: [PATCH] Modify unit test in ckb-sync and ckb-chain to use RemoteBlock related API --- chain/src/chain_controller.rs | 14 +++-- chain/src/tests/find_fork.rs | 31 +++-------- chain/src/tests/orphan_block_pool.rs | 82 +++++++++++++++++++--------- sync/src/synchronizer/mod.rs | 11 ++-- sync/src/tests/sync_shared.rs | 20 ++++++- sync/src/types/mod.rs | 15 ----- 6 files changed, 99 insertions(+), 74 deletions(-) diff --git a/chain/src/chain_controller.rs b/chain/src/chain_controller.rs index d07872ad8d..550f8cc945 100644 --- a/chain/src/chain_controller.rs +++ b/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) -> 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, 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, + peer_id: Option, switch: Option, ) -> VerifyResult { let (verify_result_tx, verify_result_rx) = ckb_channel::oneshot::channel::(); @@ -96,7 +102,7 @@ impl ChainController { let lonely_block = LonelyBlock { block, - peer_id: None, + peer_id, switch, verify_callback: Some(Box::new(verify_callback)), }; diff --git a/chain/src/tests/find_fork.rs b/chain/src/tests/find_fork.rs index cf2538a6a2..309fb86853 100644 --- a/chain/src/tests/find_fork.rs +++ b/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::(); - let (unverified_blocks_tx, _unverified_blocks_rx) = - channel::unbounded::(); + let (unverified_blocks_tx, _unverified_blocks_rx) = channel::unbounded::(); 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::(); - let (unverified_blocks_tx, _unverified_blocks_rx) = - channel::unbounded::(); + let (unverified_blocks_tx, _unverified_blocks_rx) = channel::unbounded::(); 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::(); - let (unverified_blocks_tx, _unverified_blocks_rx) = - channel::unbounded::(); + let (unverified_blocks_tx, _unverified_blocks_rx) = channel::unbounded::(); 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::(); - let (unverified_blocks_tx, _unverified_blocks_rx) = - channel::unbounded::(); + let (unverified_blocks_tx, _unverified_blocks_rx) = channel::unbounded::(); 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::(); - let (unverified_blocks_tx, _unverified_blocks_rx) = - channel::unbounded::(); + let (unverified_blocks_tx, _unverified_blocks_rx) = channel::unbounded::(); let consume_descendant_processor = ConsumeDescendantProcessor { shared: shared.clone(), unverified_blocks_tx, diff --git a/chain/src/tests/orphan_block_pool.rs b/chain/src/tests/orphan_block_pool.rs index 2974852483..f73319495f 100644 --- a/chain/src/tests/orphan_block_pool.rs +++ b/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,8 +36,13 @@ 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(); @@ -48,8 +50,8 @@ fn test_remove_blocks_by_parent() { } 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,7 +138,12 @@ 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); @@ -124,7 +151,12 @@ fn test_leaders() { 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); @@ -132,8 +164,8 @@ fn test_leaders() { let orphan_set: HashSet> = 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> = 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); diff --git a/sync/src/synchronizer/mod.rs b/sync/src/synchronizer/mod.rs index f24fa0ab0d..3b72f57605 100644 --- a/sync/src/synchronizer/mod.rs +++ b/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 {:?} {}", diff --git a/sync/src/tests/sync_shared.rs b/sync/src/tests/sync_shared.rs index a25060165e..456ecb70bc 100644 --- a/sync/src/tests/sync_shared.rs +++ b/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; diff --git a/sync/src/types/mod.rs b/sync/src/types/mod.rs index f42974ea73..2766ddc723 100644 --- a/sync/src/types/mod.rs +++ b/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, - 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,