fix(mint): fix rotate_next_keyset - #1097
Closed
vicjuma wants to merge 1 commit into
Closed
Conversation
- track selected_keyset_counter so the highest-counter keyset is actually selected, not just the last one iterated - fix debug log to reference selected_keyset.id instead of the stale loop variable keyset.id
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1097 +/- ##
==========================================
- Coverage 74.63% 74.60% -0.03%
==========================================
Files 115 115
Lines 13200 13201 +1
==========================================
- Hits 9852 9849 -3
- Misses 3348 3352 +4 ☔ View full report in Codecov by Harness. |
Collaborator
|
Already being fixed in #1058 |
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
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
cashu/mint/keysets.py:37(rotate_next_keyset) failed to updateselected_keyset_counterinside its selection loop, so the "select the active keyset with the highest counter" comparison always ran against its initial value instead of the running maximum. With a single active keyset per unit — the only case exercised so far — this has no observable effect, since there's nothing to compare against. However, the moment more than one active keyset exists for the same unit (a state NUT-02 permits), selection becomes dependent on dict iteration order rather than actual counter value, which can silently select the wrong keyset for rotation.Fixing this is low-risk and preserves current single-keyset behavior exactly, while making the function correct if that assumption ever doesn't hold — whether from future multi-keyset support, migrations, or manual DB state.
Also replaced a stray
keyset.idreference in the final debug log withselected_keyset.id.keysetwas a leftover loop variable from the selection loop above and no longer holds a meaningful value once the loop exits; using it there could log the wrong keyset ID as the one deactivated.Changes
rotate_next_keyset: correctly trackselected_keyset_counteron each iterationrotate_next_keyset: fix debug log to referenceselected_keyset.idinstead of stalekeyset.idTesting
No behavioral change for the existing single-active-keyset code path, so the existing test suite passes unmodified.