Skip to content

fix: ignore non-canonical asset names when scaling - #207

Merged
ascandone merged 2 commits into
mainfrom
fix/scaling-non-canonical-assets
Oct 1, 2026
Merged

ascandone merged 2 commits into
mainfrom
fix/scaling-non-canonical-assets

Conversation

@ascandone

Copy link
Copy Markdown
Contributor

Reported by @Azorlogh on #199.

funds.GetAssets groups an account's balances by scale, but different asset names can have the same scale: EUR and EUR/0 are both scale 0. One balance overwrote the other, and which one was kept depended on map iteration order.

With balances acc1: EUR = 5, EUR/0 = 0, EUR/2 = 100, this script:

send [EUR 1] (
  source = @acc1 with scaling through @swap
  destination = @dest
)

sometimes pays the 1 EUR directly and sometimes converts EUR/2 through @swap, because the EUR/0 balance hides the 5 EUR.

Scaling postings always use the canonical name (EUR, EUR/2, never EUR/0). A balance whose name isn't in that form can't be spent under the name the posting uses. GetAssets now skips those balances (EUR/0, EUR/02, EUR/x...). Adding them to the canonical balance instead would spend EUR the account doesn't have.

Tests:

  • a unit test for GetAssets with non-canonical names
  • a new case in scaling.num.specs.json: EUR = 4, EUR/0 = 0 must convert the 4 EUR (it failed every time before the fix)

@ascandone
ascandone requested a review from Azorlogh September 30, 2026 08:41
@NumaryBot NumaryBot added risk: medium NumaryBot classified this pull request as medium risk. bot-reviewed NumaryBot completed its review workflow for the current head. review-approved The NumaryBot review gate is satisfied for the current head. labels Sep 30, 2026

@NumaryBot NumaryBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The required automated review completed with no remaining findings.

@ascandone
ascandone enabled auto-merge (squash) October 1, 2026 09:01
@NumaryBot NumaryBot removed risk: medium NumaryBot classified this pull request as medium risk. bot-reviewed NumaryBot completed its review workflow for the current head. review-approved The NumaryBot review gate is satisfied for the current head. labels Oct 1, 2026
@ascandone
ascandone merged commit f3d9a4d into main Oct 1, 2026
5 of 6 checks passed
@ascandone
ascandone deleted the fix/scaling-non-canonical-assets branch October 1, 2026 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants