Skip to content

fix(node): honor disabled optional indexes#5

Open
metaphorics wants to merge 37 commits into
mainfrom
fix/runtime-readiness-sync-utxo
Open

fix(node): honor disabled optional indexes#5
metaphorics wants to merge 37 commits into
mainfrom
fix/runtime-readiness-sync-utxo

Conversation

@metaphorics

Copy link
Copy Markdown
Contributor

Keep txindex and compact-filter index handles optional so disabled indexes are neither opened nor advertised, while RPC lookup paths preserve Core-compatible disabled-index behavior.

Verification:

  • 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-rpc --lib --no-fail-fast

  • cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-rpc --test handler_smoke --no-fail-fast

  • FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --lib --no-default-features --features $FEATURES --no-fail-fast

  • FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --test state_storage --no-default-features --features $FEATURES --no-fail-fast

  • FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --test rpc_wiring --no-default-features --features $FEATURES --no-fail-fast

  • FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --test sync_smoke --no-default-features --features $FEATURES --no-fail-fast

  • FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --all-targets --no-default-features --features $FEATURES -- -D warnings

  • FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --no-default-features --features $FEATURES --no-fail-fast

  • cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast

Op: correct

Restores: spec: optional txindex/blockfilterindex disabled semantics

Keep txindex and compact-filter index handles optional so disabled indexes are neither opened nor advertised, while RPC lookup paths preserve Core-compatible disabled-index behavior.

Verification:

- 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-rpc --lib --no-fail-fast

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-rpc --test handler_smoke --no-fail-fast

- FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --lib --no-default-features --features $FEATURES --no-fail-fast

- FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --test state_storage --no-default-features --features $FEATURES --no-fail-fast

- FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --test rpc_wiring --no-default-features --features $FEATURES --no-fail-fast

- FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --test sync_smoke --no-default-features --features $FEATURES --no-fail-fast

- FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --all-targets --no-default-features --features $FEATURES -- -D warnings

- FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --no-default-features --features $FEATURES --no-fail-fast

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast

Op: correct

Restores: spec: optional txindex/blockfilterindex disabled semantics
Node's default feature set does not enable bitcoinconsensus, so non-taproot script verification tests must not run in that configuration. Keep the coverage in the full-feature gate.

Verification:

- 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-node --lib --no-fail-fast

- FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --lib --no-default-features --features $FEATURES --no-fail-fast

- FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --all-targets --no-default-features --features $FEATURES -- -D warnings

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast

Op: correct

Restores: spec: backend-dependent node tests only run when bitcoinconsensus is enabled
Add an ignored synthetic replay that exercises the real BlockSync/apply_block path with index hooks disabled and noop hook branches. This gives a bounded local source of apply_block profile rows without claiming mainnet or storage-index performance.

Verification:

- cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check

- RUST_LOG=bitcoin_rs_node::apply=info 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 bounded_apply_profile_replay -- --ignored --nocapture --test-threads=1

- RUST_LOG=bitcoin_rs_node::apply=info /home/alpha/dev/bitcoin-rs/bitcoin-rs/target/release/deps/sync_smoke-03e9c594e970bfab bounded_apply_profile_replay --ignored --nocapture --test-threads=1

- FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --test sync_smoke --no-default-features --features $FEATURES --no-fail-fast

- FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --all-targets --no-default-features --features $FEATURES -- -D warnings

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast

Op: extend
Add an ignored Redb-backed direct index harness for bounded local measurement of txindex ingest and filter-index persistence cost over the synthetic sync-smoke chain.

Verification:

- 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-node --no-default-features --features redb,bitcoinconsensus --test sync_smoke optional_index_redb_direct_cost -- --ignored --nocapture --test-threads=1

- FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --test sync_smoke --no-default-features --features $FEATURES --no-fail-fast

- FEATURES=rocksdb,fjall,redb,mdbx,bitcoinconsensus cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --all-targets --no-default-features --features $FEATURES -- -D warnings

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast

Op: extend
Add a Criterion harness in the coinstats crate so the existing coinstats -> utxo dependency owns the measurement without introducing a utxo -> coinstats cycle.

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-coinstats --benches --no-default-features -- -D warnings
- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --no-fail-fast
- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --bench utxo_commit_coinstats -- --sample-size 10

Synthetic benchmark results:
- no_listener: [3.7867 ms 3.9379 ms 4.1257 ms]
- coinstats_listener: [659.61 ms 664.15 ms 668.28 ms]

Caveat: this is synthetic UtxoSet::commit_block evidence, not end-to-end node runtime evidence.

Op: extend
Add a no-op UTXO listener case to the coinstats commit benchmark. This falsifies the cheap explanation that the synthetic delta is merely listener dispatch or shard hook overhead.

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-coinstats --benches --no-default-features -- -D warnings
- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --no-fail-fast
- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --bench utxo_commit_coinstats -- --sample-size 10

Synthetic benchmark results:
- no_listener: [3.8390 ms 3.9874 ms 4.0988 ms]
- noop_listener: [4.5625 ms 4.6284 ms 4.7106 ms]
- coinstats_listener: [648.83 ms 654.98 ms 660.95 ms]

Caveat: this separates synthetic hook overhead from CoinStats/MuHash work; it is not an end-to-end node runtime claim.

Op: extend
Add an ignored sync_smoke harness that replays the same non-coinbase spend fixture with the CoinStats listener attached to UTXO mutations and detached from UTXO mutations. The detached mode still wires CoinStats into ApplyHandles; it only skips the UTXO listener hook for measurement.

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 --features bitcoinconsensus,redb --test sync_smoke -- -D warnings
- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --features bitcoinconsensus,redb --test sync_smoke --no-fail-fast
- RUST_LOG=bitcoin_rs_node::sync=debug,bitcoin_rs_node::apply=info cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --features bitcoinconsensus,redb --test sync_smoke bounded_apply_profile_replay_coinstats_listener_cost -- --ignored --nocapture

Observed direct harness output in a feature-compatible test binary:
- attached: elapsed_ms=31 applied_height=102 blocks=103
- detached: elapsed_ms=14 applied_height=102 blocks=103
- height 101/102 utxo_commit_us attached: 133/159
- height 101/102 utxo_commit_us detached: 65/61

Caveat: detached mode is a measurement control, not a production configuration or proof of end-to-end mainnet speed.

Op: extend
Add a benchmark-only listener variant that runs through the existing UTXO commit path while measuring lock plus simple stats accounting without coin encoding or MuHash updates.

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-coinstats --bench utxo_commit_coinstats -- -D warnings
- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --benches --no-run
- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --bench utxo_commit_coinstats

Observed Criterion medians:
- no_listener: 3.5933 ms
- noop_listener: 4.2716 ms
- accounting_listener: 31.108 ms
- coinstats_listener: 635.70 ms

Caveat: accounting_listener is a benchmark control, not production behavior or proof of mainnet speed.

Op: extend
Add a benchmark-only direct CoinStats insert/remove case beside the UTXO commit-path listener cases. The direct case starts from the same preloaded stats state and measures the mutation loop without UTXO shard/listener dispatch.

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-coinstats --bench utxo_commit_coinstats -- -D warnings
- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --benches --no-run
- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --bench utxo_commit_coinstats

Observed Criterion medians:
- no_listener: 3.5302 ms
- noop_listener: 4.1248 ms
- accounting_listener: 52.541 ms
- coinstats_listener: 648.01 ms
- direct_coinstats_insert_remove: 145.65 ms

Caveat: direct_coinstats_insert_remove is diagnostic and excludes UTXO shard/listener dispatch; it is not production behavior or proof of mainnet speed.

Op: extend
Add benchmark-only diagnostic splits for CoinStats direct mutation attribution. The new cases mirror the private coin preimage encoder locally and measure encoding/allocation separately from MuHash3072 insert/remove over pre-encoded bytes, while keeping the combined direct mutation baseline and commit-path listener cases.

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-coinstats --bench utxo_commit_coinstats -- -D warnings
- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --benches --no-run
- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --bench utxo_commit_coinstats

Observed Criterion medians:
- no_listener: 3.7033 ms
- noop_listener: 3.9494 ms
- accounting_listener: 50.415 ms
- coinstats_listener: 629.41 ms
- direct_coinstats_insert_remove: 143.73 ms
- direct_coinstats_encode_only: 1.7696 ms
- direct_coinstats_muhash_preencoded: 145.54 ms

Caveat: split direct benchmarks are diagnostic controls and mirror private encoding; they are not production behavior or proof of mainnet speed.

Op: extend
Add a benchmark-only sharded CoinStats listener control to test whether the production listener's single write lock serializes UTXO shard workers. The sharded control routes callbacks by the same UtxoKey shard mapping as the UTXO set and keeps the existing production listener unchanged.

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-coinstats --bench utxo_commit_coinstats -- -D warnings
- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --benches --no-run
- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --bench utxo_commit_coinstats

Observed Criterion medians:
- no_listener: 3.7830 ms
- noop_listener: 3.8697 ms
- accounting_listener: 50.068 ms
- sharded_coinstats_listener: 14.963 ms
- coinstats_listener: 650.82 ms
- direct_coinstats_insert_remove: 149.71 ms
- direct_coinstats_encode_only: 1.9434 ms
- direct_coinstats_muhash_preencoded: 147.11 ms

Caveat: sharded_coinstats_listener is a benchmark control that does not combine shard snapshots; it is not production behavior or proof of mainnet speed.

