feat: expose detailed try_finalize_psbt outcomes#433
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #433 +/- ##
==========================================
+ Coverage 80.96% 81.13% +0.17%
==========================================
Files 24 24
Lines 5489 5546 +57
Branches 247 246 -1
==========================================
+ Hits 4444 4500 +56
- Misses 968 970 +2
+ Partials 77 76 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
c65f534 to
e046adc
Compare
|
Updated per some good feedback from mammal |
f3fb2a6 to
69966b6
Compare
ValuedMammal
left a comment
There was a problem hiding this comment.
I don't think After and Older are doing much good, and would prefer to leave them out of the new implementation if we plan to support try_finalize_psbt into the future.
|
Concept ACK, glad to see this as a non-breaking change. |
|
Added four commits as small follow-ups from review feedback from mammal: make finalization cleanup more explicit, preserve opaque PSBT/input metadata, stop the new |
| /// The input was already finalized before this call. | ||
| AlreadyFinalized, | ||
| /// The input was successfully finalized during this call. | ||
| Finalized, |
There was a problem hiding this comment.
question: are we using this differently ? otherwise, a single Finalized would be enough imho.
There was a problem hiding this comment.
I see where you're coming from for sure but I do think the distinctions intentional. Finalized would be this call was able to finalize the input, AlreadyFinalized would be the input already had final script/witness data before this call and we skipped it. is_finalized() treats both as success, but I thought preserving that difference was useful since this API is meant to explain what happened per input.
There was a problem hiding this comment.
Good, I think I see where you're going. Agree, it's probably useful to have that distinction if it'll be used for upstream users (e.g UX/UI).
Description
First step for #73, following ValuedMammal's suggestion.
This PR keeps the scope to PSBT finalization only:
try_finalize_psbtwhich returnsFinalizePsbtResultFinalizePsbtInputResultNotes to the reviewers
This is now additive rather than breaking:
Wallet::finalize_psbtstill returnsboolIntentionally not included here:
Wallet::signto return a richer result typeChangelog notice
Wallet::try_finalize_psbt, which returnsFinalizePsbtResultand exposes per-input finalization outcomes while preserving the existingWallet::finalize_psbtAPI.Checklists
All Submissions:
just pbefore pushingNew Features: