Repository navigation
Expose typed InputPair constructors in payjoin-ffi - #1959
Merged
Merged
Conversation
Collaborator
Coverage Report for CI Build 38280990748Coverage remained the same at 87.494%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Member
|
Handing this one over to @benalleng who's in the next timezone edit: would be really really nice to confirm this fixes the problem in the btcpayserver plugin where it originally cropped up as proof that the design is correct |
Bindings could only build an InputPair through the generic `new`, which needs a caller-supplied weight for inputs whose weight cannot be predicted. An unsigned P2TR input has no witness, so `new` fails for every Taproot coin unless the caller passes a weight, and binding users such as the BTCPay plugin pass none. Wrap the core `new_p2wpkh`, `new_p2tr_keyspend` and `new_p2tr_scriptpath_spend` constructors so bindings can contribute these inputs without building the TxIn and PSBT input by hand. The key path constructor supplies the expected weight itself.
The binding integration tests built each receiver input by hand from a TxIn and PSBT input and passed them to the generic constructor. They are also the example code binding users copy, and hand building is how Taproot receivers end up without an input weight. Build the receiver's P2WPKH inputs with `new_p2wpkh` instead.
The bindings' bitcoind test util always funds both wallets with P2WPKH coins, so binding tests cannot exercise a receiver contributing a Taproot input. Add a variant that funds both wallets with P2TR coins, mirroring the core `v2_to_v2_taproot` integration test setup. JavaScript has its own napi test utils crate, so the variant is added there too.
A receiver holding Taproot coins could not contribute an input through the bindings: an unsigned P2TR input has no witness to predict its weight from, so the generic constructor rejected it without a caller supplied weight. Run the v2 to v2 integration flow with Taproot wallets in every binding, building P2TR receiver inputs with `new_p2tr_keyspend`, so the payjoin is signed and accepted by bitcoind end to end.
The FFI PSBT input record has no `non_witness_utxo` field, so bindings had no way to contribute a legacy P2PKH input. Wrap the core `new_p2pkh` constructor, taking the funding transaction as consensus encoded bytes like the rest of the FFI. Decoding rejects trailing bytes, and a new `InputPairError::InvalidTransaction` variant reports bytes that do not decode. Output amounts above MAX_MONEY are rejected at the FFI boundary, as the other constructors do. Bindings that match exhaustively on `InputPairError` need to handle the new variant.
Receivers spending P2WSH outputs, such as multisig wallets, had to build the TxIn and PSBT input by hand and pass a weight to the generic constructor. Wrap the core `new_p2wsh` constructor. A witness script spend has no predictable weight, so the caller still supplies it.
chavic
force-pushed
the
chavic/ffi-typed-input-pair
branch
from
October 12, 2026 16:57
c20d01d to
a4e89cc
Compare
3 tasks done
Member
|
Added blocked label, awaiting example to merge |
Contributor
|
Tried BTCPay #196 on regtest with rust-payjoin Used the Payjoin v2 test payer; haven’t tried Bull Bitcoin yet. |
DanGould
approved these changes
Oct 13, 2026
DanGould
left a comment
DanGould
left a comment
Member
There was a problem hiding this comment.
utACK a4e89cc
@ValeraFinebits's used c20d01d which is the same Rust and the PR linked does show the before and after that we need.
I think InvalidTransaction is a bit of a broad name for an error that amounts to TransactionDecode really, but that's a major nit and we're not 1.0 on payjoin-ffi yet. Let's unblock BTCPayServer from upgrading 😎
spacebear21
added a commit
that referenced
this pull request
Oct 15, 2026
payjoin-ffi/CHANGELOG.md is the release notes for the Python, JavaScript, C# and Dart packages, but the changelog check only loops over the release crates, so pull requests that change payjoin-ffi/src can land without a line. The typed InputPair constructors from #1959 did exactly that: they ship in 0.25.0, whose entry says the packages jump with no API changes. Adopt the crates' convention: an "## Unreleased" section at the top that each pull request writes to and the bump only renames. Record the constructors under 0.25.0, where they ship, and correct the "no API changes" wording there and in the language changelogs. Extend the changelog check and the contributing guide to payjoin-ffi.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1955.
Bindings can only make an
InputPairwith the genericInputPair::new(txin, psbtin, expected_weight). An unsigned P2TR input has no witness. Thusnewcannot predict the input weight, and it fails if the caller gives no weight. The BTCPay plugin, Bull Bitcoin and Cake give no weight. A receiver with Taproot coins cannot contribute an input.This PR adds
new_p2wpkh,new_p2tr_keyspendandnew_p2tr_scriptpath_spendto payjoin-ffi.new_p2tr_keyspendsets the input weight. The binding integration tests now use the typed constructors. A new v2 to v2 test runs with Taproot wallets in Python, Kotlin, Dart, C# and JavaScript. In this test, the receiver contributes a P2TR input withnew_p2tr_keyspend, and bitcoind accepts the signed payjoin.The last two commits add
new_p2pkhandnew_p2wsh.new_p2pkhtakes the funding transaction as consensus-encoded bytes. It also adds theInputPairError::InvalidTransactionvariant. This is a breaking change: code that matches allInputPairErrorvariants must handle the new variant. Each of the two commits is isolated. You can drop one or both of them, and the rest of the PR does not change.Reviewer: @benalleng
Disclosure: co-authored by Claude Code.
Pull Request Checklist
Please confirm the following before requesting review: