Skip to content

Commit

Permalink
fix: increase the number of bitcoin in flight requests to 1000. (#3768)
Browse files Browse the repository at this point in the history
Before, the maximum number of queued up blocks that were tried to be
downloaded was 100. The blocks are prioritized in a deterministic BFS
fashion (closest 100 to the anchor are tried to be downloaded). Because
testnet4 can have many (>100) forked blocks, this can lead to all
closest 100 blocks to be outside of the main chain. When asked for these
blocks, the peers simply timeout. This leads to the adapter not
downloading anything, and getting stuck, as in the next iteration, the
same 100 blocks will be retried.

After, the maximum number of blocks that are to be found in the BFS is
increased to 1000. The testnet4 does not currently reach that many
forked blocks, so this is a fine temporary fix. In the future, we ought
to find a more robust way of dealing with forked blocks, (black-list
timed-out blocks? DFS instead of BFS?).

Note that the maximum number of blocks returned in the `next` field of
the response is still 100.
  • Loading branch information
mihailjianu1 authored Feb 5, 2025
1 parent 8c3920c commit fbe09a6
Showing 1 changed file with 29 additions and 7 deletions.
36 changes: 29 additions & 7 deletions rs/bitcoin/adapter/src/get_successors_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@ const MAX_NEXT_BLOCK_HEADERS_LENGTH: usize = 100;
// We limit the number of blocks because serializing many blocks to pb can take some time.
const MAX_BLOCKS_LENGTH: usize = 100;

// The maximum number of get blocks requests that can be in-flight at any given time.
const MAX_IN_FLIGHT_BLOCKS: usize = 100;

// The maximum number of get blocks requests that can be in-flight at any given time.
// The number of blocks outside of the main chain easily exceeds 100 currently on testnet4.
// Setting this to 100 would prevent the adapter from making progress,
// as it gets stuck on these blocks, whose requests timeout indefinitely.
const TESTNET4_MAX_IN_FLIGHT_BLOCKS: usize = 1000;

const BLOCK_HEADER_SIZE: usize = 80;

// The maximum number of bytes the `next` field in a response can take.
Expand Down Expand Up @@ -109,7 +118,7 @@ impl GetSuccessorsHandler {
.processed_block_hashes
.observe(request.processed_block_hashes.len() as f64);

let response = {
let (blocks, next) = {
let state = self.state.lock().unwrap();
let anchor_height = state
.get_cached_header(&request.anchor)
Expand All @@ -128,19 +137,23 @@ impl GetSuccessorsHandler {
&request.anchor,
&request.processed_block_hashes,
&blocks,
self.network,
);
GetSuccessorsResponse { blocks, next }
(blocks, next)
};
let response_next = &next[..next.len().min(MAX_NEXT_BLOCK_HEADERS_LENGTH)];
let response = GetSuccessorsResponse {
blocks,
next: response_next.to_vec(),
};
self.metrics
.response_blocks
.observe(response.blocks.len() as f64);

if !response.next.is_empty() {
if !next.is_empty() {
// TODO: better handling of full channel as the receivers are never closed.
self.blockchain_manager_tx
.try_send(BlockchainManagerRequest::EnqueueNewBlocksToDownload(
response.next.clone(),
))
.try_send(BlockchainManagerRequest::EnqueueNewBlocksToDownload(next))
.ok();
}
// TODO: better handling of full channel as the receivers are never closed.
Expand Down Expand Up @@ -220,11 +233,15 @@ fn get_successor_blocks(
}

/// Get the next headers for blocks that may possibly be sent in upcoming GetSuccessor responses.
/// This returns the first MAX_IN_FLIGHT_BLOCKS / TESTNET4_MAX_IN_FLIGHT_BLOCKS headers from the anchor in BFS fashion
/// which are not in processed nor are they in the current response. Essentially representing the
/// blocks that should be returned in the next response.
fn get_next_headers(
state: &BlockchainState,
anchor: &BlockHash,
processed_block_hashes: &[BlockHash],
blocks: &[Arc<Block>],
network: Network,
) -> Vec<BlockHeader> {
let seen: HashSet<BlockHash> = processed_block_hashes
.iter()
Expand All @@ -237,9 +254,14 @@ fn get_next_headers(
.map(|c| c.children.iter().collect())
.unwrap_or_default();

let max_in_flight_blocks = match network {
Network::Testnet4 => TESTNET4_MAX_IN_FLIGHT_BLOCKS,
_ => MAX_IN_FLIGHT_BLOCKS,
};

let mut next_headers = vec![];
while let Some(block_hash) = queue.pop_front() {
if next_headers.len() >= MAX_NEXT_BLOCK_HEADERS_LENGTH {
if next_headers.len() >= max_in_flight_blocks {
break;
}

Expand Down

0 comments on commit fbe09a6

Please sign in to comment.