psbt: preserve proprietary fields when combining PSBTs #34893

w0xlt commented at 7:56 AM on March 22, 2026: contributor

BIP 174 proprietary fields are currently parsed, serialized, and exposed by decodepsbt, but they are not preserved by combinepsbt.

The reason is that the merge paths in PartiallySignedTransaction::Merge(), PSBTInput::Merge(), and PSBTOutput::Merge() union unknown, but never union m_proprietary.

This means application-specific PSBT metadata can be lost during combination, even though BIP 174 treats proprietary records as normal PSBT key-value pairs for private or application-specific use.

This PR fixes that by preserving proprietary fields in all three merge paths.

DrahtBot added the label PSBT on Mar 22, 2026

DrahtBot commented at 7:56 AM on March 22, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34765 (rpc: detect invalid signatures in analyzepsbt by overcookedpanda)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

rkrux commented at 11:49 AM on March 23, 2026: contributor

Good catch, will review. Surprised that merging proprietary fields was missed but merging unknown fields wasn't.

In the commit message of test: add PSBT proprietary merge regression coverage:

I'm confused by this statement.

These tests document the expected behavior before the implementation fix, so the follow-up commit can be reviewed by dropping it and observing the failure.

This is the second commit in the PR, the fix is in the preceding commit not in the follow-up one.

in src/test/psbt_tests.cpp:36 in 8cd41daf9a

  31 | +    const auto global_left = MakeProprietary(PSBT_GLOBAL_PROPRIETARY, 1, 0xaa);
  32 | +    const auto global_right = MakeProprietary(PSBT_GLOBAL_PROPRIETARY, 2, 0xbb);
  33 | +    const auto input_left = MakeProprietary(PSBT_IN_PROPRIETARY, 3, 0xcc);
  34 | +    const auto input_right = MakeProprietary(PSBT_IN_PROPRIETARY, 4, 0xdd);
  35 | +    const auto output_left = MakeProprietary(PSBT_OUT_PROPRIETARY, 5, 0xee);
  36 | +    const auto output_right = MakeProprietary(PSBT_OUT_PROPRIETARY, 6, 0xff);

achow101 commented at 9:16 PM on March 23, 2026:

In 8cd41daf9a5ad9ca460c2ed9b2050d21008ecf6e "test: add PSBT proprietary merge regression coverage"

It's not necessary to have unique fields for each scope. This could just be 2 PSBTProprietary that is reused across the scopes.

w0xlt commented at 11:00 PM on March 24, 2026:

Done. Thanks.

in src/test/psbt_tests.cpp:12 in 8cd41daf9a

   7 | +
   8 | +#include <boost/test/unit_test.hpp>
   9 | +
  10 | +BOOST_FIXTURE_TEST_SUITE(psbt_tests, BasicTestingSetup)
  11 | +
  12 | +static PSBTProprietary MakeProprietary(uint8_t scope_type, uint8_t unique_tag, uint8_t value_tag)

achow101 commented at 9:16 PM on March 23, 2026:

In 8cd41daf9a5ad9ca460c2ed9b2050d21008ecf6e "test: add PSBT proprietary merge regression coverage"

Calling the subtype and value "tag" is weird, that is not terminology used anywhere else in PSBTs.

w0xlt commented at 11:00 PM on March 24, 2026:

Done. Thanks.

in src/test/psbt_tests.cpp:17 in 8cd41daf9a

  12 | +static PSBTProprietary MakeProprietary(uint8_t scope_type, uint8_t unique_tag, uint8_t value_tag)
  13 | +{
  14 | +    return PSBTProprietary{
  15 | +        .subtype = unique_tag,
  16 | +        .identifier = {'p', 's', 'b', 't'},
  17 | +        .key = {scope_type, unique_tag},

achow101 commented at 9:18 PM on March 23, 2026:

In 8cd41daf9a5ad9ca460c2ed9b2050d21008ecf6e "test: add PSBT proprietary merge regression coverage"

Using the proprietary type as part of key data is odd. key can be any garbage, and having it be specifically these values is a bit confusing.

w0xlt commented at 11:00 PM on March 24, 2026:

Done. Thanks.

in test/functional/rpc_psbt.py:1175 in 8cd41daf9a outdated

1173 | @@ -1170,6 +1174,64 @@ def test_psbt_input_keys(psbt_input, keys):
1174 |  
1175 |          self.test_decodepsbt_musig2_input_output_types()

luke-jr commented at 11:34 PM on March 23, 2026:

Would probably be good to follow the method-for-test pattern here

w0xlt commented at 11:00 PM on March 24, 2026:

Done. Thanks.

psbt: preserve proprietary fields when combining PSBTs

CombinePSBTs currently preserves unknown records but drops proprietary records at the global, input, and output levels because the Merge() paths never union m_proprietary.

Preserve proprietary records in PartiallySignedTransaction::Merge(), PSBTInput::Merge(), and PSBTOutput::Merge() so combine/merge keeps all PSBT key-value data.

5e9b722494

w0xlt force-pushed on Mar 24, 2026

test: add PSBT proprietary merge regression coverage

Add unit and functional regression tests asserting that combine/merge preserves proprietary fields at the global, input, and output scopes.

eb76e953ac

w0xlt force-pushed on Mar 24, 2026

DrahtBot added the label CI failed on Mar 24, 2026

w0xlt commented at 1:14 AM on March 25, 2026: contributor

The CI error seems unrelated.

fanquake commented at 1:16 AM on March 25, 2026: member

The CI error seems unrelated.

Yea it's #34782.

DrahtBot removed the label CI failed on Mar 25, 2026