fix(ledger): scope native-balance check to single-currency transfers#4268
Merged
TaprootFreak merged 2 commits intoJul 18, 2026
Merged
Conversation
checkNativeBalance is meant — per its own docstring — to sanity-check pure same-asset transfers (all legs ASSET/TRANSIT of ONE currency): then Σ native must be 0. It only gated on onlyAssetTransit and then looped per currency, never checking the tx is single-currency. A fee-less cross-asset micro-fill (feeAmountChf rounds to 0 and the base/quote CHF mark residual is within tolerance → no spread/fee leg) collapses to a bare 2-leg all-ASSET tx and was flagged in both currencies, for a per-currency delta that is the trade itself, not an imbalance — 44 of the 46 spurious post-cutover lines. Add the missing single-currency gate to restore the documented scope, and judge the residual in CHF at the group mark instead of natively (mirroring the §7 reconciliation unit-fix): a bare native tolerance flags sub-rappen fiat rounding — a fiat bank leg carries the unrounded output at >2 dp — yet is far too loose for BTC. That removes the remaining 2 buy_fiat lines while still catching a genuine single-currency imbalance, now reported valued in CHF. An unvalued tx falls back to the native tolerance so a real imbalance is never silently passed. Diagnostic-only: bookings and the CHF invariant are unchanged.
On the mark=0 fallback the CHF value is 0 by construction; print "unvalued (mark 0)" so the residual is not mistaken for a negligible valued amount during triage. Diagnostic text only.
Collaborator
Author
|
Reviewed across two dimensions (conformance/quality + logic/correctness), two passes to zero findings.
CI green (Build and test, review, CodeQL, analyze). Verified locally on Node 20: |
TaprootFreak
marked this pull request as ready for review
July 18, 2026 15:48
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.
Closes #4267.
Problem
checkNativeBalanceis meant — per its own docstring — to sanity-check pure same-asset transfers (all legs ASSET/TRANSIT of one currency): then Σ native must be 0. But it only gated ononlyAssetTransitand then looped per currency, never checking the tx is single-currency.A cross-asset trade is also all-ASSET/TRANSIT. It normally carries a non-asset spread/fee leg (EXPENSE/INCOME) →
onlyAssetTransit=false→ check skipped. But a fee-less micro-fill —feeAmountChfrounds to0and the base↔quote CHF mark residual is within the rounding tolerance → no spread/fee leg — collapses to a bare 2-leg all-ASSET tx. The check then ran and flagged each currency's (necessarily non-zero) trade delta as an "imbalance". That delta is the traded amount, not a fee → the log is spurious.All 46 post-cutover
native imbalance … (programming error)lines reconcile to 24 committed txs: 22 cross-asset micro-fills flagged in both currencies (44 lines) + 2 fiat settlements (2 lines). Bookings committed correctly throughout — the CHF invariant (amountChfSum = 0) is enforced; this is diagnostic-only.Fix
if (byCurrency.size !== 1) return;) — restores the exact scope the docstring already describes and removes the 44 trade-driven false positives. A second currency means a cross-asset trade, where the per-currency native delta is the trade itself and never 0 (§2.3 Major R9-2).native journal↔feed diff MUST be valued in CHF before comparing against the tolerance). A bare native tolerance flags sub-rappen fiat rounding (a fiat bank leg carries the unrounded output at >2 dp — the 2 remaining lines) yet is~52'000×too loose for BTC. An unvalued tx (no mark) falls back to the native tolerance so a real imbalance is never silently passed. A genuine single-currency imbalance is still flagged, now reported valued in CHF.No behavioural or data change to bookings; the native amount persisted for a leg is untouched.
Design note
An alternative for point 2 was to keep the native comparison and special-case fiat currencies (round the fiat leg's native amount to 2 dp, or widen the tolerance for a fiat-ticker allow-list). Valuing in CHF was preferred because it (a) needs no hard-coded fiat list, (b) handles both fiat and crypto with one mechanism, and (c) is exactly the unit treatment the ledger already applies in §7 reconciliation.
Tests
Added to
ledger-booking.service.spec.ts:Existing native-imbalance and value-boundary cases stay green (33/33).
Verification (Node 20)
prettierclean ·eslintclean ·type-checkpass ·buildpass ·jest ledger-booking.service.spec33/33 · DI graph resolves (boot reaches JwtStrategy env, i.e. past DI).