#36224 test: Add test coverage for `PartiallySignedTransaction::Merge()`
https://github.com/bitcoin/bitcoin/pull/36224 · · +86/-0 in 1 files, 3 commits · labels: Tests
Goal
- Pins down edge-case PSBT merging behavior with unit tests to prevent silent regressions
This pull request adds unit test coverage for PartiallySignedTransaction::Merge() in src/test/psbt_tests.cpp. It covers three specific areas: ensuring PSBTs with mismatched versions cannot merge, verifying the propagation of fallback_locktime, and asserting the bitwise merging behavior of m_tx_modifiable flags.
Problem: Portions of the PSBT merge logic in src/psbt.cpp lacked unit test coverage against mutants, leaving edge-case behavior like version mismatches and fallback locktimes unpinned.
Category: Test infrastructure (#36 of 45)
P4 · test coverage
- P4 because adding coverage for PSBT merging logic does not touch consensus, mempool, or network relay
- Protects an isolated data structure against mutants without addressing high-risk failure modes
The PR adds 86 lines of unit tests for mutation coverage of PartiallySignedTransaction::Merge(). Under the test category rubric, adding test coverage for its own sake ranks low unless it covers high-risk areas (such as consensus, mempool policy, or p2p relay) or catches realistic regressions.
Membership: Adds unit tests to src/test/psbt_tests.cpp covering PSBT data structure methods.
Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0
Reviewability: Ready
- Ready for review: the change is small, self-contained, and passes CI cleanly
The PR is small, builds cleanly, and passes CI with no blocking dependencies.
Author status: active
Open concerns:
- The author raised a question in the PR description regarding whether conflicting fallback_locktimes between two PSBTs should silently favor the destination PSBT or fail explicitly.
Agreement: Crickets
- No reviews or comments have been posted yet
- Author raised an open question on whether conflicting fallback locktimes should fail explicitly
No reviews or comments yet
The PR has received no reviewer activity since being opened.
Review verdicts (DrahtBot): 0
Files
86 lines under test/bench/ci.
- src/test/psbt_tests.cpp +86/-0
Uncertainties
- Whether the author's question regarding fallback_locktime merge semantics in the PR description will prompt a behavior change rather than just testing existing behavior.
Card
This PR adds unit test coverage for PartiallySignedTransaction::Merge() in src/test/psbt_tests.cpp, covering version compatibility, fallback locktime propagation, and modifiable transaction flags. It addresses gaps identified via mutation testing in existing PSBT code. Under test category guidelines, adding test coverage for its own sake without addressing high-risk paths or fixing a bug is marginal. The PR is clean and ready for review, though no reviewers have commented yet.