#36261 test: cover PSBT unknown field merging
https://github.com/bitcoin/bitcoin/pull/36261 · · +130/-0 in 1 files, 1 commits · labels: Tests
Goal
- Ensure combinepsbt preserves unknown key-value pairs across global, input, and output maps
- Prevents regressions for tooling and workflows using proprietary or newly defined PSBT fields
Adds functional test cases to `test/functional/rpc_psbt.py` validating that `combinepsbt` preserves unknown key-value pairs across global, input, and output maps. The test covers both PSBTv0 and PSBTv2 across duplicate values, conflicting values, and fields present in only one of the inputs.
Problem: The `combinepsbt` RPC's handling of unknown PSBT fields lacked explicit functional test coverage across various edge cases, risking regressions in BIP 174 and BIP 370 compliance for tooling and workflows using proprietary or newly defined PSBT fields.
Category: RPC / REST / ZMQ (#39 of 52)
P3 · test coverage
- P3 because it adds regression test coverage without altering RPC behavior or fixing an active bug
- Pins combinepsbt handling of unknown fields to safeguard callers relying on PSBT extensions
Pins RPC behavior for `combinepsbt` unknown field merging as recommended in #34893. While it does not fix a bug or alter RPC behavior itself, it provides worthwhile regression coverage for callers using PSBT extensions.
Membership: Tests the behavior of the `combinepsbt`, `decodepsbt`, and `createpsbt` RPC endpoints.
Factors: security/stability 0, bug 0, performance 0, user value 1, leverage 1
Reviewability: Ready
- Ready for review: clean, self-contained functional test addition with CI passing
CI is passing, the diff is clean and self-contained (+130 lines in a functional test), and review feedback has been addressed.
Author status: Active; addressed review comments and pushed updates.
Resolved concerns:
- polespinasa suggested testing the case where unknown fields are present in only one PSBT and missing from the other, which the author added.
- polespinasa suggested refactoring assertions into a helper function, but noted it was purely a style preference; the author preferred explicit assertions and kept them.
Agreement: Strong
- Approach approval for the test coverage additions (w0xlt)
- Concept approval after author added requested test coverage for single-sided fields (polespinasa)
Strong: w0xlt gave Approach ACK and polespinasa gave Concept ACK with review feedback incorporated
Reviewers who worked on or requested this test coverage in #35310 and #34893 reviewed and approved the approach.
- w0xlt commented 'Approach ACK'
- polespinasa commented 'Concept ACK' and suggested an additional test case that was added
Review verdicts (DrahtBot): 0
- Approach ACK: w0xlt
- Concept ACK: polespinasa
Files
130 lines under test/bench/ci.
- test/functional/rpc_psbt.py +130/-0
Card
This PR adds functional test coverage to verify that combinepsbt correctly preserves unknown key-value pairs across global, input, and output maps for both PSBTv0 and PSBTv2. It covers duplicate identical keys, conflicting values where the first PSBT wins, and missing keys. It completes work started in #35310 and recommended in #34893. Review state is ready with Concept ACK and Approach ACK from reviewers.