diff --git a/Cargo.lock b/Cargo.lock index 09dda9f6d..d8c6979e7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -407,7 +407,6 @@ dependencies = [ "metrics", "metrics-exporter-prometheus", "parking_lot", - "proptest", "quanta", "rayon", "rustix", diff --git a/bin/bitcoin-rs/tests/head_sync_e2e.rs b/bin/bitcoin-rs/tests/head_sync_e2e.rs index 7b12fd877..675a4dd5f 100644 --- a/bin/bitcoin-rs/tests/head_sync_e2e.rs +++ b/bin/bitcoin-rs/tests/head_sync_e2e.rs @@ -13,20 +13,17 @@ #![expect(clippy::expect_used, reason = "process test assertions")] -#[path = "support/clean_stderr.rs"] -mod clean_stderr; - use std::time::{Duration, Instant}; use bitcoin::p2p::message::NetworkMessage; use bitcoin::p2p::message_blockdata::Inventory; use bitcoin_rs_e2e::helpers::{ - best_hash, block_count, build_chain, connection_count, genesis_block, wait_for, + assert_clean_stderr, best_hash, block_count, build_chain, connection_count, genesis_block, + wait_for, }; use bitcoin_rs_e2e::live_peer::LivePeer; use bitcoin_rs_e2e::live_peer::pump_until_tip; use bitcoin_rs_e2e::{Error, Kind, ProcessNode}; -use clean_stderr::assert_clean_stderr; /// T1+T2: a block announced by `inv` is availability, not a body order: the /// node fetches headers first, and the admitted tip's body rides a window diff --git a/bin/bitcoin-rs/tests/live_head_carried_e2e.rs b/bin/bitcoin-rs/tests/live_head_carried_e2e.rs index ce02d4cc4..408549d48 100644 --- a/bin/bitcoin-rs/tests/live_head_carried_e2e.rs +++ b/bin/bitcoin-rs/tests/live_head_carried_e2e.rs @@ -13,20 +13,17 @@ #![expect(clippy::expect_used, reason = "process test assertions")] -#[path = "support/clean_stderr.rs"] -mod clean_stderr; - use std::time::{Duration, Instant}; use bitcoin::p2p::message::NetworkMessage; use bitcoin::p2p::message_blockdata::Inventory; use bitcoin_rs_e2e::helpers::{ - best_hash, block_count, build_chain, connection_count, genesis_block, wait_for, + assert_clean_stderr, best_hash, block_count, build_chain, connection_count, genesis_block, + wait_for, }; use bitcoin_rs_e2e::live_peer::LivePeer; use bitcoin_rs_e2e::live_peer::pump_until_tip; use bitcoin_rs_e2e::{Error, Kind, ProcessNode}; -use clean_stderr::assert_clean_stderr; /// Pumps until a `getdata` requests `hash` (serving every request /// type-faithfully), up to `dur`. Returns true when the request was seen. diff --git a/bin/bitcoin-rs/tests/overhaul_external_miner.rs b/bin/bitcoin-rs/tests/overhaul_external_miner.rs index 4618348c1..8a76906f7 100644 --- a/bin/bitcoin-rs/tests/overhaul_external_miner.rs +++ b/bin/bitcoin-rs/tests/overhaul_external_miner.rs @@ -8,7 +8,7 @@ use bitcoin::consensus::encode::serialize_hex; use bitcoin_rs_e2e::helpers::{ COINBASE_MATURITY, assemble_block_from_template, mature_funding, op_true_script, spend_anyone, }; -use bitcoin_rs_e2e::{Kind, ProcessNode}; +use bitcoin_rs_e2e::{Kind, ProcessNode, ValueExt}; use serde_json::{Value, json}; const FEE_SATS: u64 = 10_000; @@ -35,7 +35,7 @@ fn external_miner_assembles_template_and_submits_block() -> TestResult { let template = node.rpc("getblocktemplate", &json!([{"rules": ["segwit"]}]))?; assert_eq!( - required_u64(&template, "height")?, + template.u64_field("height")?, u64::from(COINBASE_MATURITY) + 2, "template must extend the current tip" ); @@ -50,17 +50,17 @@ fn external_miner_assembles_template_and_submits_block() -> TestResult { ); let entry = &template_txs[0]; assert_eq!( - required_str(entry, "hash")?, + entry.str_field("hash")?, spend.compute_wtxid().to_string(), "rendered hash must be the spend wtxid" ); assert_eq!( - required_u64(entry, "fee")?, + entry.u64_field("fee")?, FEE_SATS, "rendered fee must match the spend fee" ); assert!( - required_u64(entry, "weight")? > 0, + entry.u64_field("weight")? > 0, "rendered weight must be positive" ); let depends = entry @@ -78,7 +78,7 @@ fn external_miner_assembles_template_and_submits_block() -> TestResult { ); let info = node.rpc("getblockchaininfo", &json!([]))?; - let tip_height = required_u64(&info, "blocks")?; + let tip_height = info.u64_field("blocks")?; assert_eq!( tip_height, u64::from(COINBASE_MATURITY) + 2, @@ -87,7 +87,7 @@ fn external_miner_assembles_template_and_submits_block() -> TestResult { let mempool = node.rpc("getmempoolinfo", &json!([]))?; assert_eq!( - required_u64(&mempool, "size")?, + mempool.u64_field("size")?, 0, "mempool must be empty after block inclusion" ); @@ -96,17 +96,3 @@ fn external_miner_assembles_template_and_submits_block() -> TestResult { let _ = node.stop(); Ok(()) } - -fn required_str<'a>(value: &'a Value, key: &str) -> TestResult<&'a str> { - value - .get(key) - .and_then(Value::as_str) - .ok_or_else(|| format!("template missing string {key}").into()) -} - -fn required_u64(value: &Value, key: &str) -> TestResult { - value - .get(key) - .and_then(Value::as_u64) - .ok_or_else(|| format!("template missing u64 {key}").into()) -} diff --git a/bin/bitcoin-rs/tests/support/clean_stderr.rs b/bin/bitcoin-rs/tests/support/clean_stderr.rs deleted file mode 100644 index f3b32822c..000000000 --- a/bin/bitcoin-rs/tests/support/clean_stderr.rs +++ /dev/null @@ -1,20 +0,0 @@ -//! Shared process-stderr assertion for the live-wire sync harnesses. - -use bitcoin_rs_e2e::ProcessNode; -use bitcoin_rs_e2e::helpers::node_stderr; - -/// Asserts the node's stderr shows no panic and no `PrevHashMismatch` — -/// `context` names where a mismatch would indicate commit churn. -pub(crate) fn assert_clean_stderr(node: &ProcessNode, context: &str) { - let stderr = node_stderr(node); - assert_eq!( - stderr.matches("panic").count(), - 0, - "node stderr contains a panic" - ); - assert_eq!( - stderr.matches("PrevHashMismatch").count(), - 0, - "node stderr shows PrevHashMismatch: {context}" - ); -} diff --git a/bin/bitcoin-rs/tests/support/dependency_graph.rs b/bin/bitcoin-rs/tests/support/dependency_graph.rs index a090add6d..5d731cd14 100644 --- a/bin/bitcoin-rs/tests/support/dependency_graph.rs +++ b/bin/bitcoin-rs/tests/support/dependency_graph.rs @@ -83,13 +83,13 @@ struct FeatureDependency { /// Parsed workspace dependency graph used by the gates. pub(crate) struct WorkspaceGraph { /// Normal `bitcoin-rs-*` dependencies per crate. - pub normal_deps: BTreeMap>, + normal_deps: BTreeMap>, /// Storage engine dependencies per crate. - pub engine_deps: BTreeMap>, + engine_deps: BTreeMap>, /// External ZMQ implementation dependencies per crate. - pub zmq_deps: BTreeMap>, + zmq_deps: BTreeMap>, /// Cargo feature implies per crate. - pub features: BTreeMap>>, + features: BTreeMap>>, /// Feature selections on normal/build workspace edges, excluding dev fixtures. production_deps: BTreeMap>, /// Number of workspace packages seen in the metadata. diff --git a/clippy.toml b/clippy.toml index 813af339c..5486318a5 100644 --- a/clippy.toml +++ b/clippy.toml @@ -2,8 +2,3 @@ msrv = "1.95.0" cognitive-complexity-threshold = 15 type-complexity-threshold = 250 too-many-arguments-threshold = 8 -disallowed-types = [ - { path = "std::sync::Mutex", reason = "use parking_lot::Mutex" }, - { path = "std::sync::RwLock", reason = "use parking_lot::RwLock" }, - { path = "std::collections::HashMap", reason = "use hashbrown::HashMap or hashbrown::HashTable" }, -] diff --git a/crates/node/Cargo.toml b/crates/node/Cargo.toml index aeace6ee3..16983e627 100644 --- a/crates/node/Cargo.toml +++ b/crates/node/Cargo.toml @@ -30,25 +30,24 @@ workspace = true # selection: the engine a run uses is `validation.engine` at runtime. The # default is kernel-free so the C++ toolchain is opt-in. default = ["fjall", "zmq"] +# Component crates own the storage-backend fan-out: each of chainstate's and +# index's backend features already forwards to storage + utxo under the same +# feature name, so listing storage/utxo here duplicates those edges. rocksdb = [ "bitcoin-rs-chainstate/rocksdb", "bitcoin-rs-index/rocksdb", - "bitcoin-rs-storage/rocksdb", - "bitcoin-rs-utxo/rocksdb", ] fjall = [ "bitcoin-rs-chainstate/fjall", "bitcoin-rs-index/fjall", - "bitcoin-rs-storage/fjall", - "bitcoin-rs-utxo/fjall", ] redb = [ "bitcoin-rs-chainstate/redb", "bitcoin-rs-index/redb", - "bitcoin-rs-storage/redb", - "bitcoin-rs-utxo/redb", ] -kernel = ["bitcoin-rs-chainstate/kernel", "bitcoin-rs-consensus/kernel"] +# chainstate's kernel feature is the kernel-capability propagator for the +# chain stack and already forwards to `bitcoin-rs-consensus/kernel`. +kernel = ["bitcoin-rs-chainstate/kernel"] zmq = ["bitcoin-rs-rpc/zmq"] [dependencies] @@ -90,7 +89,6 @@ tempfile.workspace = true bitcoin-rs-chain = { workspace = true, features = ["test-seam"] } bitcoin-rs-p2p = { workspace = true, features = ["test-seam"] } bitcoin-rs-chainstate = { workspace = true, features = ["test-seam"] } -proptest.workspace = true criterion.workspace = true sonic-rs.workspace = true serde_json.workspace = true diff --git a/crates/node/README.md b/crates/node/README.md index 44faf7604..694d19daa 100644 --- a/crates/node/README.md +++ b/crates/node/README.md @@ -1,8 +1,6 @@ - `kernel`: compiles bitcoinkernel support in (`bitcoin-rs-consensus/kernel`). Selection is the runtime `validation.engine` setting (`native` by default); the feature alone never routes consensus verification to the kernel. -- `prometheus-http`: enables the `metrics-exporter-prometheus/http-listener` feature; - the production listener itself is controlled by `metrics_bind`. Part of [`bitcoin-rs`](../../README.md); see [`CONCEPTS.md`](../../CONCEPTS.md) for the project vocabulary. diff --git a/crates/rpc/README.md b/crates/rpc/README.md index 74fa0604c..37a7ca635 100644 --- a/crates/rpc/README.md +++ b/crates/rpc/README.md @@ -47,7 +47,7 @@ no backend cargo feature (`g17_dependency_direction` proves both from ### 4. Non-blocking event notifications - **ZMQ Framing**: ZeroMQ notifications (`ZmqPublisher` in `crates/rpc/src/zmq.rs`) emit 3-part multipart frames `[topic, body, 4-byte LE sequence]`. - **Non-Blocking Delivery**: Socket writes must use non-blocking sends (`zmq::DONTWAIT`). Notification buffer saturation must drop messages at the high-water mark rather than stalling block validation or consensus execution. -- **Reorg Sequencing & Notification Order**: Chain-transition rollback and admission orchestration in `crates/node/src/chain_effects.rs` guarantees block disconnect events (`D`, published during rollback) are emitted before block connect events (`C`, published by `after_connect`). +- **Reorg Sequencing & Notification Order**: Chain-transition rollback and admission orchestration in `crates/node/src/chain_effects.rs` guarantees block disconnect events (`D`, published during rollback) are emitted before block connect events (`C`, published by `on_connect`). ### 5. Architectural guardrails - **No Generic Middleware**: Do not introduce heavy async web framework stacks (Axum, Actix, Tower) into `RpcServer`. diff --git a/deny.toml b/deny.toml index e77d4da8c..72c96edbc 100644 --- a/deny.toml +++ b/deny.toml @@ -8,7 +8,6 @@ no-default-features = false [advisories] version = 2 yanked = "deny" -ignore = [] [licenses] version = 2 diff --git a/docs/policies/p2p-compatibility.md b/docs/policies/p2p-compatibility.md index 322a0e48e..d41bcfb36 100644 --- a/docs/policies/p2p-compatibility.md +++ b/docs/policies/p2p-compatibility.md @@ -156,14 +156,14 @@ TXR-09 is the trickled inventory schedule, `m_next_inv_send_time` at connection policy, no `getaddr` response, and no addr/addrv2 gossip. Outbound peer discovery runs through DNS-seed bootstrap (on by default: `run_dns_peer_maintenance`, `crates/p2p/src/service.rs`, seeds from - `Network::dns_seeds`) and the configured `--connect`/`--addnode` - surfaces. + `Network::dns_seeds`), the configured `--connect` peers, and the + `addnode` RPC. 5. **Service bits**: the advertised set follows storage (`init.cpp:2022-2026`): `NETWORK | WITNESS` normally, `NETWORK_LIMITED | WITNESS` when `storage.prune_target_mb > 0`, so a pruned node never claims a full block history. No `NODE_BLOOM` or `NODE_COMPACT_FILTERS` — those services do not exist here. 6. **Timestamp**: `version.timestamp` is always 0 (§4). 7. **Automatic misbehavior bans** (§6) absent; manual bans only. 8. **Chain-sync timeout scope**: a full-relay outbound connection that stops bringing a better chain is timed out as Core does (`ConsiderEviction`, `net_processing.cpp:5498-5550`), with one `getheaders` probe at 20 minutes (`CHAIN_SYNC_TIMEOUT`, definition `net_processing.cpp:109`) and the first four outbound connections to reach the tip protected (`MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT`, definition `net_processing.cpp:107`). Protection and the timeout both key on a tip the peer actually handed us, never on the height its handshake claimed: Core reads `pindexBestKnownBlock` there, not `nStartingHeight` (use-site `net_processing.cpp:3203-3210`). An operator-pinned connection is exempt in both, as it is in Core: `IsOutboundOrBlockRelayConn()` excludes `ConnectionType::MANUAL` (`net_processing.cpp:5502`). Block-relay-only connections are exempt here; Core times out both outbound classes. A connection dialed for blocks alone is therefore never replaced by this timer. -9. **Download budgets**: bitcoin-rs bounds one sync at `PENDING_BUDGET = 256` in-flight bodies and `RECEIVED_BLOCK_BUDGET = 256` staged bodies (`crates/p2p/src/download_window.rs:56,60`), and stripes at `MAX_BLOCKS_IN_TRANSIT_PER_PEER = 16` once `MIN_PEERS_FOR_FANOUT = 8` eligible peers exist (`:109,118`), where Core runs one `BLOCK_DOWNLOAD_WINDOW = 1024` ahead of the last common block with the same 16 per peer (`net_processing.cpp:151,133`). The 256 depth is measured, not assumed: a bounded 0–150,000 daemon single-peer IBD run at this window was 1.52× the 128-block control (`crates/p2p/src/download_window.rs:52-53`). The shallower window is a bounded divergence kept by operator decision: it caps buffered bodies and re-request work per connection instead of matching Core's depth. -10. **Extra-peer selection**: once a stale tip needs no extra full-relay connection, bitcoin-rs retires the newest automatic full-relay outbound connection that sits one beyond the slots and is older than `MINIMUM_CONNECT_TIME` (`retire_extra_full_relay_connection`, `crates/p2p/src/service.rs:1086`). An operator-pinned connection is outside the count and the victim set both, as in Core: neither `IsFullOutboundConn()` nor `IsBlockOnlyConn()` includes `ConnectionType::MANUAL` (`net_processing.cpp:5558-5604`). Core's `EvictExtraOutboundPeers` (`net_processing.cpp:5604-5668`) instead retires the connection that announced a block longest ago, breaking a tie by dropping the most recently connected one. The retired count is the same; the retired connection is not. A pinned outbound peer therefore neither creates an excess nor stands as a victim, matching `IsFullOutboundConn`/`IsBlockOnlyConn` excluding `ConnectionType::MANUAL` (`net_processing.cpp:5558-5604`). +9. **Download budgets**: bitcoin-rs bounds one sync at `PENDING_BUDGET = 256` in-flight bodies and `RECEIVED_BLOCK_BUDGET = 256` staged bodies (`crates/p2p/src/download_window/policy.rs:54,58`), and stripes at `MAX_BLOCKS_IN_TRANSIT_PER_PEER = 16` once `MIN_PEERS_FOR_FANOUT = 8` eligible peers exist (`:107,116`), where Core runs one `BLOCK_DOWNLOAD_WINDOW = 1024` ahead of the last common block with the same 16 per peer (`net_processing.cpp:151,133`). The 256 depth is measured, not assumed: a bounded 0–150,000 daemon single-peer IBD run at this window was 1.52× the 128-block control (`crates/p2p/src/download_window/policy.rs:50-51`). The shallower window is a bounded divergence kept by operator decision: it caps buffered bodies and re-request work per connection instead of matching Core's depth. +10. **Extra-peer selection**: once a stale tip needs no extra full-relay connection, bitcoin-rs retires the newest automatic full-relay outbound connection that sits one beyond the slots and is older than `MINIMUM_CONNECT_TIME` (`retire_extra_full_relay_connection`, `crates/p2p/src/service.rs:976`). An operator-pinned connection is outside the count and the victim set both, as in Core: neither `IsFullOutboundConn()` nor `IsBlockOnlyConn()` includes `ConnectionType::MANUAL` (`net_processing.cpp:5558-5604`). Core's `EvictExtraOutboundPeers` (`net_processing.cpp:5604-5668`) instead retires the connection that announced a block longest ago, breaking a tie by dropping the most recently connected one. The retired count is the same; the retired connection is not. A pinned outbound peer therefore neither creates an excess nor stands as a victim, matching `IsFullOutboundConn`/`IsBlockOnlyConn` excluding `ConnectionType::MANUAL` (`net_processing.cpp:5558-5604`). 11. **Unanswered ping**: Core sends one ping per `PING_INTERVAL` and remembers the nonce it asked for; when the pong has not arrived by `TIMEOUT_INTERVAL` after that ping, `MaybeSendPing` ends the connection regardless of any other traffic (`net_processing.cpp:5698-5712`). bitcoin-rs probes on the same cadence and ends a connection when either direction has been silent for `TIMEOUT_INTERVAL`, but it credits any inbound message as receive activity and keeps no outstanding-ping record, so a peer that never answers a probe while other traffic continues is not retired by that rule here. 12. **Inbound admission**: bitcoin-rs refuses an inbound socket once the live inbound count reaches `max_peer_connections - outbound_full_relay_slots - outbound_block_relay_slots` (default `200 - 8 - 2 = 190`, `net.h:1124-1127`), closing the stream before a handshake lease exists (`crates/p2p/src/listener.rs`, `PeerTable::try_register_inbound`). Core derives the same remainder and then scores an eviction (`AttemptToEvictConnection`, `net.cpp:1695-1735`) to make room. The eviction scoring is deliberately not implemented: no bitcoin-rs sync path depends on being able to displace an inbound peer, the resource-exhaustion defect closes at the admission boundary, and adding a second peer-selection policy would need an acceptance requirement it does not have. The operator-visible consequence is that the 191st inbound connection is refused rather than replacing a chosen peer. diff --git a/e2e/src/helpers.rs b/e2e/src/helpers.rs index 414b0ca7e..df3a836b6 100644 --- a/e2e/src/helpers.rs +++ b/e2e/src/helpers.rs @@ -517,3 +517,19 @@ pub fn wait_for(dur: Duration, check: &mut dyn FnMut() -> bool) -> bool { pub fn node_stderr(node: &ProcessNode) -> String { std::fs::read_to_string(node.evidence.join("stderr.log")).unwrap_or_default() } + +/// Asserts the node's stderr shows no panic and no `PrevHashMismatch` — +/// `context` names where a mismatch would indicate commit churn. +pub fn assert_clean_stderr(node: &ProcessNode, context: &str) { + let stderr = node_stderr(node); + assert_eq!( + stderr.matches("panic").count(), + 0, + "node stderr contains a panic" + ); + assert_eq!( + stderr.matches("PrevHashMismatch").count(), + 0, + "node stderr shows PrevHashMismatch: {context}" + ); +} diff --git a/e2e/src/node.rs b/e2e/src/node.rs index c39b4e1b0..2a989b262 100644 --- a/e2e/src/node.rs +++ b/e2e/src/node.rs @@ -1,7 +1,7 @@ //! Process custody for `bitcoin-rs` and pinned Bitcoin Core nodes. use std::collections::VecDeque; -use std::fs::{self, File}; +use std::fs::{self, File, OpenOptions}; use std::io::{Read, Write as _}; use std::net::{SocketAddr, TcpListener}; use std::path::{Path, PathBuf}; @@ -769,32 +769,64 @@ impl Drop for ProcessNode { /// Retains the newest `MAX_OUTPUT` bytes of a child's stream — the tail is /// where a late crash or error loop actually shows up; the head is least -/// diagnostic. The tail is materialized to `file` at EOF. +/// diagnostic. The tail is mirrored to `file` as it is captured so evidence +/// readers see output while the child is still running. fn capture_output(mut reader: impl Read + Send + 'static, file: PathBuf) -> JoinHandle<()> { std::thread::spawn(move || { - let Ok(mut file) = File::create(&file) else { + let path = file; + let Ok(mut file) = File::create(&path) else { return; }; let mut tail: VecDeque = VecDeque::new(); let limit = usize::try_from(MAX_OUTPUT).unwrap_or(usize::MAX); + // File bytes already dropped from `tail`; once they reach `limit` the + // file (stale head + live tail) is compacted back to the tail. + let mut stale = 0_usize; let mut buffer = [0_u8; 8192]; loop { match reader.read(&mut buffer) { Ok(0) | Err(_) => break, Ok(count) => { tail.extend(buffer[..count].iter().copied()); + let _ = file.write_all(&buffer[..count]); let excess = tail.len().saturating_sub(limit); if excess > 0 { tail.drain(..excess); + stale += excess; } + if stale >= limit && publish_tail(&path, tail.make_contiguous()).is_ok() { + // The rename orphaned `file` onto the old inode — reopen + // the published path to keep appending to it. + if let Ok(fresh) = OpenOptions::new().append(true).open(&path) { + file = fresh; + } + stale = 0; + } + let _ = file.flush(); } } } - let _ = file.write_all(tail.make_contiguous()); + // Rest state: file is exactly the retained tail, honoring MAX_OUTPUT. + if stale > 0 { + let _ = publish_tail(&path, tail.make_contiguous()); + } let _ = file.flush(); }) } +/// Writes `bytes` to a side file, then atomically renames it over `path`: a +/// concurrent reader of the log always sees one complete generation — never +/// a truncation window between `set_len(0)` and a rewrite. +fn publish_tail(path: &Path, bytes: &[u8]) -> std::io::Result<()> { + let side = path.with_extension("tmp"); + { + let mut tmp = File::create(&side)?; + tmp.write_all(bytes)?; + tmp.flush()?; + } + fs::rename(&side, path) +} + #[cfg(test)] mod tests { use super::*; diff --git a/scripts/import_qa_assets.py b/scripts/import_qa_assets.py index 20b7c1ad1..dcdbbac3f 100644 --- a/scripts/import_qa_assets.py +++ b/scripts/import_qa_assets.py @@ -65,12 +65,33 @@ def _emit(output: Path, seed: bytes) -> None: def _mask_rust_raw_strings(text: str) -> str: """Hide raw-string bodies from regexes that locate Rust declarations.""" raw_start = re.compile(r'(?:b|c)?r(#{0,255})"') + # 'x', '\'', '\n', '\u{41}', b'x' — a bare " inside must not toggle in_string. + char_literal = re.compile(r"'(?:\\u\{[0-9a-fA-F_]{1,6}\}|\\.|[^'\\])'") out: list[str] = [] index = 0 + in_string = False while index < len(text): + if in_string: + char = text[index] + out.append(char) + index += 1 + if char == "\\" and index < len(text): + out.append(text[index]) + index += 1 + elif char == '"': + in_string = False + continue raw = raw_start.match(text, index) if raw is None: - out.append(text[index]) + if text[index] == "'": + literal = char_literal.match(text, index) + if literal is not None: + out.append(literal.group(0)) + index = literal.end() + continue + char = text[index] + out.append(char) + in_string = char == '"' index += 1 continue hashes = raw.group(1) @@ -85,16 +106,39 @@ def _mask_rust_raw_strings(text: str) -> str: def _commands(source: Path) -> dict[str, bytes]: - text = _mask_rust_raw_strings(_strip_rust_comments(source.read_text())) + stripped = _strip_rust_comments(source.read_text()) + # The mask preserves offsets (raw-string bodies -> spaces, newlines kept), so + # a span found in the masked text indexes the same range in `stripped`. + masked = _mask_rust_raw_strings(stripped) table = re.search( - r"pub\s+const\s+COMMANDS\s*:\s*&\[Command\]\s*=\s*&\[(.*?)\];", - text, re.S, + r"pub\s+const\s+COMMANDS\s*:\s*&\[&str\]\s*=\s*&\[(.*?)\];", + masked, re.S, ) if table is None: raise ValueError("Cannot find the P2P COMMANDS inventory") - commands = re.findall(r'\bname\s*:\s*"([a-z0-9]{1,12})"', table.group(1)) - if (not commands or len(commands) > 256 or len(set(commands)) != len(commands) - or len(commands) != len(re.findall(r"\bCommand\s*\{", table.group(1)))): + table_source = stripped[table.start(1):table.end(1)] + # Sequential parse: every comma-separated entry must be one complete + # literal — r#"x"junk"# is not the entry "x" and must not parse as it. + literal = re.compile( + r'(?:(b|c)?r(#{0,255})"([^"]*)"\2|(?:b|c)?"([^"]*)")' + ) + whitespace = re.compile(r"\s*") + name = re.compile(r"[a-z0-9]{1,12}") + commands: list[str] = [] + pos = whitespace.match(table_source).end() + while pos < len(table_source): + entry = literal.match(table_source, pos) + value = entry and (entry.group(3) or entry.group(4)) + if value is None or name.fullmatch(value) is None: + raise ValueError("Invalid P2P COMMANDS inventory") + commands.append(value) + pos = whitespace.match(table_source, entry.end()).end() + if pos >= len(table_source): + break + if table_source[pos] != ",": + raise ValueError("Invalid P2P COMMANDS inventory") + pos = whitespace.match(table_source, pos + 1).end() + if not commands or len(commands) > 256 or len(set(commands)) != len(commands): raise ValueError("Invalid P2P COMMANDS inventory") return {name: bytes([index]) for index, name in enumerate(commands)} diff --git a/scripts/install-bitcoind.sh b/scripts/install-bitcoind.sh index ec659e548..954ad2118 100755 --- a/scripts/install-bitcoind.sh +++ b/scripts/install-bitcoind.sh @@ -5,8 +5,8 @@ # against the hardcoded SHA-256, and extracts bitcoind. Prints the bitcoind # path on stdout (log lines go to stderr). # -# eval "$(scripts/install-bitcoind.sh --export)" # scripts/install-bitcoind.sh --print-path +# eval "$(scripts/install-bitcoind.sh --export)" # exports BITCOIND_COMMAND # # Owner: docs/contracts/core-differential.md (CORE-01). diff --git a/scripts/tests/test_import_qa_assets.py b/scripts/tests/test_import_qa_assets.py index 6ac9aeab4..874bfab15 100644 --- a/scripts/tests/test_import_qa_assets.py +++ b/scripts/tests/test_import_qa_assets.py @@ -56,9 +56,9 @@ def setUp(self): for path in (self.p2p, self.scripts, self.asm, self.inventory.parent, self.script_target.parent): path.mkdir(parents=True, exist_ok=True) self.inventory.write_text('''// COMMANDS documentation mentions "decoy". -pub const COMMANDS: &[Command] = &[ - Command { name: "ping", status: CommandStatus::Served }, - Command { name: "pong", status: CommandStatus::Ignored }, +pub const COMMANDS: &[&str] = &[ + "ping", + "pong", ]; pub const CORE_UNTYPED_COMMANDS: &[&str] = &["outside"]; ''') @@ -96,8 +96,8 @@ def test_inventory_order_and_non_inventory_strings(self): self.assert_seed("p2p_message", b"\x01payload") def test_missing_or_ambiguous_inventory_fails_closed(self): - for inventory in ("let x = bitcoin_rs_p2p::COMMANDS;", "pub const COMMANDS: &[Command] = &[];", - 'pub const COMMANDS: &[Command] = &[Command { name: "ping" }, Command { name: "ping" }];'): + for inventory in ("let x = bitcoin_rs_p2p::COMMANDS;", "pub const COMMANDS: &[&str] = &[];", + 'pub const COMMANDS: &[&str] = &["ping", "ping"];'): with self.subTest(inventory=inventory): self.inventory.write_text(inventory) with self.assertRaises(ValueError): diff --git a/scripts/tests/test_import_qa_assets_boundaries.py b/scripts/tests/test_import_qa_assets_boundaries.py index c5b9b3812..9f2ec4958 100644 --- a/scripts/tests/test_import_qa_assets_boundaries.py +++ b/scripts/tests/test_import_qa_assets_boundaries.py @@ -42,11 +42,11 @@ def write_message(self, name, payload=b""): (self.source / name).write_bytes(message) def test_commented_commands_do_not_shift_decoder_selection(self): - self.inventory.write_text('''pub const COMMANDS: &[Command] = &[ - // Command { name: "ghost" }, - Command { name: "verack" }, - /* nested /* Command { name: "phantom" }, */ ]; still a comment */ - Command { name: "ping" }, + self.inventory.write_text('''pub const COMMANDS: &[&str] = &[ + // "ghost", + "verack", + /* nested /* "phantom", */ ]; still a comment */ + "ping", ]; ''') self.write_message("ping", b"payload") @@ -57,7 +57,7 @@ def test_commented_commands_do_not_shift_decoder_selection(self): def test_raw_strings_before_commands_do_not_stop_inventory_scan(self): self.inventory.write_text('''const NOTE: &str = r#""/*"#; - pub const COMMANDS: &[Command] = &[Command { name: "ping" }]; + pub const COMMANDS: &[&str] = &["ping"]; ''') self.write_message("ping", b"payload") self.map_p2p() @@ -67,8 +67,7 @@ def test_raw_string_delimiters_preserve_live_command_selection(self): # Rust Reference raw-string grammar (matching hash-delimited terminators): # https://doc.rust-lang.org/reference/tokens.html#raw-string-literals # Strings before COMMANDS are fixture data, not inventory or comments. - table = ('pub const COMMANDS: &[Command] = &[' - 'Command { name: "ping" }, Command { name: "verack" }];') + table = 'pub const COMMANDS: &[&str] = &["ping", "verack"];' self.write_message("verack") for prefix in ("r", "br", "cr"): for count in (0, 1, 2, 3, 255): @@ -90,26 +89,25 @@ def test_raw_string_delimiters_preserve_live_command_selection(self): def test_unterminated_raw_literal_is_rejected(self): self.inventory.write_text('const BAD: &str = r##"not terminated;\n' - 'pub const COMMANDS: &[Command] = &[' - 'Command { name: "verack" }];') + 'pub const COMMANDS: &[&str] = &["verack"];') with self.assertRaisesRegex(ValueError, "raw string"): self.map_p2p() self.assertFalse(self.output.exists()) def test_header_only_command_becomes_a_selector_only_seed(self): - self.inventory.write_text('pub const COMMANDS: &[Command] = &[Command { name: "verack" }];') + self.inventory.write_text('pub const COMMANDS: &[&str] = &["verack"];') self.write_message("verack") self.map_p2p() self.assertEqual([path.read_bytes() for path in self.output.iterdir()], [b"\0"]) def test_one_byte_budget_supports_a_selector_only_seed(self): - self.inventory.write_text('pub const COMMANDS: &[Command] = &[Command { name: "verack" }];') + self.inventory.write_text('pub const COMMANDS: &[&str] = &["verack"];') self.write_message("verack") self.map_p2p(budget=1) self.assertEqual([path.read_bytes() for path in self.output.iterdir()], [b"\0"]) def test_truncated_envelopes_never_become_selector_only_seeds(self): - self.inventory.write_text('pub const COMMANDS: &[Command] = &[Command { name: "verack" }];') + self.inventory.write_text('pub const COMMANDS: &[&str] = &["verack"];') message = b"\0" * 4 + b"verack".ljust(12, b"\0") + b"\0" * 8 for length in range(len(message)): (self.source / str(length)).write_bytes(message[:length]) diff --git a/scripts/tests/test_import_qa_assets_provenance.py b/scripts/tests/test_import_qa_assets_provenance.py index ca6600786..d5162b25f 100644 --- a/scripts/tests/test_import_qa_assets_provenance.py +++ b/scripts/tests/test_import_qa_assets_provenance.py @@ -45,7 +45,7 @@ def setUp(self): (scripts / MAPPER.name).write_bytes(MAPPER.read_bytes()) inventory = self.root / "crates/p2p/src/compat.rs" inventory.parent.mkdir(parents=True) - inventory.write_text('pub const COMMANDS: &[Command] = &[Command { name: "ping" }];\n') + inventory.write_text('pub const COMMANDS: &[&str] = &["ping"];\n') harness = self.root / "fuzz/fuzz_targets/script_eval.rs" harness.parent.mkdir(parents=True) harness.write_bytes(OWNER_HARNESS.read_bytes())