Skip to content

Commit 758bb18

Browse files
committed
use simple impl
1 parent 9cd735f commit 758bb18

3 files changed

Lines changed: 19 additions & 129 deletions

File tree

crates/cfxcore/core/src/sync/message/new_block_hashes.rs

Lines changed: 19 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -29,76 +29,14 @@ impl Decodable for NewBlockHashes {
2929

3030
impl Handleable for NewBlockHashes {
3131
fn handle(self, ctx: &Context) -> Result<(), Error> {
32-
debug!("on_new_block_hashes, msg={:?}", self);
33-
34-
// Determine which block hashes are unknown to us, but only when at
35-
// least one consumer actually needs the result.
36-
//
37-
// Two consumers exist:
38-
// 1. The `log_block_source` logging path, which uses the unknown
39-
// hashes for approximate first-seen deduplication so that only the
40-
// first peer to propagate a block hash (before we download its
41-
// header) generates a [BLOCK_SOURCE] log entry.
42-
// 2. The header-request path (active only outside catch-up mode),
43-
// which filters out hashes whose headers we already have to avoid
44-
// redundant requests.
45-
//
46-
// When neither consumer is active — i.e. we are in catch-up mode with
47-
// logging disabled — the `block_header_by_hash` lookups are skipped
48-
// entirely and `unknown_hashes` stays empty (no allocation). This
49-
// restores the pre-logging performance characteristics: zero overhead
50-
// in catch-up mode.
51-
//
52-
// The result is computed once and shared by both consumers, avoiding
53-
// duplicate lookups. A `Vec<&H256>` is used instead of a `HashSet`
54-
// because the typical NewBlockHashes message contains only 1–2 hashes,
55-
// making linear iteration cheaper than HashSet allocation and hashing.
56-
//
57-
// Trade-off: this uses the existing block_header_by_hash lookup
58-
// (in-memory HashMap + DB fallback) for approximate first-seen
59-
// deduplication. It is not a strict first-seen guarantee: multiple
60-
// peers propagating the same hash within the header-download window
61-
// (typically milliseconds to a few seconds) each generate a log
62-
// entry. After the header is cached, all subsequent NewBlockHashes
63-
// for the same block are silently suppressed, which is the desired
64-
// behavior for tracking block propagation sources.
65-
let in_catch_up_mode = ctx.manager.catch_up_mode();
66-
67-
let unknown_hashes: Vec<&H256> = if ctx
68-
.manager
69-
.protocol_config
70-
.log_block_source
71-
|| !in_catch_up_mode
72-
{
73-
self.block_hashes
74-
.iter()
75-
.filter(|hash| {
76-
ctx.manager
77-
.graph
78-
.data_man
79-
.block_header_by_hash(hash)
80-
.is_none()
81-
})
82-
.collect()
83-
} else {
84-
Vec::new()
85-
};
86-
87-
if ctx.manager.protocol_config.log_block_source {
88-
let peer_addr = ctx
89-
.peer_addr
90-
.as_ref()
91-
.map(|s| s.as_str())
92-
.unwrap_or("unknown");
93-
for hash in &unknown_hashes {
94-
info!(
95-
"[BLOCK_SOURCE] hash={:#x} from_node={} from_addr={}",
96-
hash, ctx.node_id, peer_addr
97-
);
98-
}
99-
}
32+
debug!(
33+
"on_new_block_hashes, msg={:?} from_node={} from_addr={}",
34+
self,
35+
ctx.node_id,
36+
ctx.peer_addr.as_deref().unwrap_or("unknown")
37+
);
10038

101-
if in_catch_up_mode {
39+
if ctx.manager.catch_up_mode() {
10240
// If a node is in catch-up mode and we are not in test-mode, we
10341
// just simple ignore new block hashes.
10442
if ctx.manager.protocol_config.test_mode {
@@ -112,8 +50,18 @@ impl Handleable for NewBlockHashes {
11250
return Ok(());
11351
}
11452

115-
let headers_to_request: Vec<H256> =
116-
unknown_hashes.into_iter().cloned().collect();
53+
let headers_to_request = self
54+
.block_hashes
55+
.iter()
56+
.filter(|hash| {
57+
ctx.manager
58+
.graph
59+
.data_man
60+
.block_header_by_hash(&hash)
61+
.is_none()
62+
})
63+
.cloned()
64+
.collect::<Vec<_>>();
11765

11866
ctx.manager.request_block_headers(
11967
ctx.io,
@@ -126,52 +74,3 @@ impl Handleable for NewBlockHashes {
12674
Ok(())
12775
}
12876
}
129-
130-
#[cfg(test)]
131-
mod tests {
132-
use super::*;
133-
134-
#[test]
135-
fn rlp_round_trip_empty() {
136-
let original = NewBlockHashes {
137-
block_hashes: vec![],
138-
};
139-
let encoded = rlp::encode(&original);
140-
let decoded: NewBlockHashes = rlp::decode(&encoded).unwrap();
141-
assert_eq!(original, decoded);
142-
}
143-
144-
#[test]
145-
fn rlp_round_trip_single_hash() {
146-
let original = NewBlockHashes {
147-
block_hashes: vec![H256::from_low_u64_be(42)],
148-
};
149-
let encoded = rlp::encode(&original);
150-
let decoded: NewBlockHashes = rlp::decode(&encoded).unwrap();
151-
assert_eq!(original, decoded);
152-
}
153-
154-
#[test]
155-
fn rlp_round_trip_multiple_hashes() {
156-
let original = NewBlockHashes {
157-
block_hashes: vec![
158-
H256::from_low_u64_be(1),
159-
H256::from_low_u64_be(2),
160-
H256::from_low_u64_be(3),
161-
H256::random(),
162-
],
163-
};
164-
let encoded = rlp::encode(&original);
165-
let decoded: NewBlockHashes = rlp::decode(&encoded).unwrap();
166-
assert_eq!(original, decoded);
167-
}
168-
169-
#[test]
170-
fn rlp_decode_rejects_short_element() {
171-
// 0xc1 0x01 is an RLP list with one 1-byte element.
172-
// H256 requires 32 bytes, so this should fail to decode.
173-
let short_element: &[u8] = &[0xc1, 0x01];
174-
let result: Result<NewBlockHashes, _> = rlp::decode(short_element);
175-
assert!(result.is_err());
176-
}
177-
}

crates/cfxcore/core/src/sync/synchronization_protocol_handler.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -427,11 +427,6 @@ pub struct ProtocolConfiguration {
427427
pub check_status_genesis: bool,
428428

429429
pub pos_started_as_voter: bool,
430-
431-
/// Enable logging of NewBlockHashes source peer information (IP and
432-
/// NodeId). Designed for bootnode deployment to track block propagation.
433-
/// Default: false. Set to true in production via config to enable.
434-
pub log_block_source: bool,
435430
}
436431

437432
impl SynchronizationProtocolHandler {

crates/config/src/configuration.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -405,9 +405,6 @@ build_config! {
405405
(pos_fix_cip156_transition_view, (u64), u64::MAX)
406406
(dev_pos_private_key_encryption_password, (Option<String>), None)
407407
(pos_started_as_voter, (bool), true)
408-
// Enable logging of NewBlockHashes source peer (IP and NodeId) for
409-
// bootnode deployment to track block propagation. Default: false.
410-
(log_block_source, (bool), false)
411408

412409
// Light node section
413410
(ln_epoch_request_batch_size, (Option<usize>), None)
@@ -960,7 +957,6 @@ impl Configuration {
960957
.expect("set to genesis if none"),
961958
check_status_genesis: self.raw_conf.check_status_genesis,
962959
pos_started_as_voter: self.raw_conf.pos_started_as_voter,
963-
log_block_source: self.raw_conf.log_block_source,
964960
}
965961
}
966962

0 commit comments

Comments
 (0)