fix(mint): delete orphan blank outputs on melt change early-return#1016
Open
gorrdy wants to merge 3 commits into
Open
fix(mint): delete orphan blank outputs on melt change early-return#1016gorrdy wants to merge 3 commits into
gorrdy wants to merge 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1016 +/- ##
==========================================
+ Coverage 75.04% 75.07% +0.03%
==========================================
Files 111 111
Lines 12244 12246 +2
==========================================
+ Hits 9188 9194 +6
+ Misses 3056 3052 -4 ☔ View full report in Codecov by Sentry. |
Collaborator
|
looks like the CI is failing |
Contributor
|
@gorrdy , you've got this test failing: `FAILED tests/mint/test_mint.py::test_generate_change_promises_zero_fee_deletes_all_blanks - assert 0 == 4
|
KvngMikey
reviewed
May 27, 2026
| # Clean up the blank outputs the wallet sent for fee return; otherwise | ||
| # they remain in `promises` with c_ IS NULL and collide with later | ||
| # operations that re-derive the same B_ (e.g. NUT-13 seed restore). | ||
| if melt_id: |
Contributor
There was a problem hiding this comment.
if melt_id and outputs is not None: # only clean up if something was stored
`_generate_change_promises` already deletes the wallet's blank NUT-08 outputs from `promises` after signing them. But when the function takes the early-return branch (overpaid_fee <= 0 or no outputs supplied) those rows were left behind with `c_ IS NULL` — and later operations that re-derive the same `B_` (notably NUT-13 seed restore) collide with them and surface as `OutputsArePendingError`. Also soften the negative-overpaid_fee log to debug with a message that explains the case: in practice the backend can report a fee greater than the wallet-provided reserve (e.g. an LNbits backend skimming a service fee on top of the routing fee), so an error-level log here is noisy and misleading. Adds two regression tests covering both the `overpaid_fee == 0` and `overpaid_fee < 0` branches.
The two `overpaid_fee == 0` / `overpaid_fee < 0` cases differ only in one integer offset, so collapse them into a single parametrized test and drop the `_run_melt_with_patched_fee` helper / `INVOICE_64_SAT` module constant — the invoice is inlined to match the style of the other tests in this file.
…lanks with fix The test was added in 9fed0f0 (PR cashubtc#795) with a name asserting deletion ("..._deletes_all_blanks") but assertions baked in the then-current bug where the overpaid_fee <= 0 early-return skipped cleanup. This PR completes the fix; flip the assertions to match the fixed behavior the test name already claimed. Also apply KvngMikey's review suggestion: guard the cleanup with `outputs is not None` so we don't issue a no-op DELETE when the wallet sent no blanks to begin with.
gorrdy
force-pushed
the
fix/melt-orphan-blank-outputs
branch
from
226048f to
d82ebf3
Compare
Contributor
Author
|
Should be ok now. |
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
3 participants
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.
Summary
_generate_change_promisesdeletes the wallet's blank NUT-08 outputsfrom the
promisestable after signing them, but the early-returnbranch (
overpaid_fee <= 0or no outputs supplied) skipped thatcleanup. Those rows were left behind with
c_ IS NULLand collided withlater operations that re-derive the same
B_— most visibly NUT-13seed restore, which would surface as
OutputsArePendingError.delete_blinded_messages_melt_idcleanup in theearly-return branch.
overpaid_fee < 0log fromerrortodebugwith aclearer message. In practice this is reachable whenever the LN backend
reports a fee greater than the wallet-provided reserve (e.g. an LNbits
backend skimming a service fee on top of the routing fee), so the
original "this should not happen" wording is misleading and the
error-level severity is noisy.
overpaid_fee == 0and
overpaid_fee < 0cases.Test plan
poetry run pytest tests/mint/test_mint_melt.py -k early_return -vpasses both parametrize cases (overpaid_fee_zero,overpaid_fee_negative).main(without the ledger fix) — orphan rowsassertion trips.
poetry run pytest tests/mint/test_mint_melt.py.