Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: don't clone blocks during get_successors #3687

Merged
merged 3 commits into from
Feb 5, 2025
Merged

fix: don't clone blocks during get_successors #3687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c5c6dd6
arc<block>
mihailjianu1 Jan 30, 2025
362fe47
comment #1
mihailjianu1 Feb 3, 2025
5266fb7
Merge branch 'master' into mihailjianu1/bitcoin-no-block-cloning
mihailjianu1 Feb 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions rs/bitcoin/adapter/src/blockchainstate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use bitcoin::{

use ic_btc_validation::{validate_header, HeaderStore, ValidateHeaderError};
use ic_metrics::MetricsRegistry;
use std::collections::HashMap;
use std::{collections::HashMap, sync::Arc};
use thiserror::Error;

use bitcoin::Work;
Expand Down Expand Up @@ -94,7 +94,7 @@ pub struct BlockchainState {
header_cache: HashMap<BlockHash, HeaderNode>,

/// This field stores a hashmap containing BlockHash and the corresponding Block.
block_cache: HashMap<BlockHash, Block>,
block_cache: HashMap<BlockHash, Arc<Block>>,

/// This field contains the known tips of the header cache.
tips: Vec<Tip>,
Expand Down Expand Up @@ -240,7 +240,7 @@ impl BlockchainState {
.add_header(block.header)
.map_err(AddBlockError::Header)?;
self.tips.sort_unstable_by(|a, b| b.work.cmp(&a.work));
self.block_cache.insert(block_hash, block);
self.block_cache.insert(block_hash, Arc::new(block));
self.metrics
.block_cache_size
.set(self.get_block_cache_size() as i64);
Expand Down Expand Up @@ -317,10 +317,10 @@ impl BlockchainState {
hashes
}

/// This method takes a list of block hashes as input.
/// For each block hash, if the corresponding block is stored in the `block_cache`, the cached block is returned.
pub fn get_block(&self, block_hash: &BlockHash) -> Option<&Block> {
self.block_cache.get(block_hash)
/// This method takes a block hash
/// If the corresponding block is stored in the `block_cache`, the cached block is returned.
pub fn get_block(&self, block_hash: &BlockHash) -> Option<Arc<Block>> {
self.block_cache.get(block_hash).cloned()
}

/// Used when the adapter is shutdown and no longer requires holding on to blocks.
Expand Down
6 changes: 3 additions & 3 deletions rs/bitcoin/adapter/src/get_successors_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub struct GetSuccessorsRequest {
#[derive(Clone, Eq, PartialEq, Debug)]
pub struct GetSuccessorsResponse {
/// Blocks found in the block cache.
pub blocks: Vec<Block>,
pub blocks: Vec<Arc<Block>>,
/// Next set of headers to be sent to the canister.
pub next: Vec<BlockHeader>,
}
Expand Down Expand Up @@ -151,7 +151,7 @@ fn get_successor_blocks(
anchor: &BlockHash,
processed_block_hashes: &[BlockHash],
allow_multiple_blocks: bool,
) -> Vec<Block> {
) -> Vec<Arc<Block>> {
let seen: HashSet<BlockHash> = processed_block_hashes.iter().copied().collect();

let mut successor_blocks = vec![];
Expand Down Expand Up @@ -204,7 +204,7 @@ fn get_next_headers(
state: &BlockchainState,
anchor: &BlockHash,
processed_block_hashes: &[BlockHash],
blocks: &[Block],
blocks: &[Arc<Block>],
) -> Vec<BlockHeader> {
let seen: HashSet<BlockHash> = processed_block_hashes
.iter()
Expand Down