Op: extend
Document the current CoinStatsListener visibility boundary while UtxoSet::commit_block is in flight. The test pauses after the first forwarded listener insert, observes listener-only state, then checks post-commit and post-finish_block snapshots.

Evidence:

- cargo test -p bitcoin-rs-coinstats --test snapshot_with_muhash snapshot_can_observe_mid_block_listener_state_before_finish_block

- cargo test -p bitcoin-rs-coinstats --test snapshot_with_muhash

- cargo fmt --all -- --check

- cargo clippy -p bitcoin-rs-coinstats --all-targets -- -D warnings

Op: extend
Extend the ignored replay cost harness with a test-local listener wrapper that forwards to the real CoinStatsListener and counts callback kinds. This keeps production code untouched while making attached/detached replay evidence less blind.

Evidence:

- cargo test -p bitcoin-rs-node --test sync_smoke --no-default-features --features bitcoinconsensus,redb bounded_apply_profile_replay_coinstats_listener_cost -- --ignored --nocapture

- fresh sync_smoke binary: attached elapsed_ms=28 listener_total_calls=106; detached elapsed_ms=15 listener_total_calls=0; test result ok

- cargo fmt --all -- --check

- cargo clippy -p bitcoin-rs-node --test sync_smoke --no-default-features --features bitcoinconsensus,redb -- -D warnings

Op: extend
Replace the single CoinStats listener write lock with private per-UTXO-shard signed deltas and a linearization barrier for snapshots and block folds. This preserves the public listener surface while removing the global callback serialization seen in the UTXO commit benchmark.

Evidence:

- cargo fmt --all -- --check

- cargo clippy -p bitcoin-rs-coinstats --all-targets -- -D warnings

- cargo test -p bitcoin-rs-coinstats

- cargo test --workspace --no-fail-fast

- cargo bench -p bitcoin-rs-coinstats --bench utxo_commit_coinstats -- utxo_commit_coinstats

Op: compress
Replace test expect calls with explicit error propagation and keep the bitcoinconsensus-only test helper feature-gated so default clippy and feature builds both compile.\n\nEvidence:\n- cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check\n- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --all-targets -- -D warnings\n- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --no-default-features --features bitcoinconsensus,rocksdb,redb --no-fail-fast\n\nOp: correct\nRestores: spec:bitcoin-rs-node clippy -D warnings all-targets gate
Use the existing BlockSync pending-budget window as the getdata issuance cap instead of keeping a second per-tick batch cap. The first tick now fills the pending window from the next applied height, and a second tick with the window full sends only getheaders.\n\nEvidence:\n- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node tick_fills_pending_budget_from_next_applied_height_when_gap_exceeds_budget\n- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node second_tick_does_not_re_request_already_pending_blocks\n- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node sync::tests\n- cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check\n- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --all-targets -- -D warnings\n- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --no-default-features --features bitcoinconsensus,rocksdb,redb --no-fail-fast\n\nOp: extend
Track dirty CoinStats listener shards so finish_block and snapshot skip clean shard deltas instead of combining identity MuHash accumulators every block. Existing linearize locking remains the synchronization contract; dirty bits are hints under that lock.

Evidence: bounded apply replay coin_stats_us fell from 29,710us to 2,613us for index_hooks=disabled and from 14,252us to 2,513us for index_hooks=noop in /tmp/sync-smoke-bounded-apply-profile-logs*.

Op: compress
Track dirty UTXO shards while bucketing a block commit, use fixed shard bucket arrays, and execute the single-shard case directly under the stable-view lock. This removes the all-shard scan and Rayon dispatch overhead for the common tiny-block path without changing the public UTXO contract.

Evidence: bounded apply replay non-genesis utxo_commit_us fell from 5,750us to 1,462us for index_hooks=disabled and from 5,321us to 1,850us for index_hooks=noop in /tmp/sync-smoke-bounded-apply-profile-logs-dirty-*.

Op: compress
Add a regression oracle for the zero-dirty-shard UTXO commit path. The test seeds both folded CoinStats base state and an unfinished dirty delta, then proves an empty BlockChanges commit preserves the UTXO set and listener snapshot.

Op: correct
Restores: spec:empty UTXO commits preserve existing set and CoinStats listener state
Replace the empty ignored G1 scaffold with a HEAD-bound evidence gate. The gate now requires declared mainnet header samples from bitcoin-rs and bitcoind, verifies complete 0..=tip coverage, rejects malformed or duplicate hashes, and compares the maps byte-for-byte.

This is an offline evidence validator only; it does not claim live tip freshness or implement a headers-only runtime mode.

Op: correct
Restores: spec:G1 header parity gate must not pass without bound evidence
Add opt-in headers-only runtime plumbing through config, CLI, env, and production BlockSync construction. In headers-only mode BlockSync anchors genesis as a header, advances by header tip height, sends headers requests without body getdata, and drains inbound block bodies without buffering or applying them. RPC height lookup now resolves the published active header tree so getblockhash works without block records and stale records cannot override active headers.

Evidence:

- 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-rpc --test handler_smoke getblockhash_ -- --nocapture

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node headers_only_tick -- --nocapture

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --test config_layered headers_only_defaults_false_and_layers_enable -- --exact --nocapture

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-node --features redb,bitcoinconsensus --test sync_smoke -- --nocapture

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-rpc --test handler_smoke -- --nocapture

- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --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 -p bitcoin-rs --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --workspace --no-fail-fast

Op: extend
Add benchmark-only controls for sharded counters, encoding, and MuHash arithmetic to the synthetic UTXO commit CoinStats benchmark. Production code is unchanged; this is an attribution probe only and makes no runtime speed claim.

Evidence:\n- cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check\n- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --benches -- -D warnings\n- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --bench utxo_commit_coinstats -- --sample-size 10 --measurement-time 1 --warm-up-time 1

Caveat: benchmark-only attribution probe, not production behavior or proof of node/runtime speed.

Op: extend
Add a benchmark-only direct MuHash probe using the same OutPoint::as_bytes payload as the callback-shaped sharded MuHash listener benchmark. This reconciles attribution evidence before any production optimization and makes no runtime speed claim.

Evidence:\n- cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check\n- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --benches -- -D warnings\n- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-coinstats --bench utxo_commit_coinstats direct_muhash_outpoint_bytes -- --sample-size 10 --measurement-time 1 --warm-up-time 1

Caveat: benchmark-only attribution probe, not production behavior or proof of node/runtime speed.

Task: #233

Op: extend
Add bounded apply-local attribution for script verification outliers. The new event keeps the existing script_verify_us contract intact and decomposes it into compute_verify_flags, verify_block_transactions, cumulative transaction verifier time, and residual block-local overhead only when a block crosses the outlier threshold.

Evidence:

- cargo fmt --all -- --check

- cargo test -p bitcoin-rs-node verify_block_transactions --no-default-features --features rocksdb,bitcoinconsensus -- --nocapture

- cargo clippy -p bitcoin-rs --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings

- cargo test -p bitcoin-rs --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus

- cargo test --workspace --no-fail-fast

Op: correct

Restores: spec:evidence-first attribution before script optimization
Record the slowest transaction verifier calls inside outlier script verification blocks. The probe stays apply-local, bounded to the existing outlier gate, and emits no metrics labels or consensus-surface changes.

Evidence:

- cargo fmt --all -- --check

- cargo test -p bitcoin-rs-node verify_block_transactions --no-default-features --features rocksdb,bitcoinconsensus -- --nocapture

- cargo test -p bitcoin-rs-node script_verify_profile_keeps_bounded_slowest_transactions --no-default-features --features rocksdb,bitcoinconsensus -- --nocapture

- cargo clippy -p bitcoin-rs --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings

- cargo test -p bitcoin-rs --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus

- cargo test --workspace --no-fail-fast

Op: correct

Restores: spec:evidence-first attribution before script optimization
Expose a profiled borrowed transaction verifier that preserves the existing verifier API and returns coarse timing buckets for node-owned attribution. The node keeps all logging behind the existing outlier and top-transaction gates.

Evidence:

- cargo fmt --all -- --check

- cargo test -p bitcoin-rs-consensus --no-fail-fast

- cargo test -p bitcoin-rs-node verify_block_transactions --no-default-features --features rocksdb,bitcoinconsensus -- --nocapture

- cargo clippy -p bitcoin-rs --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings

- cargo test -p bitcoin-rs --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus

- cargo test --workspace --no-fail-fast

Op: correct

Restores: spec:evidence-first attribution before script optimization
Remove the temporary public transaction verifier profiling API after collecting bounded signet attribution evidence. Keep node-level bounded top transaction timing and add a regression that underfunded transactions fail before script execution.\n\nEvidence:\n- cargo fmt --all -- --check\n- cargo test -p bitcoin-rs-consensus underfunded_transaction_fails_before_script_execution -- --nocapture\n- cargo test -p bitcoin-rs-consensus --no-fail-fast\n- cargo test -p bitcoin-rs-node verify_block_transactions --no-default-features --features rocksdb,bitcoinconsensus -- --nocapture\n- cargo test -p bitcoin-rs-node script_verify_profile_keeps_bounded_slowest_transactions --no-default-features --features rocksdb,bitcoinconsensus -- --nocapture\n- cargo clippy -p bitcoin-rs --all-targets --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus -- -D warnings\n- cargo test -p bitcoin-rs --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus\n- cargo test --workspace --no-fail-fast\n\nOp: compress
Add a dev-only Criterion benchmark target for the script interpreter boundary. The fixture separates non-empty witness conversion, transaction clone/mutation, serialization, bitcoinconsensus verification of a pre-serialized transaction, and full Interpreter::execute on a 400-input transaction.

The benchmark target is explicitly gated on the bitcoinconsensus feature so no-default feature checks do not build the backend-only slice accidentally. This is diagnostic smoke evidence only, not a signet-wide root-cause or Bitcoin Core comparison.

Evidence:

- cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check

- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-script --no-default-features --features bitcoinconsensus --bench interpreter_execute_profile --no-run

- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-script --all-targets --no-default-features -- -D warnings

- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-script --all-targets --no-default-features --features bitcoinconsensus -- -D warnings

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-script --no-default-features --features bitcoinconsensus --no-fail-fast

- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-script --no-default-features --features bitcoinconsensus --bench interpreter_execute_profile

- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --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

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus

Op: extend
Drop a test-module import that is unused under the no-default all-targets clippy graph. This keeps the consensus clippy gate warning-free without changing verifier behavior.

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-consensus --all-targets --no-default-features -- -D warnings

- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --all-targets --no-default-features --features bitcoinconsensus -- -D warnings

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --no-fail-fast

- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --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

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus

Op: compress
Add a bitcoinconsensus-gated Criterion benchmark for the production RustValidator::verify_tx path. The fixture uses a 400-input OP_TRUE transaction with non-empty witness data and a BTreeMap UTXO view, so the timed path includes the current verifier bookkeeping, UTXO lookups, per-input interpreter boundary, and sigop cost path.

This is evidence-only diagnostic coverage for #191. It is not representative signet/mainnet proof, not a root-cause claim, and not a Bitcoin Core comparison.

Benchmark sample:

- verify_transaction_profile/verify_tx_op_true_400_input: [384.56 ms 387.83 ms 390.87 ms] on one tuned local Criterion run

Evidence:

- cargo fmt --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml --all -- --check

- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --no-default-features --features bitcoinconsensus --bench verify_transaction_profile --no-run

- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --all-targets --no-default-features -- -D warnings

- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --all-targets --no-default-features --features bitcoinconsensus -- -D warnings

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --no-fail-fast

- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --no-default-features --features bitcoinconsensus --bench verify_transaction_profile

- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --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

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus

Op: extend
Serialize each non-coinbase transaction once in the bitcoinconsensus verifier path and feed those bytes directly to rust-bitcoin for non-taproot script checks. The current consensus caller passes the input's own scriptSig and witness back into Interpreter::execute, so the cloned-and-mutated transaction was byte-equivalent to the original transaction for this non-taproot path.

Keep the interpreter fallback for P2TR+TAPROOT and for no-bitcoinconsensus builds so taproot behavior and disabled-backend behavior stay on the existing code path. Add OP_TRUE/OP_FALSE consensus tests for the direct path.

Local benchmark evidence only:

- before: verify_transaction_profile/verify_tx_op_true_400_input [384.56 ms 387.83 ms 390.87 ms]

- after: verify_transaction_profile/verify_tx_op_true_400_input [354.94 ms 359.52 ms 364.62 ms]

This is a synthetic OP_TRUE 400-input fixture; it is not signet/mainnet proof, not a root-cause claim, and not a Bitcoin Core comparison.

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-consensus --all-targets --no-default-features -- -D warnings

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --no-default-features --no-fail-fast

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --no-default-features --features kernel -- --include-ignored

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --no-fail-fast

- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --all-targets --no-default-features --features bitcoinconsensus -- -D warnings

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-script --no-fail-fast

- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --no-default-features --features bitcoinconsensus --bench verify_transaction_profile

- cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --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

- cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs --no-fail-fast --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus

Op: compress
Add branch-boundary regressions for the serialized bitcoinconsensus verifier path. The tests pin the non-taproot direct serialized transaction input, the P2TR+TAPROOT local fallback, and the feature-disabled interpreter backend path.

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-consensus --all-targets --no-default-features -- -D warnings; cargo clippy --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --all-targets --no-default-features --features bitcoinconsensus -- -D warnings; cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --no-default-features --no-fail-fast; cargo test --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --no-default-features --features bitcoinconsensus --no-fail-fast; 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:verifier backend branch invariants
Add one explicit tx_valid-derived workload for the production RustValidator::verify_tx path. The fixture hardcodes the selected row, converts excluded Core flags to STANDARD-minus-excluded flags, and fails closed before Criterion measures it. This keeps the benchmark as one falsifier instead of a partial Core-vector parser.

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-consensus --benches --features bitcoinconsensus -- -D warnings

- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-consensus --bench verify_transaction_profile verify_tx_core_valid_vector -- --test

Op: extend
Move the script_verify timer start after verify flag construction so script_verify_us and node.apply_block.script_verify_seconds no longer include compute_verify_flags_us. This preserves the apply/verification behavior and restores attribution exclusivity for profiling output.

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 --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 -p bitcoin-rs-node --no-default-features --features rocksdb,fjall,redb,mdbx,bitcoinconsensus --test sync_smoke bounded_apply_profile_replay -- --ignored

Op: correct

Restores: spec:apply profiling attribution exclusivity
Avalanche the synthetic benchmark seed before deriving txid prefix bytes so the UTXO commit benchmark exercises all 256 shards instead of one modulo-4 residue class. Add a fail-closed distribution assertion before the benchmark summary so the measurement oracle cannot silently regress to the biased generator.

This changes the synthetic workload and Criterion baseline; it is not a production performance regression or speed claim.

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-utxo --benches --no-default-features -- -D warnings

- cargo bench --manifest-path /home/alpha/dev/bitcoin-rs/bitcoin-rs/Cargo.toml -p bitcoin-rs-utxo --no-default-features --bench utxo_commit -- --warm-up-time 1 --measurement-time 1 --sample-size 10

Op: correct

Restores: spec:UTXO benchmark shard-distribution oracle
Restore the synthetic benchmark invariant that generated coinstats UTXO keys exercise every shard instead of leaking raw seed low bytes into shard selection.

The benchmark input distribution changes, so prior coinstats Criterion baselines are not apples-to-apples evidence. This commit is benchmark maintenance only; it makes no production speed or readiness claim.

Evidence: cargo fmt --all -- --check; cargo clippy -p bitcoin-rs-coinstats --benches --no-default-features -- -D warnings; targeted Criterion bench completed with the shard assertion enabled.

Op: correct

Restores: spec:synthetic coinstats benchmark exercises all UTXO shards
The replay fixture spends a legacy OP_TRUE output. The no-bitcoinconsensus backend only carries the local Taproot key-path fallback, so this fixture has always depended on the bitcoinconsensus script backend.

Gate the correctness test and bounded profiling harnesses on the feature instead of letting default node tests fail with a misleading final-tip mismatch at height 100.

Evidence: cargo fmt --all -- --check; default exact sync_smoke legacy-spend test filtered out; bitcoinconsensus exact sync_smoke legacy-spend test passed; both bitcoinconsensus exact ignored replay harnesses passed; cargo clippy -p bitcoin-rs-node --test sync_smoke --features bitcoinconsensus -- -D warnings.

Op: correct

Restores: spec:legacy OP_TRUE replay tests require bitcoinconsensus script backend
Copilot AI review requested due to automatic review settings June 2, 2026 15:13
@metaphorics

Copy link
Copy Markdown
Contributor Author

@codex review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces sharded tracking for CoinStatsListener to improve performance, adds a headers-only synchronization mode, and makes txindex and blockfilterindex optional. It also adds transaction verification profiling and several benchmark suites. The review feedback highlights a critical bug in apply_signed_delta where large positive deltas cause incorrect decrements, and suggests optimization opportunities such as lazily serializing transactions for Taproot inputs and removing redundant timers in block application.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +182 to +194
#[cfg(feature = "bitcoinconsensus")]
let serialized_tx = encode::serialize(tx);

for (input_index, (input, prevout)) in tx.input.iter().zip(input_prevouts.iter()).enumerate() {
#[cfg(feature = "bitcoinconsensus")]
verify_input_script(
input_index,
input,
prevout,
tx,
serialized_tx.as_slice(),
flags,
)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Serializing the transaction eagerly before the input loop is inefficient when all inputs are Taproot (P2TR), as Taproot script verification bypasses bitcoinconsensus and uses the local interpreter directly. We can lazily serialize the transaction only when a non-Taproot input is encountered by passing a &mut Option<Vec<u8>> to verify_input_script.

Suggested change
#[cfg(feature = "bitcoinconsensus")]
let serialized_tx = encode::serialize(tx);
for (input_index, (input, prevout)) in tx.input.iter().zip(input_prevouts.iter()).enumerate() {
#[cfg(feature = "bitcoinconsensus")]
verify_input_script(
input_index,
input,
prevout,
tx,
serialized_tx.as_slice(),
flags,
)?;
#[cfg(feature = "bitcoinconsensus")]
let mut serialized_tx = None;
for (input_index, (input, prevout)) in tx.input.iter().zip(input_prevouts.iter()).enumerate() {
#[cfg(feature = "bitcoinconsensus")]
verify_input_script(
input_index,
input,
prevout,
tx,
&mut serialized_tx,
flags,
)?;

Comment on lines +212 to +236
fn verify_input_script(
input_index: usize,
input: &bitcoin::TxIn,
prevout: &bitcoin::TxOut,
tx: &bitcoin::Transaction,
serialized_tx: &[u8],
flags: VerifyFlags,
) -> Result<(), ConsensusError> {
let script = Script::from_bytes(prevout.script_pubkey.as_bytes());
if script.is_p2tr() && flags.contains(VerifyFlags::TAPROOT) {
return verify_input_script_with_interpreter(input_index, input, prevout, tx, flags);
}

script
.verify_with_flags(
input_index,
prevout.value,
serialized_tx,
flags.consensus_bits(),
)
.map_err(|error| ConsensusError::Script {
input_index,
reason: format!("script verification failed: {error}"),
})
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update verify_input_script to accept &mut Option<Vec<u8>> and lazily serialize the transaction using get_or_insert_with only when a non-Taproot input is verified.

fn verify_input_script(
    input_index: usize,
    input: &bitcoin::TxIn,
    prevout: &bitcoin::TxOut,
    tx: &bitcoin::Transaction,
    serialized_tx: &mut Option<Vec<u8>>,
    flags: VerifyFlags,
) -> Result<(), ConsensusError> {
    let script = Script::from_bytes(prevout.script_pubkey.as_bytes());
    if script.is_p2tr() && flags.contains(VerifyFlags::TAPROOT) {
        return verify_input_script_with_interpreter(input_index, input, prevout, tx, flags);
    }

    let tx_bytes = serialized_tx.get_or_insert_with(|| encode::serialize(tx));
    script
        .verify_with_flags(
            input_index,
            prevout.value,
            tx_bytes,
            flags.consensus_bits(),
        )
        .map_err(|error| ConsensusError::Script {
            input_index,
            reason: format!("script verification failed: {error}"),
        })
}

Comment on lines +280 to +288
fn apply_signed_delta(value: &mut u64, delta: i128) {
if let Ok(increment) = u64::try_from(delta) {
*value = value.saturating_add(increment);
return;
}

let decrement = u64::try_from(delta.saturating_neg()).unwrap_or(u64::MAX);
*value = value.saturating_sub(decrement);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If delta is positive but larger than u64::MAX, u64::try_from(delta) will fail, causing the function to fall through to the negative delta handling logic. In that block, delta.saturating_neg() will be negative, its conversion to u64 will fail, and it will default to u64::MAX, resulting in a decrement of value instead of an addition. Explicitly checking if delta >= 0 ensures correctness for all possible values of delta.

Suggested change
fn apply_signed_delta(value: &mut u64, delta: i128) {
if let Ok(increment) = u64::try_from(delta) {
*value = value.saturating_add(increment);
return;
}
let decrement = u64::try_from(delta.saturating_neg()).unwrap_or(u64::MAX);
*value = value.saturating_sub(decrement);
}
fn apply_signed_delta(value: &mut u64, delta: i128) {
if delta >= 0 {
let increment = u64::try_from(delta).unwrap_or(u64::MAX);
*value = value.saturating_add(increment);
} else {
let decrement = u64::try_from(delta.saturating_neg()).unwrap_or(u64::MAX);
*value = value.saturating_sub(decrement);
}
}

Comment thread crates/node/src/apply.rs
Comment on lines +233 to +243
let script_verify_started = quanta::Instant::now();
let block_transactions_started = quanta::Instant::now();
let script_verify_result =
verify_block_transactions(handles, block, height, locktime_cutoff, verify_flags);
let block_transactions_dur = block_transactions_started.elapsed();
let script_verify_dur = script_verify_started.elapsed();
metrics::histogram!("node.apply_block.script_verify_seconds")
.record(script_verify_dur.as_secs_f64());
script_verify_result?;
let script_verify_profile = script_verify_result?;
if script_verify_dur.as_micros() >= SCRIPT_VERIFY_OUTLIER_BLOCK_US {
let verify_block_transactions_us = block_transactions_dur.as_micros();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The block_transactions_started and block_transactions_dur timers are redundant as they measure the exact same block of code as script_verify_started and script_verify_dur. We can eliminate the extra timer and reuse script_verify_dur.

    let script_verify_started = quanta::Instant::now();
    let script_verify_result = 
        verify_block_transactions(handles, block, height, locktime_cutoff, verify_flags);
    let script_verify_dur = script_verify_started.elapsed();
    metrics::histogram!("node.apply_block.script_verify_seconds")
        .record(script_verify_dur.as_secs_f64());
    let script_verify_profile = script_verify_result?;
    if script_verify_dur.as_micros() >= SCRIPT_VERIFY_OUTLIER_BLOCK_US {
        let verify_block_transactions_us = script_verify_dur.as_micros();

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f3e04e2d82

ℹ️ 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 thread crates/node/src/apply.rs
%error,
"tx_index failed to ingest block; best-effort path continues"
);
if let Some(tx_index) = &handles.tx_index {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Gate the in-memory transaction cache with txindex

When running with the new default txindex = false, this guard skips the persistent txindex ingest, but the loop immediately above still inserts every confirmed transaction into handles.transactions unconditionally. During a normal full sync with txindex disabled, the node will still retain all confirmed transactions in RAM even though getrawtransaction now refuses to use that cache without ctx.indexer, so disabling the optional index does not actually avoid the largest resource cost. Please skip or bound that confirmed transaction cache when handles.tx_index is None.

Useful? React with 👍 / 👎.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR primarily restores Bitcoin Core-compatible behavior for optional indexes (txindex / basic block filter index) so that when disabled they are not opened, not advertised via RPC, and related RPCs fail with Core-like “index disabled” errors. The PR also includes additional changes around headers-only sync, coinstats listener concurrency, script verification profiling, and new Criterion benchmarks.

Changes:

  • Make txindex and compact-filter index handles truly optional across node state, apply pipeline, and RPC context/handlers (including Core-compatible disabled-index RPC behavior).
  • Add/expand headers-only sync mode wiring (config + sync behavior + tests) and extend RPC getblockhash behavior to use active headers when block records are missing/stale.
  • Introduce performance/profiling instrumentation and Criterion benchmarks (script interpreter, consensus tx verification, UTXO/coinstats commit).

Reviewed changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
crates/utxo/src/set.rs Optimizes UTXO commits by tracking and processing only dirty shards.
crates/utxo/benches/utxo_commit.rs Improves synthetic txid distribution and asserts shard coverage in benchmarks.
crates/script/Cargo.toml Adds Criterion bench configuration for script crate.
crates/script/benches/interpreter_execute_profile.rs Adds diagnostic benchmark for Interpreter::execute boundary costs.
crates/rpc/tests/handler_smoke.rs Adds/updates RPC smoke tests for disabled indexes and header-first getblockhash.
crates/rpc/src/handlers/tx.rs Gates confirmed-tx lookup for getrawtransaction on txindex being enabled.
crates/rpc/src/handlers/chain.rs Returns “index disabled” for getblockfilter; updates getindexinfo to omit disabled indexes and use Core-style names.
crates/rpc/src/error.rs Introduces RpcError::IndexDisabled with Core-like -1 error code.
crates/rpc/src/context.rs Makes filter_index optional; prefers active header chain for height→hash lookups.
crates/node/tests/sync_smoke.rs Extends sync smoke tests and adds ignored profiling harnesses; updates ApplyHandles optional index hooks.
crates/node/tests/state_storage.rs Tests that optional indexes aren’t opened/created when disabled; validates Electrum requires txindex.
crates/node/tests/rpc_wiring.rs Updates wiring test for optional index handles (Option-wrapped).
crates/node/tests/config_layered.rs Adds layered-config test coverage for headers_only.
crates/node/src/sync.rs Adds headers-only mode behavior (no getdata, drop inbound blocks, genesis header bootstrapping).
crates/node/src/state.rs Makes txindex/filterindex handles optional; Electrum helpers now return Result and require txindex storage.
crates/node/src/run.rs Wires Electrum startup through fallible txindex-dependent helpers; passes optional txindex to RPC context.
crates/node/src/config.rs Adds headers_only configuration, CLI/env layering, and Electrum→txindex validation.
crates/node/src/apply.rs Makes optional indexes best-effort (skip when disabled); adds script verification profiling attribution.
crates/consensus/src/verify_tx.rs Refactors input script verification to use bitcoinconsensus for non-taproot and local fallback for taproot; adds tests.
crates/consensus/Cargo.toml Adds Criterion bench configuration for consensus crate.
crates/consensus/benches/verify_transaction_profile.rs Adds diagnostic benchmark for production tx verification path.
crates/coinstats/tests/snapshot_with_muhash.rs Adds extensive tests around listener correctness under commit/undo and mid-block snapshots.
crates/coinstats/src/stats.rs Reworks coinstats listener into sharded deltas with linearized folding/snapshotting.
crates/coinstats/Cargo.toml Adds Criterion bench configuration for coinstats crate.
crates/coinstats/benches/utxo_commit_coinstats.rs Adds synthetic benchmark comparing listener strategies and coinstats overhead.
Cargo.lock Adds Criterion dependencies to affected crates.
bin/bitcoin-rs/tests/gates/g01_headers_only_sync.rs Reworks ignored gate test to validate externally collected headers-only evidence bound to git HEAD.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +280 to +288
fn apply_signed_delta(value: &mut u64, delta: i128) {
if let Ok(increment) = u64::try_from(delta) {
*value = value.saturating_add(increment);
return;
}

let decrement = u64::try_from(delta.saturating_neg()).unwrap_or(u64::MAX);
*value = value.saturating_sub(decrement);
}
Comment thread crates/node/src/config.rs
Comment on lines 112 to +116
pub txindex: bool,
/// Whether the compact block filter index is enabled.
pub blockfilterindex: bool,
/// Whether P2P sync stops at headers and drops block bodies from the sync channel.
pub headers_only: bool,
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants