Skip to content

feat(chain): validate unconfirmed transaction input scripts - #2469

Draft
LLFourn wants to merge 1 commit into
bitcoindevkit:masterfrom
LLFourn:input-validation-pr
Draft

LLFourn wants to merge 1 commit into
bitcoindevkit:masterfrom
LLFourn:input-validation-pr

Conversation

@LLFourn

@LLFourn LLFourn commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator

Description

An invalid unconfirmed transaction supplied by a chain source can make a wallet's outputs look spent and influence RBF or CPFP decisions. This change checks input scripts during canonicalization and excludes candidates whose scripts fail, together with candidates that depend on them.

This does not make BDK secure against malicious Electrum servers. It removes a very straightforward attack vector that directly results in loss of funds rather than something that needs to be combined with a social attack.

  • Add bdk_script_verify, a pure-Rust, no_std + alloc verifier adapted from rbitcoin. Its only runtime dependency is bitcoin. verify_input accepts a caller-owned VerificationCache; verify_transaction reuses one cache across all inputs.
  • Add TxGraph::validate_tx, with per-input Valid, Invalid and Unknown results and information about reliance on floating txouts. Full parent transactions take precedence over floating outputs; missing required prevouts produce Unknown.
  • Check only transactions eligible through observation. Anchored and explicitly assumed transactions retain their existing precedence. For each txid, prefer a whole Valid copy over Unknown and exclude it only when every retained copy is Invalid. The canonical view carries that exact copy; task-local memoization avoids rechecking copies across shared ancestry and rejected walks.
  • Retain complete witness variants by wtxid, without combining inputs from different copies. Persist these variants through changesets and SQLite schema v4.

The decision and its tradeoffs are documented in docs/input-validation.md.

Notes to the reviewers

The following behavior changes and known limitations are documented in the tests and ADR:

  1. Floating prevouts are trusted assertions from the chain source. A false amount can make a genuine segwit/taproot spend appear Invalid, including a wallet transaction with foreign inputs. Full parents avoid this ambiguity.
  2. Unsigned or placeholder-script drafts marked as seen can stop being canonical once their parents are available. assume_canonical remains an explicit override.

Validity-aware witness selection for ordinary graph queries such as get_tx is left for future work; these queries retain their deterministic, non-validating copy preference. For observed unconfirmed transactions, canonicalization selects a Valid copy when available and carries it into the canonical view, so a smaller junk witness cannot displace a retained valid copy. Anchored and explicitly assumed transactions retain their validation bypass.

Witness copies are currently unbounded. Verification shares transaction-wide signature hashes within each whole copy; legacy signature hashing and input-specific script work retain their own costs.

The verifier checks consensus script rules only. Transaction-level checks such as amounts, fees, finality, maturity, weight and block sigop limits are outside its scope. Related trust-boundary discussions: #2287 and #2340.

Validation

Before/after attack reproduction: unconfirmed_invalid_candidate_never_wins_on_last_seen creates a valid P2WPKH spend and a forged conflicting spend with a signature committing to different outputs, then marks the forged transaction as more recently seen. The same test body and helpers were run against the PR parent and the PR commit, omitting only unrelated imports/helpers for the new validation API:

  • Parent c5e87cc9: fails at assert!(view.tx(bad_txid).is_none()); the forged transaction is canonicalized.
  • The prepared implementation passes: the forged transaction and its output are excluded and the valid spend remains canonical.

Witness-copy attack reproduction: junk_witness_cannot_exclude_a_valid_transaction_or_its_descendants starts with a valid payment and child, then inserts a smaller invalid witness copy with the same txid. The identical test fails against the earlier implementation (11a1d846, built in an isolated target directory) because the payment disappears. It passes with the selection fix, returning the exact valid payment and child. Further tests cover all-invalid copies, Unknown fallback, deterministic ties, every arrival permutation, persistence reloads and anchored/assumed bypass.

The repository also includes regressions for invalid spends with only a floating prevout and for exclusion of descendants of an invalid transaction. Shared-cache tests check independent dependency reporting after errors, missing data and reordered checks; deterministic counters verify reuse. Isolated before/after release builds measured 1024-input graph validation improving from 104.3 ms to 28.5 ms for segwit and 393.9 ms to 27.9 ms for Taproot. Signing and graph construction are excluded; the ADR records the fixtures, smaller input counts and limitations.

  • Existing chain canonicalization tests and signed-spend tests cover legacy, segwit and taproot transactions, floating-prevout provenance, witness persistence and task-local memoization.
  • Bitcoin Core script vectors: 761 exercised rows, including constructed tapscript cases; 461 policy-failure rows are skipped.
  • Core tx_valid / tx_invalid and BIP341 wallet vectors cover transaction and signature-hash cases. The invalid-vector suite exercises 70 rows and skips 9 transaction-level and 14 policy cases.
  • Differential tests compare every input against Bitcoin Core through the pinned bitcoinkernel crate (0.3.2). Only the kernel's script-failure result counts as invalid; malformed-case errors fail the test.
  • A prior extended run exercised 600,000 generated cases plus deterministic sweeps: 1,034,359 inputs with no disagreement. The same saved inputs also agreed with libbitcoinconsensus. All 34 individually injected verifier faults were detected; deterministic limit sweeps cover the limit faults.

The kernel dependency is test-only and builds a pinned Core master snapshot (70d9ec7f), rather than a tagged release. Its compiled src/script files match Core v32.1; included-header differences are declaration/comment refactors. Native test builds need CMake, a C++ compiler and Boost headers; CI installs Boost in the relevant jobs.

Checks on the prepared branch:

  • Workspace tests with all features
  • Chain and script-verifier tests without default features
  • Workspace Clippy with all features/targets and warnings denied (Rust 1.96)
  • Nightly formatting and workspace rustdoc with warnings denied
  • Workspace library build without default features on MSRV 1.85
Verifier reuse and licensing

Depending directly on rbitcoin's consensus crate would add its block-validation/job architecture, std requirements and a newer MSRV. C++ consensus/kernel libraries do not meet the runtime portability requirements; the kernel is used as the test oracle. rust-bitcoin provides parsing and signature hashing but not a full script interpreter, and miniscript's interpreter covers only miniscript-shaped scripts.

The adapted interpreter and helpers come from rbitcoin commit 49210ea82130474a4df366e253d1346a16369fdb, licensed MIT OR Apache-2.0. File headers and the crate README retain attribution to rbitcoin contributors. VerifyScript orchestration follows Core's execution flow. The BIP143 implementation preserves raw hash-type bytes that rust-bitcoin's normalized segwit-v0 API does not expose.

Core JSON test vectors come from commit 633044f1, under the MIT license, with attribution in tests/data/README.md. The bitcoinkernel dev-dependency is MIT-licensed.

Changelog notice

Add input-script validation for observed unconfirmed candidates during canonicalization, a TxGraph::validate_tx API and the standalone bdk_script_verify crate. Preserve complete witness variants in changesets and SQLite schema v4, and select eligible whole copies for the canonical view. Anchored and explicitly assumed transactions bypass validation. See the ADR for draft-transaction behavior, floating-prevout trust and unbounded witness-copy storage.

Checklists

  • Added tests for the feature and documented behavior changes
  • Added public API documentation and an ADR
  • Retained source and license attribution
  • Followed contribution guidelines, including a signed commit

Check input scripts while canonicalizing transactions eligible through
observation, against full parents or floating prevouts. Reject Invalid
candidates and their descendants; retain Valid and Unknown candidates.
Anchored and explicitly assumed transactions bypass these checks.

Add the pure-Rust, no_std bdk_script_verify crate and TxGraph::validate_tx
with per-input results and floating-prevout provenance. Memoize results
only for the lifetime of one canonicalization task. Expose a caller-owned
VerificationCache for verify_input and a whole-transaction convenience API.
Reuse transaction-wide signature hashes within each copy while keeping
per-input dependency reports independent.

Keep complete witness variants by wtxid and persist them through changesets
and SQLite schema v4. During canonicalization, prefer a Valid whole copy
over Unknown and exclude a txid only if all copies are Invalid. Carry
the selected copy into the view, so junk witnesses cannot hide a retained
valid payment. Document the ordinary graph-query preference and the
floating-prevout and unsigned-draft tradeoffs in the ADR.

Validate the verifier with Core vectors, signed spends, generated and
deterministic differential tests against a pinned libbitcoinkernel.
Retain attribution for the code adapted from rbitcoin.
@codecov

codecov Bot commented Nov 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.10656% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.12%. Comparing base (c5e87cc) to head (6cdbe19).

Files with missing lines Patch % Lines
crates/script_verify/src/lib.rs 68.25% 20 Missing ⚠️
crates/script_verify/src/interpreter.rs 98.69% 6 Missing and 5 partials ⚠️
crates/chain/src/rusqlite_impl.rs 89.39% 7 Missing ⚠️
crates/chain/src/tx_graph.rs 96.62% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2469      +/-   ##
==========================================
+ Coverage   83.62%   86.12%   +2.49%     
==========================================
  Files          31       37       +6     
  Lines        6339     7704    +1365     
  Branches      292      431     +139     
==========================================
+ Hits         5301     6635    +1334     
- Misses        970      997      +27     
- Partials       68       72       +4     
Flag Coverage Δ
rust 86.12% <97.10%> (+2.49%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant