Enhance node functionality with BIP support and sync improvements#2
Open
metaphorics wants to merge 38 commits into
Open
Enhance node functionality with BIP support and sync improvements#2metaphorics wants to merge 38 commits into
metaphorics wants to merge 38 commits into
Conversation
The node defaults its data directory to .bitcoin-rs, which leaves runtime smoke data in the repository root as untracked files. Ignore the root data directory alongside other generated local artifacts. ODIN-Gate: pass Verification: git check-ignore -v .bitcoin-rs .bitcoin-rs/ Verification: git ls-files .bitcoin-rs Verification: git status --short --ignored Op: compress
Wire DNS seed resolution into startup and bound the shared outbound peer admission path so DNS bootstrap and RPC addnode cannot create unbounded queue or dial fanout. The drain tracks active outbound connection threads and suppresses duplicate active or connected addresses before spawning. RPC addnode now fails fast under queue pressure instead of blocking or persisting a failed add. Verification: - cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check - cargo test -p bitcoin-rs-rpc --no-default-features --features rocksdb,fjall,redb,mdbx addnode - cargo test -p bitcoin-rs-node --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus dns_seed_bootstrap - cargo test -p bitcoin-rs-node --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus outbound_ - cargo test -p bitcoin-rs-p2p --no-default-features --features rocksdb,fjall,redb peer_manager_resolves_configured_seeds - cargo test -p bitcoin-rs-p2p --no-default-features --features rocksdb,fjall,redb system_dns_resolver_uses_configured_port_for_literal_hosts - cargo clippy --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings - cargo test --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus - adversarial final review PASS ODIN-Gate: pass Op: extend
Add a narrow addr() scanobject subset to scantxoutset and back it with a read-only UTXO-set scriptPubKey scan. Unsupported descriptors and malformed scanobjects fail instead of being silently ignored. Verification: - cargo fmt --all -- --check - cargo test -p bitcoin-rs-rpc --no-default-features --features "rocksdb,fjall,redb,mdbx" scantxoutset - cargo test -p bitcoin-rs-utxo --no-default-features scan_script_pubkeys_returns_matching_live_outputs -- --exact - cargo clippy --workspace --all-targets --no-default-features --features "rocksdb,fjall,redb,mdbx,bitcoinconsensus" -- -D warnings - cargo test --workspace --no-fail-fast --no-default-features --features "rocksdb,fjall,redb,mdbx,bitcoinconsensus" Op: extend
Correct headers-first sync so a fresh tree can only root the network genesis, block downloads walk forward from the applied tip, and block application extends the applied chain rather than the header tip. Bootstrap now applies the real genesis block before draining queued blocks or requesting block data, preserving applied_tip as a fully applied block cursor. Import rejection tests now seed connected fixtures so validation assertions reach their intended consensus rules after the applied-tip predecessor guard. Evidence: - cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check - FEATURES='rocksdb,fjall,redb,mdbx,bitcoinconsensus'; cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --all-targets --no-default-features --features "" -- -D warnings - FEATURES='rocksdb,fjall,redb,mdbx,bitcoinconsensus'; cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast --no-default-features --features "" Op: correct Restores: spec:headers-first sync keeps genesis height and downloads contiguous blocks after applied tip
Keep strict block application while making the sync layer tolerate P2P block delivery order. Received blocks are bounded, aged, and applied only when they match the next expected active-chain hash after the applied tip.\n\nVerification:\n- cargo fmt --manifest-path Cargo.toml --all -- --check\n- cargo test --manifest-path Cargo.toml -p bitcoin-rs-node --test sync_smoke --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus\n- cargo test --manifest-path Cargo.toml -p bitcoin-rs-node sync::tests --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus\n- cargo clippy --manifest-path Cargo.toml --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings\n- cargo test --manifest-path Cargo.toml --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus\n\nOp: correct\nRestores: spec:headers-first sync applies requested blocks in applied-tip order despite out-of-order P2P delivery
Match Bitcoin Core CScript integer encoding for the BIP34 coinbase-height prefix. Signet activates BIP34 at height 1, where Core expects OP_1 rather than raw pushdata 0101; live block 1 uses scriptSig 5151. Verification: - cargo fmt --manifest-path Cargo.toml --all -- --check - cargo test --manifest-path Cargo.toml -p bitcoin-rs-consensus --no-default-features --features bitcoinconsensus bip34 - cargo clippy --manifest-path Cargo.toml --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings - cargo test --manifest-path Cargo.toml --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus - live signet smoke: getblockcount advanced to 16 Op: correct Restores: spec:BIP34 coinbase height prefix matches Bitcoin Core CScript integer encoding on signet height 1
Add a raw-byte regression for the live signet height-one coinbase scriptSig prefix 5151 so the Core-compatible OP_1 BIP34 path is pinned without relying only on Builder-generated fixture bytes. Verification: - cargo fmt --manifest-path Cargo.toml --all -- --check - cargo test --manifest-path Cargo.toml -p bitcoin-rs-consensus --no-default-features --features bitcoinconsensus bip34 - cargo clippy --manifest-path Cargo.toml --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings - cargo test --manifest-path Cargo.toml --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus Op: correct Restores: test:signet_block_one_coinbase_prefix_passes
The bitcoin 0.32 Network enum is exhaustive for the five variants matched here, so the wildcard fallback is unreachable and makes kernel builds noisy. Remove it without changing any supported network mapping. Verification: - cargo fmt --manifest-path Cargo.toml --all -- --check - cargo test --manifest-path Cargo.toml -p bitcoin-rs-consensus --no-default-features --features kernel -- --include-ignored - cargo clippy --manifest-path Cargo.toml -p bitcoin-rs-consensus --all-targets --no-default-features --features kernel -- -D warnings Op: correct Restores: spec:kernel consensus sanity is warning-clean under the pinned bitcoin Network enum
Expose the mdbx backend feature from the index and filters crates and forward the node mdbx feature to both runtime index users. The node state module already instantiates txindex and filter index stores with MdbxStore; the Cargo feature graph needs to say that directly. Also make the node storage-backend invariant explicit: a node build with no compiled storage backend is invalid. That path was already unusable and led to cfg-stripped storage enums with confusing follow-on diagnostics. Verification: - cargo fmt --all -- --check - FEATURES="rocksdb,fjall,redb,mdbx,bitcoinconsensus"; cargo build -p bitcoin-rs --bin bitcoin-rs --no-default-features --features "$FEATURES" - cargo check -p bitcoin-rs-index --no-default-features --features mdbx - cargo check -p bitcoin-rs-filters --no-default-features --features mdbx - cargo check -p bitcoin-rs-node --no-default-features --features mdbx - FEATURES="rocksdb,fjall,redb,mdbx,bitcoinconsensus"; cargo check -p bitcoin-rs-node --no-default-features --features "$FEATURES" Op: correct Restores: spec:node mdbx runtime index feature graph matches MdbxStore use
Replace the ignored no-op G14 performance gate with a fail-closed verifier for externally collected mainnet evidence. The gate stays ignored because it does not run live IBD benchmarks itself; explicit runs now require scenario metadata, bind the evidence SHA to the current git HEAD, and enforce the documented G14 thresholds.\n\nVerification:\n- cargo fmt --all -- --check\n- cargo clippy --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings\n- cargo test --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus\n- G14 matrix: no-env/stale-sha/budget-breach/NaN fail; threshold evidence passes\n\nOp: correct\nRestores: spec:G14 performance budgets fail closed when explicit evidence is absent and reject supplied evidence outside PLAN.md thresholds
Add regression coverage for two contextual nBits rejection seams: block import rejects a non-retarget child with changed nBits and leaves the accepted tip unchanged, and header sync rejects a retarget-boundary header that keeps stale parent nBits when the computed retarget value differs. Op: correct Restores: spec:contextual nBits validation rejects non-retarget continuity and retarget-boundary mismatches before accepting the candidate
Add an exact gettxoutsetinfo(["muhash"]) assertion for the fresh empty UTXO set using Bitcoin Core's known empty MuHash digest. Op: correct Restores: spec:gettxoutsetinfo muhash exposes the Core-compatible empty UTXO-set digest
Convert the ignored G2 parity placeholder into a fail-closed verifier for externally collected bitcoin-rs and bitcoind MuHash samples. Evidence must bind to current HEAD, a clean tracked tree, mainnet IBD, bitcoind, MuHash, the 10,000-block cadence, and exact sample equality. Verification: - cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check - cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus - dirty-tree G2 ignored run fails closed on tracked modifications Op: correct Restores: spec:G2 full IBD MuHash parity gate fails closed when explicit evidence is absent and rejects stale, malformed, incomplete, or mismatched samples
Extend the existing out-of-order block sync smoke test to wire the UTXO listener the same way production handles do and assert the externally visible CoinStats snapshot after buffered block application. This is a bounded sync-boundary invariant test. It does not prove live mainnet IBD MuHash parity, spend/removal parity, restart persistence, reorg handling, or performance superiority. Verification: - cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check - cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus Op: correct Restores: spec:block-sync buffered apply publishes the same CoinStats/MuHash state through ApplyHandles that UTXO listener updates produce
Add a deterministic snapshot fixture for the UTXO removal listener path. The test commits two outputs, removes one, checks the listener CoinStats/MuHash state against direct CoinStats mutation, and checks the snapshot trailer uses the post-removal MuHash. This is a bounded UTXO/coinstats mutation invariant. It does not prove live block spend application, reorg handling, restart persistence, mainnet IBD parity, or performance superiority. Verification: - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --test snapshot_with_muhash - cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --test snapshot_with_muhash -- -D warnings - cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check - cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus Op: correct Restores: spec:coinstats snapshot trailers track UTXO listener MuHash after committed removals
Add a bounded BlockSync smoke test that applies a mature coinbase spend into a non-coinbase output, then spends that output, all through the real inbound header/block path. Assert the spent UTXOs disappear, the final output remains live, and the shared CoinStats/MuHash snapshot matches an independently replayed model with removals. Evidence: - cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check - cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --test sync_smoke --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --test sync_smoke --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus - cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus Op: correct Restores: spec:block-sync/apply_block spend removal updates shared UTXO and CoinStats/MuHash through listener path
Add an opt-in node-local G2 MuHash sampler that writes verifier-compatible sample evidence for applied blocks. Require the sample path and positive target tip together so interval-only files cannot masquerade as G2 evidence. Startup reads and validates existing samples before node storage/ZMQ setup. Runtime write failures stay telemetry-only after tip publication so sampler I/O cannot pretend the block apply rolled back. Evidence: - cargo fmt --all -- --check - cargo clippy --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings - cargo test --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus Op: extend
Add a bounded Bitcoin Core sample normalizer for the existing G2 verifier evidence contract. The helper calls an operator-managed bitcoin-cli gettxoutsetinfo muhash query for the required G2 heights and emits height:muhash text without starting bitcoind, running IBD, or changing verifier semantics. Verification: bash -n scripts/collect-g2-bitcoind-muhash-samples.sh; git diff --check; cargo fmt --all -- --check; cargo clippy --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings; cargo test --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus Op: extend
Limit the node mdbx feature to the backend forwards required by node-facing filters and index support. The broader runtime-backend forwarding was review sprawl, not a load-bearing part of this fix. Evidence: cargo fmt --all -- --check; cargo clippy --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings; cargo test --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus. Op: correct Restores: spec:node mdbx feature forwards only required filters/index backend support
Create the configured node data directory before opening the optional G2 MuHash sampler. Sample files under the data directory are valid configs; sampler validation must not reject them because the node data directory has not been created yet. Evidence: cargo fmt --all -- --check; cargo clippy --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings; cargo test --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus. Op: correct Restores: spec:G2 sample paths under node data_dir remain valid configs
Connect block transactions against an ordered block-local UTXO view so later non-coinbase transactions can spend earlier same-block outputs without admitting forward references, same-block coinbase spends, or stale BIP68 metadata. Net same-block created-and-spent outputs before committing the block changes. Evidence: cargo test -p bitcoin-rs-node --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus same_block --lib; cargo test -p bitcoin-rs-node --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus coinbase_maturity --lib; cargo test -p bitcoin-rs-node --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus bip68 --lib; cargo clippy --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings; cargo test --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus. Op: correct Restores: spec:block validation observes prior non-coinbase same-block outputs while preserving BIP68 and coinbase-maturity invariants
Reject coinbase transactions whose scriptSig length is outside the base CheckTransaction 2..=100 byte range. Wire the same invariant into node block transaction verification without running full coinbase prevout/script checks. Verification: - cargo test -p bitcoin-rs-consensus --no-default-features --features bitcoinconsensus coinbase_script_sig - cargo test -p bitcoin-rs-node --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus verify_block_transactions_rejects_bad_coinbase_script_sig - cargo fmt --all -- --check - cargo clippy --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings - cargo test --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus Op: correct Restores: spec: Bitcoin CheckTransaction coinbase scriptSig length 2..=100
Reject inbound headers payloads whose CompactSize count exceeds the Bitcoin protocol maximum before allocating or decoding header entries. Evidence: - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-p2p --no-default-features --test wire_codec -- --nocapture - cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check - cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus Op: correct Restores: spec: Bitcoin P2P headers message count <= 2000
Restore block-local transaction verification visibility for coinbase outputs after validating their scriptSig size. Same-block coinbase spends now pass the script/prevout phase and remain rejected by the coinbase maturity pass. Evidence: - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus verify_block_transactions_defers_same_block_coinbase_spend_to_maturity -- --nocapture - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus verify_block_transactions_rejects_bad_coinbase_script_sig -- --nocapture - cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check - cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus Op: correct Restores: spec: block-local transaction verification overlay includes earlier same-block coinbase outputs
Reject Core-style merkle tree mutations where equal sibling hashes are paired before odd-tail duplication. This matches CheckMerkleRoot's bad-txns-duplicate invariant without turning every duplicate txid into a non-contextual block-rule failure. Evidence: - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --no-default-features --features bitcoinconsensus duplicate_transaction_ids_are_rejected_even_with_matching_merkle_root -- --nocapture - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --no-default-features --features bitcoinconsensus duplicate_transaction_ids_without_merkle_mutation_reach_later_validation -- --nocapture - cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus - cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check - cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --no-default-features --features kernel -- --include-ignored Op: correct Restores: spec: CheckMerkleRoot rejects duplicate-subtree merkle mutations
Reject inventory-vector payloads above the existing MAX_INV_PER_MSG limit at the wire decoder before entry decoding. Covers inv, getdata, and notfound payloads that decode as Vec<Inventory>; the dispatch getdata cap remains as defense-in-depth. Op: correct Restores: spec: Bitcoin P2P inventory-vector messages are capped at 50,000 entries at the wire validation boundary
Reject getblocks requests whose block locator exceeds the existing MAX_LOCATOR_HASHES bound, mirroring the getheaders dispatch validation while keeping locator-length logic in one helper. Op: correct Restores: spec: inbound getblocks locator hashes are capped at MAX_LOCATOR_HASHES
Reject getheaders and getblocks locator payloads above MAX_LOCATOR_HASHES at the wire decoder before locator vector allocation. Keep dispatch locator checks as defense-in-depth and preserve the dispatch constant path via re-export. Op: correct Restores: spec: inbound locator-bearing P2P messages are capped at MAX_LOCATOR_HASHES at the wire validation boundary
Reject addr and addrv2 payloads above MAX_ADDR_MESSAGE_COUNT at the wire decoder before address vector allocation. This keeps address-relay behavior unchanged while enforcing the protocol message-size boundary. Op: correct Restores: spec: inbound addr and addrv2 messages are capped at 1000 address entries at the wire validation boundary
There was a problem hiding this comment.
Pull request overview
This PR strengthens node correctness and evidence-driven verification by adding G2/G14 “external evidence” gates, improving block/header sync robustness, extending RPC/UTXO functionality for script-based UTXO scanning, tightening consensus validation (coinbase scriptSig bounds + merkle mutation detection), and hardening P2P message decoding limits. It also bumps bitcoin-rs-p2p to 0.1.1 and introduces supporting tests and helper scripts.
Changes:
- Add externally-validated evidence gates for G2 (MuHash parity) and G14 (mainnet performance budgets), plus a helper script + smoke tests to collect/normalize Core MuHash samples.
- Improve node sync/apply correctness: bootstrap genesis when needed, buffer out-of-order blocks, refine filter generation for same-block spends, and add DNS seed bootstrap + bounded outbound queue behavior.
- Extend RPC/UTXO support: implement
scantxoutsetaddr()-based scanning backed by a new UTXOscan_script_pubkeysAPI.
Reviewed changes
Copilot reviewed 41 out of 44 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/collect-g2-bitcoind-muhash-samples.sh | Adds a helper to collect/normalize bitcoind MuHash samples at contract heights. |
| crates/utxo/tests/commit_roundtrip.rs | Adds a test covering new scriptPubKey scan behavior. |
| crates/utxo/src/shard.rs | Implements per-shard scriptPubKey scanning into an aggregate scan result. |
| crates/utxo/src/set.rs | Introduces UtxoScan/ScannedUtxo and set/view scan APIs. |
| crates/utxo/src/lib.rs | Re-exports new scan types from the utxo crate. |
| crates/rpc/tests/handler_smoke.rs | Adds RPC smoke coverage for empty MuHash and getblockfilter not-found behavior; adjusts fixture setup. |
| crates/rpc/src/handlers/wallet.rs | Implements addr()-only scantxoutset backed by UTXO script scanning + tests. |
| crates/rpc/src/handlers/network.rs | Switches addnode outbound enqueue to try_send with explicit full/disconnected errors + tests. |
| crates/rpc/src/handlers/chain.rs | Makes getblockfilter return NotFound when filter/header rows are missing. |
| crates/p2p/tests/wire_codec.rs | Adds decoder bound tests for inv/getdata/notfound/addr/addrv2/locator/headers. |
| crates/p2p/src/wire.rs | Enforces payload element caps when decoding several P2P messages; adds constants. |
| crates/p2p/src/peer.rs | Adds compact-block negotiation state and a system DNS resolver implementation + tests. |
| crates/p2p/src/lib.rs | Re-exports SystemDnsResolver. |
| crates/p2p/src/fsm.rs | Records inbound sendcmpct preferences during handshake/ready states + tests. |
| crates/p2p/src/dispatch.rs | Applies locator bounds to getheaders/getblocks at dispatch level + tests; re-exports locator cap. |
| crates/p2p/Cargo.toml | Bumps crate version to 0.1.1. |
| crates/node/tests/sync_smoke.rs | Adds sync tests for buffering out-of-order blocks, coin-stats/UTXO tracking, and G2 sample writing/config validation. |
| crates/node/tests/config_layered.rs | Adds layered-config tests for G2 MuHash sample path/tip-height precedence and validation. |
| crates/node/src/sync.rs | Buffers out-of-order blocks, bootstraps genesis on empty applied tip, and refines getdata selection/pending marking. |
| crates/node/src/state.rs | Adds bounded outbound queue, config validation on open, and wires in G2 MuHash sampling. |
| crates/node/src/run.rs | Adds DNS seed bootstrap worker and outbound connection drain controls (active limit, handle reaping). |
| crates/node/src/lib.rs | Adds the new internal g2_muhash module. |
| crates/node/src/import.rs | Refactors import tests to ensure genesis is imported first; adds nBits mismatch rejection test. |
| crates/node/src/g2_muhash.rs | Implements persistent G2 MuHash sample writer with contract validation and tests. |
| crates/node/src/config.rs | Adds g2_muhash_samples/g2_muhash_tip_height config options, layering, and validation. |
| crates/node/src/apply.rs | Improves applied-predecessor logic, same-block spend handling (UTXO view/filter/changes), coinbase scriptSig validation, and optional G2 sampling. |
| crates/node/Cargo.toml | Adjusts backend feature wiring (notably mdbx feature list) and adds tempfile dev-dependency. |
| crates/index/Cargo.toml | Adds an mdbx feature mapping to storage mdbx. |
| crates/filters/Cargo.toml | Adds an mdbx feature mapping to storage mdbx. |
| crates/consensus/src/verify_tx.rs | Adds consensus check for coinbase scriptSig size bounds + tests. |
| crates/consensus/src/verify_block.rs | Implements explicit merkle-root calculation + merkle mutation detection + tests. |
| crates/consensus/src/lib.rs | Exposes new verify function and introduces ConsensusError::MerkleMutation and CoinbaseScriptSigSize. |
| crates/consensus/src/kernel.rs | Makes kernel chain-type mapping explicit for all bitcoin::Network variants. |
| crates/consensus/src/bip34.rs | Uses script::Builder::push_int for BIP34 height-prefix validation; expands tests for small-height encodings. |
| crates/coinstats/tests/snapshot_with_muhash.rs | Refactors txout helper and adds removal tracking test for MuHash/snapshot trailer. |
| crates/chain/tests/header_sync_roundtrip.rs | Expands header-sync tests: empty-tree root rejection, retarget nBits mismatch, and genesis inclusion handling. |
| crates/chain/src/header_sync.rs | Prevents accepting non-genesis headers as the first header in an empty BlockTree. |
| Cargo.toml | Bumps workspace bitcoin-rs-p2p dependency version to 0.1.1. |
| Cargo.lock | Updates lockfile for bitcoin-rs-p2p 0.1.1 and adds tempfile where needed. |
| bin/bitcoin-rs/tests/gates/g14_perf_budgets.rs | Adds an ignored gate that validates external mainnet performance evidence via env contract. |
| bin/bitcoin-rs/tests/gates/g02_ibd_muhash_parity.rs | Adds an ignored gate that validates external MuHash parity evidence via env contract. |
| bin/bitcoin-rs/tests/g2_bitcoind_muhash_script.rs | Adds smoke tests for the MuHash collection script (format, normalization, error paths). |
| bin/bitcoin-rs/Cargo.toml | Adds tempfile dev-dependency for new script smoke tests. |
| .gitignore | Ignores /.bitcoin-rs/ directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6082045535
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Fix stale block-buffer pruning on idle ticks, enforce scantxoutset scanobject contracts, preserve addnode pinning when the outbound queue is full, and gate Unix-only G2 helper tests. Op: correct Restores: spec:PR #2 review-comment contracts for sync buffer pruning, scantxoutset inputs, addnode persistence, and Unix-only helper tests
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fea2b4ced
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
Author
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly strengthens the node's consensus and synchronization logic by introducing stricter Merkle root mutation checks, coinbase scriptSig size validation, and an out-of-order block buffer. It also implements MuHash sampling for IBD parity verification, address-based UTXO scanning for the scantxoutset RPC, and DNS seed bootstrapping for the P2P layer. The review feedback focuses on optimizing the synchronization logic by reducing redundant locking and pruning operations within the block buffer and ensuring that block header statuses are correctly updated to Active upon successful application.
Enable bitcoin-rs-storage/mdbx when the node mdbx feature is selected so cfg(feature = "mdbx") code can reference MdbxStore. Op: correct Restores: spec:node mdbx feature exposes the storage MDBX backend it directly uses
Reject scantxoutset start calls that omit scanobjects and remove the stale whole-set summary test that contradicted the RPC scan contract. Op: correct Restores: spec:scantxoutset start requires non-empty scanobjects for descriptor scans
Remove the unused direct workspace dependency declaration for crossbeam-skiplist. The crate remains in Cargo.lock through fjall/lsm-tree, which is the live owner of that transitive dependency. Op: compress
Contributor
Author
|
@copilot review |
Contributor
Author
|
@codex review |
Contributor
Author
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces significant enhancements to consensus validation, node synchronization, and RPC functionality. Key changes include the implementation of Merkle tree mutation checks to address CVE-2012-2459, coinbase scriptSig size enforcement, and a more robust block synchronization process that buffers out-of-order blocks. Additionally, the scantxoutset RPC now supports address scanning, and new infrastructure for DNS seed bootstrapping and G2 MuHash sampling has been added. Feedback highlights a performance optimization opportunity in the UTXO scanning logic, suggesting the use of a HashSet to reduce lookup complexity from linear to logarithmic.
Comment on lines
+164
to
+186
| &self, | ||
| scripts: &[ScriptBuf], | ||
| scan: &mut UtxoScan, | ||
| ) -> Result<(), UtxoError> { | ||
| let cell = self.inner.read(); | ||
| cell.with_dependent(|_arena, table| { | ||
| for record in &table.table { | ||
| for output in record.iter_outputs() { | ||
| scan.txouts = scan.txouts.saturating_add(1); | ||
| let script = script_slice(table, output).ok_or(UtxoError::CorruptArena)?; | ||
| if scripts.iter().any(|target| target.as_bytes() == script) { | ||
| scan.unspents.push(ScannedUtxo { | ||
| outpoint: OutPoint::new(record.txid(), output.vout), | ||
| txout: txout_from_parts(output.value, script), | ||
| coinbase: output.coinbase, | ||
| height: output.height, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| Ok(()) | ||
| }) | ||
| } |
There was a problem hiding this comment.
The scan_script_pubkeys implementation performs an scantxoutset, this will be significantly slower than necessary. Converting the scripts slice to a HashSet (or BTreeSet for byte slices) before starting the shard iteration would reduce the complexity to
References
- Identify performance bottlenecks and optimize for efficiency by avoiding unnecessary nested iterations over large data structures.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff754c9cd9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+286
to
+287
| Err(TrySendError::Full(_) | TrySendError::Disconnected(_)) if persist => {} | ||
| Err(TrySendError::Full(_)) => { |
There was a problem hiding this comment.
Do not ignore TrySendError::Full/Disconnected for persistent add requests here: this now returns success even when no connection attempt was queued. In this codebase, added_nodes is only used for RPC reporting (repo-wide search shows no reconnect worker consuming it), so swallowing the send failure means addnode ... add can silently do nothing in production when the queue is full or closed.
Useful? React with 👍 / 👎.
Comment on lines
79
to
+80
| self.drain_inbound_headers(); | ||
| self.ensure_genesis_tip(); |
There was a problem hiding this comment.
This ordering regresses initial header acceptance with the new empty-tree root rule in accept_headers (validate_empty_tree_root): if a peer header batch arrives while the tree is still empty, drain_inbound_headers runs first and rejects non-genesis headers as missing-parent, then drops that batch. Seed genesis before header draining so early/unsolicited headers messages are not discarded.
Useful? React with 👍 / 👎.
…builds Agent-Logs-Url: https://github.com/gosuda/bitcoin-rs/sessions/3a7e582e-4853-4ec4-8f46-66e71f330c89 Co-authored-by: metaphorics <152830360+metaphorics@users.noreply.github.com>
Contributor
Author
|
@codex review |
Contributor
Author
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces significant enhancements to the node's block processing and synchronization logic, including the implementation of performance and parity gates (G2 and G14), improved handling of same-block spends, and out-of-order block buffering. It also adds support for BIP158 filter generation during block application, DNS seed bootstrapping for P2P connections, and the scantxoutset RPC method for address scanning. Feedback from the review highlights the use of the unstable let_chains feature which may break builds on stable Rust, and suggests using saturating_duration_since for more robust time calculations with Instant.
Comment on lines
+209
to
+213
| if let Some(parent) = parent_dir(path) | ||
| && let Ok(dir) = std::fs::File::open(parent) | ||
| { | ||
| let _ = dir.sync_all(); | ||
| } |
There was a problem hiding this comment.
The use of the let_chains feature (&& let) is currently unstable in Rust. Unless this project explicitly requires a nightly toolchain with this feature enabled, this will cause a compilation error on stable Rust. It is safer to use nested if let statements.
Suggested change
| if let Some(parent) = parent_dir(path) | |
| && let Ok(dir) = std::fs::File::open(parent) | |
| { | |
| let _ = dir.sync_all(); | |
| } | |
| if let Some(parent) = parent_dir(path) { | |
| if let Ok(dir) = std::fs::File::open(parent) { | |
| let _ = dir.sync_all(); | |
| } | |
| } |
| } | ||
|
|
||
| fn prune_received_blocks(received: &mut HashMap<Hash256, ReceivedBlock>, now: Instant) { | ||
| received.retain(|_hash, entry| now.duration_since(entry.received_at) < RECEIVED_BLOCK_TIMEOUT); |
There was a problem hiding this comment.
Using duration_since on Instant can panic if the clock is not monotonic (though Instant aims to be). Using saturating_duration_since is more defensive and prevents potential panics in edge cases.
Suggested change
| received.retain(|_hash, entry| now.duration_since(entry.received_at) < RECEIVED_BLOCK_TIMEOUT); | |
| received.retain(|_hash, entry| now.saturating_duration_since(entry.received_at) < RECEIVED_BLOCK_TIMEOUT); |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Comment on lines
+77
to
+90
| fn scanobjects_param(params: &Value) -> Result<&sonic_rs::Array, RpcError> { | ||
| let array = params_array(params)?; | ||
| let Some(scanobjects) = array.get(1) else { | ||
| return Err(RpcError::InvalidParams( | ||
| "scanobjects are required for scantxoutset start", | ||
| )); | ||
| }; | ||
| let scanobjects = scanobjects | ||
| .as_array() | ||
| .ok_or(RpcError::InvalidType("scanobjects must be an array"))?; | ||
| if scanobjects.is_empty() { | ||
| return Err(RpcError::InvalidParams("scanobjects must not be empty")); | ||
| } | ||
| Ok(scanobjects) |
Comment on lines
+34
to
+42
| /// Time after which a received out-of-order block is discarded. | ||
| const RECEIVED_BLOCK_TIMEOUT: Duration = Duration::from_mins(1); | ||
| /// Maximum number of received blocks waiting for their predecessor. | ||
| const RECEIVED_BLOCK_BUDGET: usize = 128; | ||
|
|
||
| struct ReceivedBlock { | ||
| block: bitcoin::Block, | ||
| received_at: Instant, | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces new evidence verification gates for mainnet performance and MuHash parity, adds smoke tests for the G2 MuHash collection script, and improves header validation logic. It also bumps the
bitcoin-rs-p2pcrate version. The main focus is on strengthening test coverage and ensuring correctness of externally collected benchmarking and parity evidence.New evidence verification gates:
G14performance budget verification gate ing14_perf_budgets.rs, which checks externally collected mainnet performance evidence against documented budgets for block validation throughput, UTXO commit latency, Electrum history latency, and memory usage.G2MuHash parity verification gate ing02_ibd_muhash_parity.rs, which asserts that externally collected MuHash samples frombitcoin-rsandbitcoindmatch at required heights, enforcing strict evidence contracts.Test improvements:
g2_bitcoind_muhash_script.rs, covering output format, normalization, and error handling.Chain header validation:
header_sync.rsby adding a check that prevents accepting headers with missing parents when the block tree is empty, unless the header is the network genesis block.header_sync_roundtrip.rsto importvalidate_header_nbitsand other identifiers directly.Dependency updates:
bitcoin-rs-p2pdependency version from0.1.0to0.1.1inCargo.toml.