Skip to content

fix(finalizer): set finalized to false on error and add full coverage#44

Merged
ValuedMammal merged 2 commits intobitcoindevkit:masterfrom
noahjoeris:add-finalizer-tests
Apr 11, 2026
Merged

fix(finalizer): set finalized to false on error and add full coverage#44
ValuedMammal merged 2 commits intobitcoindevkit:masterfrom
noahjoeris:add-finalizer-tests

Conversation

@noahjoeris
Copy link
Copy Markdown
Contributor

Description

This PR fixes a bug in Finalizer::finalize() where finalized remained true after an input finalization error, for example when a signature is missing. That could cause PSBT output metadata to be cleared even though finalization did not fully succeed.
I also added tests with full coverage.

@ValuedMammal
Copy link
Copy Markdown
Collaborator

Why not use is_finalized on the FinalizeMap directly to check if the outputs can be cleared and do away with the finalized variable altogether?

@noahjoeris noahjoeris force-pushed the add-finalizer-tests branch from 17f3734 to cfdad3d Compare April 9, 2026 11:31
@noahjoeris
Copy link
Copy Markdown
Contributor Author

yep makes sense. Done.

Copy link
Copy Markdown
Collaborator

@ValuedMammal ValuedMammal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK cfdad3d

@ValuedMammal ValuedMammal merged commit abd3b1f into bitcoindevkit:master Apr 11, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants