fix(psbt): sanity check psbt before signing#486
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #486 +/- ##
==========================================
+ Coverage 80.30% 80.39% +0.09%
==========================================
Files 24 24
Lines 5417 5432 +15
Branches 245 246 +1
==========================================
+ Hits 4350 4367 +17
+ Misses 989 988 -1
+ Partials 78 77 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
110CodingP
left a comment
110CodingP
left a comment
There was a problem hiding this comment.
Thanks for working on this!
Do you also plan to work on this part of #52 ?
"In the signer module, the previous transaction contained in a PSBT input is not validated against the outpoint for legacy and segwit v0 transactions. This is checked when creating a transaction, but this module may be used to sign a PSBT as an external participant."
| } | ||
|
|
||
| #[test] | ||
| fn test_wallet_sign_rejects_malformed_psbt_input_count_mismatch() { |
There was a problem hiding this comment.
Plus I am sorry I wasn't able to understand what this test is trying to check. Could you please add some more comments here?
There was a problem hiding this comment.
This test effectively calls the wallet::sign with a malformed Psbt and expects the wallet to return an error instead of panicking.
So what I did here is crafting a raw transaction that initially just has 1 input then transformed it into a Psbt type to which I added more inputs (3 vs 1 in original tx).
There was a problem hiding this comment.
I see but how does the input count relate to the error raised?
There was a problem hiding this comment.
I'm not sure I get your question but that's the essence of the audit report.
When Wallet::sign will be called with the current setup (3 inputs in PSBT but 1 in raw tx), the get_utxo_for will return None and then the SignerError::MissingNonWitnessUtxo will be returned.
Not sure if it answers the question.
There was a problem hiding this comment.
We get the error even without the other 2 inputs because the first input itself does not contain a non_witness_utxo!
2 participants
Description
This PR attempts to sanity check the PSBT before signing. It closes audit issue #52
Notes to the reviewers
Changelog notice
Checklists
All Submissions:
just pbefore pushingBugfixes: