Add `expected_weight` field to `InputPair` #772

arturgontijo · 2025-06-15T14:47:23Z

This PR implements a way to applications be able to set the weight of their inputs via expected_weight field of InputPair.
By doing that we will be able to solve #659

This PR also unblock the Liana's integration as it deals with P2wsh:
https://github.com/arturgontijo/liana/blob/payjoin-v3/lianad/src/payjoin/receiver.rs#L191-L197

TODO (if high level design is ok):

Proper set the commit message
Fix 2 broken tests (and improve existing ones)
Add tests

arminsabouri

Thank you for taking this challenge on. Do you mind squashing your commits. And writing a commit message that captures the high level changes as well as relevant design decisions. In general Lint and Fmt fixes should be applied in the commit that introduces the issue. Thanks

payjoin/src/psbt/mod.rs

payjoin/src/receive/mod.rs

payjoin/src/receive/v1/mod.rs

payjoin/src/send/v1.rs

coveralls · 2025-06-16T17:15:56Z

Pull Request Test Coverage Report for Build 16150204062

Details

174 of 178 (97.75%) changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.2%) to 85.571%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
payjoin/src/core/receive/mod.rs	60	61	98.36%
payjoin/src/core/psbt/mod.rs	1	4	25.0%

Totals
Change from base Build 16149447007:	0.2%
Covered Lines:	7514
Relevant Lines:	8781

💛 - Coveralls

spacebear21

At a high-level the current design seems backwards.

I think InternalInputPair should remain mostly untouched. Instead, expected_weight should be added to the public-facing InputPair. When an InputPair is constructed for a predictable input type (p2pkh, p2wpkh...) we can use the expected_input_weight method to calculate the expected weight from the script and store that in InputPair.expected_weight. For p2sh/p2sh... the caller should specify expected_weight manually and it gets stored as-is. This way we shouldn't really need to change contribute_inputs except to use the expected_weight field instead of the expected_input_weight() method.

payjoin/src/psbt/mod.rs

payjoin/src/receive/mod.rs

payjoin/src/receive/v1/mod.rs

arturgontijo · 2025-06-16T23:39:59Z

@0xBEEFCAF3 @spacebear21 ready to be reviewed again...

payjoin/src/psbt/mod.rs

payjoin/src/receive/v1/mod.rs

arturgontijo · 2025-06-20T23:04:11Z

Marking this as a draft as we still have things to discuss offline.

arminsabouri

Thanks for pushing this forward. I had a few items. Main concern is unrelated changes.

payjoin/src/receive/v1/mod.rs

payjoin/src/receive/mod.rs

payjoin/src/psbt/mod.rs

payjoin/src/send/v1.rs

payjoin/src/psbt/mod.rs

payjoin/src/receive/v1/mod.rs

payjoin/src/psbt/mod.rs

payjoin/src/receive/v1/mod.rs

arminsabouri

Approach ACK.

payjoin/src/receive/v1/mod.rs

shinghim · 2025-06-25T20:57:23Z

payjoin-cli/src/app/wallet.rs

        ..Default::default()
    };
-    InputPair::new(txin, psbtin).expect("Input pair should be valid")
+    InputPair::new(txin, psbtin, None).expect("Input pair should be valid")


Here, and in other places where InputPair::new is called, wouldn't this cause an issue if this is a script (p2sh, p2wsh, p2tr) spend? From @spacebear21's comment in #583:

the unlocking script (i.e. signature) could be any arbitrary script of unpredictable weight and can't be known in advance of signing. Since wallets should have the information required to predict the max size of the eventual signature, we can have them provide an "max expected weight" or similar parameter when constructing InputPairs.

If we're just putting None here, we'd run into the same issue of just not having the expected weight field wouldn't we?

Yes, we should probably switch to using the new_p2wpkh and other constructors for payjoin-cli. IIRC ListUnspentResultEntry has an address type field we could match on.

I think for payjoin-cli we could leave script spends unsupported for now since it's unlikely for bitcoin-cli users and not supported now anyway.

The InputPair::new() calls the expected_input_weight() (in the first place) that sets the expected_weight (if AddressType is supported), if that call breaks we then try to set the weight using the given expected_weight param (Option<>).
That said, wouldn't that cover the AddressType check?

Am i correct in assuming there's an objective/correct way to calculate the expected weight? If we're calling expected_input_weight in InputPair::new, we would hopefully be doing the calculation the correct way, and wouldn't need the user to provide the expected weight themselves, since they would just be doing what expected_input_weight is doing. If that's the case, i think it would make sense to keep the new field in InputPair, but populate it only through InputPair::new calling expected_input_weight internally. Then, the constructors would be more ergonomic, and the user wouldn't need to do duplicate work of calculating the weight themselves

The expected_weight field (provided by users/applications) would be used in situations where the expected_input_weight() is not able to predict the weight of the given InputPair (the issue that this PR tries to solve points out to one of them: #659)

oh i see, the expected weight can only really be calculated for the key spends, and this is mostly for the script spends, which will just return an error if the user doesn't provide something, as those aren't supported in expected_input_weight()

arturgontijo · 2025-06-25T23:18:12Z

payjoin/src/receive/mod.rs

-        assert_eq!(p2sh_pair.psbtin.non_witness_utxo.unwrap(), utxo);
+            InputPair::new_p2sh(utxo.clone(), outpoint, redeem_script.clone(), Some(sequence));
+
+        assert_eq!(


@spacebear21 @shinghim what do you guys recommend me to do here?
The issue is that while calling new() (from new_p2sh()) it fails in the match at:
https://github.com/payjoin/rust-payjoin/blob/master/payjoin/src/psbt/mod.rs#L203
As the given script is not a nested p2wpkh (tested by is_p2wpkh()).

Similar to the changes to InputPair::new, I think the script spends should have the additional field in their constructor to provide the expected weight. IIUC those are the types of inputs that actually need the user to provide them, since the key spends have a known max value (P2PKH_COMPRESSED_MAX/P2WPKH_MAX/NESTED_P2WPKH_MAX and maybe P2TR_KEY_DEFAULT_SIGHASH? though I'm not familiar enough with P2TR to know if we should return this value as the expected weight if the address is P2TR since there could also be a script path, right?)

IIRC we decided to solve the p2tr script path spends in a seperate PR. Documented here #788

And yes i agree. Script path input constructors should have a param for weight.

arturgontijo · 2025-06-27T10:41:19Z

PR is now ready to get another round of reviews.
@0xBEEFCAF3 I used rebase+squash+new commit message, lmk if it's ok now.

payjoin/src/receive/mod.rs

arminsabouri

I think this is moving in the right direction. There a few smaller items. And request to catch duplicate input pairs for the reciver typestate field. One thing i don't love is that we have to create a dummy input_pair in InputPair::new(...). I think this can be resolved if we refactor expected_input_weight to be a pure function and not part of internal input pair. I can take a look into that

payjoin/src/psbt/mod.rs

payjoin/src/receive/error.rs

payjoin/src/receive/mod.rs

payjoin/src/receive/v1/mod.rs

payjoin/src/receive/mod.rs

arminsabouri · 2025-06-27T18:07:10Z

Last push cleans up test_multiple_contribute_inputs() and resolves the duplicate input pair issue in contribute inputs

arminsabouri · 2025-06-27T18:48:23Z

Ended up reverting the dedup logic after noticing that we dont dedup inputs going into the payjoin psbt.

arminsabouri

One more thing i noticed. I will push a fix for this my self.

payjoin/src/core/receive/mod.rs

spacebear21 · 2025-07-03T16:02:20Z

payjoin/src/core/receive/mod.rs

    pub fn new_p2wsh(
        txout: TxOut,
        outpoint: OutPoint,
        witness_script: ScriptBuf,


Is there any reason to keep the witness_script parameter? IIRC it was only needed to calculate the expected weight but that's no longer relevant.

Is there any reason to keep the witness_script parameter?

Can't think of a good reason. I will remove it in a seperate commit on this PR.

one thing to note. Some wallets will require witness_script to be present in the psbt when signing and they may need to insert them later after they use the constructor. The same is true of key derivation fields as well.

For now I think the solution for fancier wallets will be to use the new_unchecked method and pass their own PsbtInput/TxIn. We'll just need to ensure those fields don't get cleared out elsewhere.

payjoin/src/core/receive/mod.rs

spacebear21 · 2025-07-03T16:07:32Z

payjoin/src/core/receive/mod.rs

+        // P2wsh yet is not supported when expected weight is not provided
+        assert_eq!(
+            p2wsh_pair.err().unwrap(),
+            PsbtInputError::from(InternalPsbtInputError::from(InputWeightError::NotSupported))


This makes me wonder if we should change this variant name since "NotSupported" implies it just needs to be implemented, but really this is more like "impossible to predict weight"

payjoin/src/core/receive/v1/mod.rs

Co-authored-by: Armin Sabouri <armins88@gmail.com> This change allows applications to specify the weight of their own inputs via `InputPair::new()` method. This enables more accurate fee estimation for complex inputs, such as P2WSH. It solves issue payjoin#659.

Witness script was used to help derive the expected input weight. We now require the application to provide the weight for variable weight inputs (p2wsh).

spacebear21

ACK 311e775 - though there is a failing FFI check in CI.

arminsabouri · 2025-07-08T17:30:34Z

though there is a failing FFI check in CI.

Whoops missed that. Thanks
Last push should fix that https://github.com/payjoin/rust-payjoin/compare/311e775998d1ac8ab50f64c9d537a0b24347d9ff..92996ecc04e1cd06bf8d91966fc22c3b7cf50445

arminsabouri

Approving so my req for change doesnt block merge status

spacebear21

reACK 92996ec. Nice work!

arminsabouri requested changes Jun 15, 2025

View reviewed changes

arturgontijo mentioned this pull request Jun 15, 2025

InputPair's lack of a constructor seems to cause mistakes #583

Open

arminsabouri reviewed Jun 15, 2025

View reviewed changes

arturgontijo force-pushed the 659-round-II branch 2 times, most recently from 44fdd80 to 96c0b58 Compare June 15, 2025 16:16

spacebear21 requested changes Jun 16, 2025

View reviewed changes

payjoin/src/psbt/mod.rs Show resolved Hide resolved

payjoin/src/receive/mod.rs Outdated Show resolved Hide resolved

payjoin/src/receive/v1/mod.rs Outdated Show resolved Hide resolved

arturgontijo force-pushed the 659-round-II branch 2 times, most recently from 9c90464 to fcee2cd Compare June 16, 2025 23:37

arturgontijo changed the title ~~Add contribute_inputs_with_weights()~~ Add InputPair::new_with_weight() Jun 17, 2025

spacebear21 reviewed Jun 17, 2025

View reviewed changes

payjoin/src/psbt/mod.rs Outdated Show resolved Hide resolved

payjoin/src/receive/v1/mod.rs Outdated Show resolved Hide resolved

payjoin/src/receive/v1/mod.rs Outdated Show resolved Hide resolved

payjoin/src/receive/v1/mod.rs Outdated Show resolved Hide resolved

arturgontijo force-pushed the 659-round-II branch from fcee2cd to 762890e Compare June 18, 2025 22:36

DanGould force-pushed the master branch 2 times, most recently from 61351cb to 9f403f1 Compare June 20, 2025 20:17

arturgontijo force-pushed the 659-round-II branch from 762890e to 0e4d27a Compare June 20, 2025 23:01

arturgontijo marked this pull request as draft June 20, 2025 23:02

arminsabouri reviewed Jun 21, 2025

View reviewed changes

spacebear21 requested changes Jun 23, 2025

View reviewed changes

arturgontijo force-pushed the 659-round-II branch from 0e4d27a to 14da2a7 Compare June 24, 2025 11:24

arturgontijo changed the title ~~Add InputPair::new_with_weight()~~ Add expected_weight field to InputPair Jun 24, 2025

arturgontijo force-pushed the 659-round-II branch 2 times, most recently from 92ea08e to fb5bc60 Compare June 24, 2025 14:11

arminsabouri reviewed Jun 24, 2025

View reviewed changes

payjoin/src/receive/v1/mod.rs Outdated Show resolved Hide resolved

arturgontijo force-pushed the 659-round-II branch 2 times, most recently from e570cd9 to f134955 Compare June 25, 2025 20:18

shinghim reviewed Jun 25, 2025

View reviewed changes

arturgontijo force-pushed the 659-round-II branch from f134955 to 454bd17 Compare June 25, 2025 23:05

arturgontijo commented Jun 25, 2025

View reviewed changes

arturgontijo force-pushed the 659-round-II branch 3 times, most recently from 6581cb9 to 190ec3a Compare June 26, 2025 23:01

shinghim reviewed Jun 27, 2025

View reviewed changes

payjoin/src/receive/mod.rs Outdated Show resolved Hide resolved

arminsabouri requested changes Jun 27, 2025

View reviewed changes

arminsabouri force-pushed the 659-round-II branch 2 times, most recently from c6885eb to 9579130 Compare June 27, 2025 18:06

arminsabouri force-pushed the 659-round-II branch from 9579130 to 73c3245 Compare June 27, 2025 18:39

arturgontijo force-pushed the 659-round-II branch from 73c3245 to b16d70b Compare June 27, 2025 20:22

arminsabouri force-pushed the 659-round-II branch from b16d70b to 1be77b7 Compare June 30, 2025 14:27

arminsabouri reviewed Jun 30, 2025

View reviewed changes

payjoin/src/core/receive/mod.rs Outdated Show resolved Hide resolved

arminsabouri force-pushed the 659-round-II branch 2 times, most recently from b706f7d to 4cef427 Compare June 30, 2025 17:00

spacebear21 reviewed Jul 3, 2025

View reviewed changes

arminsabouri force-pushed the 659-round-II branch from 4cef427 to 311e775 Compare July 7, 2025 13:00

arturgontijo and others added 2 commits July 7, 2025 09:18

Remove witness_script from input ctor

92996ec

Witness script was used to help derive the expected input weight. We now require the application to provide the weight for variable weight inputs (p2wsh).

arminsabouri mentioned this pull request Jul 8, 2025

Input weight prediction for P2WSH inputs #659

Closed

spacebear21 approved these changes Jul 8, 2025

View reviewed changes

spacebear21 mentioned this pull request Jul 8, 2025

De-duplicate inputs in contribute_inputs #861

Closed

arminsabouri force-pushed the 659-round-II branch from 311e775 to 92996ec Compare July 8, 2025 17:29

arminsabouri approved these changes Jul 8, 2025

View reviewed changes

arminsabouri requested a review from spacebear21 July 8, 2025 17:58

spacebear21 approved these changes Jul 8, 2025

View reviewed changes

arminsabouri merged commit 26eff9b into payjoin:master Jul 8, 2025
15 checks passed

arminsabouri mentioned this pull request Jul 15, 2025

Fix weight estimation for P2TR inputs #880

Closed

Add expected_weight field to InputPair #772

Add expected_weight field to InputPair #772

Uh oh!

Conversation

arturgontijo commented Jun 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arminsabouri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coveralls commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 16150204062

Details

💛 - Coveralls

Uh oh!

spacebear21 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arturgontijo commented Jun 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arturgontijo commented Jun 20, 2025

Uh oh!

arminsabouri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arminsabouri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shinghim Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spacebear21 Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shinghim Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arturgontijo Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Add `expected_weight` field to `InputPair` #772

Add `expected_weight` field to `InputPair` #772

arturgontijo commented Jun 15, 2025 •

edited

Loading

coveralls commented Jun 16, 2025 •

edited

Loading

shinghim Jun 25, 2025 •

edited

Loading

spacebear21 Jun 25, 2025 •

edited

Loading

shinghim Jun 26, 2025 •

edited

Loading

arturgontijo Jun 25, 2025 •

edited

Loading

arturgontijo commented Jun 27, 2025 •

edited

Loading

arminsabouri Jul 7, 2025 •

edited

Loading