Skip to content

Commit 2608017

Browse files
authored
fix: reduce the bitcoin adapter maximum response size to 1MB for testnet4 (#3769)
Before, the total response of the `get_successors` rpc in the bitcoin adapter was 2MB, regardless of the bitcoin network. In the worst case, this lead to the response containing either : - ~100 medium 20KB blocks, each with many small transactions, or - 2 large 1MB blocks, each with really many small transactions Both of those cases made serialization take really long, and the `get_successors` request would exceed the [50ms timeout in consensus](https://github.com/dfinity/ic/blob/b7a0b3d3b896cdaabc06c0ee6cd13cd54c939e67/rs/config/src/bitcoin_payload_builder_config.rs#L12). After, the maximum response size is reduced to just 1MB for the testnet4 network. This speeds up the serialization, while not sacrificing the canister performance too much (on average, there will be 2x more requests to the adapter when the canister starts up, but after it reaches the tip, it anyway receives maximum 1 block per request). Note that the Bitcoin mainnet network can't suffer from the same problem because: 1. Its blocks are quicker to serialize due to the transactions being larger 2. There are no storm blocks / crazy difficulty variations on it, hence the adapter will always send 1 block per response (which is anyway enforced after block height [750_000](https://github.com/dfinity/ic/blob/b7a0b3d3b896cdaabc06c0ee6cd13cd54c939e67/rs/bitcoin/adapter/src/get_successors_handler.rs#L236)).
1 parent 8a5bdfd commit 2608017

File tree

4 files changed

+24
-6
lines changed

4 files changed

+24
-6
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rs/bitcoin/adapter/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ DEPENDENCIES = [
2222
"@crate_index//:serde_json",
2323
"@crate_index//:slog",
2424
"@crate_index//:slog-async",
25+
"@crate_index//:static_assertions",
2526
"@crate_index//:thiserror",
2627
"@crate_index//:tokio",
2728
"@crate_index//:tokio-socks",

rs/bitcoin/adapter/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ serde = { workspace = true }
3030
serde_json = { workspace = true }
3131
slog = { workspace = true }
3232
slog-async = { workspace = true }
33+
static_assertions = { workspace = true }
3334
thiserror = { workspace = true }
3435
tokio = { workspace = true }
3536
tokio-socks = "0.5.1"

rs/bitcoin/adapter/src/get_successors_handler.rs

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use std::{
55

66
use bitcoin::{block::Header as BlockHeader, Block, BlockHash, Network};
77
use ic_metrics::MetricsRegistry;
8+
use static_assertions::const_assert_eq;
89
use tokio::sync::mpsc::Sender;
910
use tonic::Status;
1011

@@ -21,6 +22,10 @@ use crate::{
2122
// for pagination on the replica side to work as expected.
2223
const MAX_RESPONSE_SIZE: usize = 2_000_000;
2324

25+
// Lower than mainnet's response size. The main reason is large serialization time
26+
// for large blocks.
27+
const TESTNET4_MAX_RESPONSE_SIZE: usize = 1_000_000;
28+
2429
// Max number of next block headers that can be returned in the `GetSuccessorsResponse`.
2530
const MAX_NEXT_BLOCK_HEADERS_LENGTH: usize = 100;
2631

@@ -38,6 +43,14 @@ const MAX_NEXT_BYTES: usize = MAX_NEXT_BLOCK_HEADERS_LENGTH * BLOCK_HEADER_SIZE;
3843
// Having this as a soft limit as necessary to prevent large blocks from stalling consensus.
3944
const MAX_BLOCKS_BYTES: usize = MAX_RESPONSE_SIZE - MAX_NEXT_BYTES;
4045

46+
const TESTNET4_MAX_BLOCKS_BYTES: usize = TESTNET4_MAX_RESPONSE_SIZE - MAX_NEXT_BYTES;
47+
48+
const_assert_eq!(MAX_NEXT_BYTES + MAX_BLOCKS_BYTES, MAX_RESPONSE_SIZE);
49+
const_assert_eq!(
50+
MAX_NEXT_BYTES + TESTNET4_MAX_BLOCKS_BYTES,
51+
TESTNET4_MAX_RESPONSE_SIZE
52+
);
53+
4154
// Max height for sending multiple blocks when connecting the Bitcoin mainnet.
4255
const MAINNET_MAX_MULTI_BLOCK_ANCHOR_HEIGHT: BlockHeight = 750_000;
4356

@@ -108,6 +121,7 @@ impl GetSuccessorsHandler {
108121
&request.anchor,
109122
&request.processed_block_hashes,
110123
allow_multiple_blocks,
124+
self.network,
111125
);
112126
let next = get_next_headers(
113127
&state,
@@ -151,6 +165,7 @@ fn get_successor_blocks(
151165
anchor: &BlockHash,
152166
processed_block_hashes: &[BlockHash],
153167
allow_multiple_blocks: bool,
168+
network: Network,
154169
) -> Vec<Arc<Block>> {
155170
let seen: HashSet<BlockHash> = processed_block_hashes.iter().copied().collect();
156171

@@ -162,6 +177,11 @@ fn get_successor_blocks(
162177
.map(|c| c.children.iter().collect())
163178
.unwrap_or_default();
164179

180+
let max_blocks_size = match network {
181+
Network::Testnet4 => TESTNET4_MAX_BLOCKS_BYTES,
182+
_ => MAX_BLOCKS_BYTES,
183+
};
184+
165185
// Compute the blocks by starting a breadth-first search.
166186
while let Some(block_hash) = queue.pop_front() {
167187
if !seen.contains(block_hash) {
@@ -170,7 +190,7 @@ fn get_successor_blocks(
170190
Some(block) => {
171191
let block_size = block.total_size();
172192
if response_block_size == 0
173-
|| (response_block_size + block_size <= MAX_BLOCKS_BYTES
193+
|| (response_block_size + block_size <= max_blocks_size
174194
&& successor_blocks.len() < MAX_BLOCKS_LENGTH
175195
&& allow_multiple_blocks)
176196
{
@@ -780,9 +800,4 @@ mod test {
780800
u32::MAX
781801
);
782802
}
783-
784-
#[test]
785-
fn response_size() {
786-
assert_eq!(MAX_NEXT_BYTES + MAX_BLOCKS_BYTES, MAX_RESPONSE_SIZE);
787-
}
788803
}

0 commit comments

Comments
 (0)