Skip to content

fix(accounting): resolve cutover marks by currency and guard every opening namespace against retry re-opens#4253

Merged
TaprootFreak merged 4 commits into
developfrom
fix/ledger-cutover-mark-resolution
Jul 16, 2026
Merged

fix(accounting): resolve cutover marks by currency and guard every opening namespace against retry re-opens#4253
TaprootFreak merged 4 commits into
developfrom
fix/ledger-cutover-mark-resolution

Conversation

@TaprootFreak

@TaprootFreak TaprootFreak commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Problem

The production ledger cutover has been stuck in its fail-loud retry loop since the release, aborting every 5 minutes on:

Cutover opening <logId>:buy_fiat-owed:<rowId> without a mark would silently drop the value (CHF liability, never revalued by mark-to-market) — retry when the mark feed is available

openBuyFiatOwed resolved the output mark from row.outputAsset?.id. But buy_fiat.outputAsset is a Fiat (@ManyToOne(() => Fiat)), while LedgerMarkCache is keyed by asset id (its points come from the FinancialDataLog, whose assets map the LogJob writes from Asset entities). The two tables' ids collide: fiat 2 is EUR, asset 2 is DeFiChain/dBTC. Every foreign-currency owed opening therefore missed its mark and aborted the run.

No value was ever mispriced: the snapshot's asset keys start at 111 and the DeFiChain legacy assets below 24 carry no approxPriceChf, so the lookup could only ever miss — the m6 fail-loud guard did exactly its job. That is a property of today's data, not a structural guarantee, which is why the fix removes the cross-namespace lookup rather than patching around it.

Three further defects are fixed together, because fixing only the first one walks straight into the next.

bankMark had no untracked-bank resolution. A bank without an asset, or a bank_tx whose accountIban matches none, yielded no mark — and openUnattributed aggregates its rows, so a single such credit aborts the whole opening. Several EUR banks in production have no asset. The forward bank-tx consumer already resolves this via a representative same-currency asset (§4.2a); the cutover did not.

The per-row opening namespaces were not mutually exclusive across retries — on both source paths. :buy_fiat:, :buy_fiat-owed: and :buy_fiat-paymentLink: (and likewise :buy_crypto: / :buy_crypto-owed:) are disjoint sourceId namespaces, but the routing between them is mutable and re-evaluated on every retry while the snapshot stays pinned: received vs. owed routes on outputAmount, paymentLink vs. the rest on cryptoInput.paymentLinkPayment. A row that moved bucket between two runs got a second opening under the new sourceId — alreadyBooked only guards its own namespace — silently double-counting equity with no alarm. Both directions are real code paths (payout routing sets outputAmount; resetAmlCheck clears it). On the buy_crypto path the consequence is worse than a double count: once an owed opening exists the forward consumer skips seq0 and seq1, so the received opening would never close and would leave a permanent phantom liability on top of the doubled equity.

A comment asserted a rollback that does not happen. Per-row openings each commit in their own transaction and stay committed after a throw; idempotency rests entirely on the pinned snapshot plus alreadyBooked. That false claim is precisely why the namespace gap above went unnoticed.

Fix

  • currencyMark(currency, …) replaces fiatMark(assetId, …): CHF is valued 1:1, any other currency resolves through a representative same-currency tracked-bank asset (§4.2a — the mark is identical across same-currency bank assets, so any tracked bank supplies it; the representative pick mirrors the forward consumer's id ASC order, which is the actual correctness condition — a different pick would leave the liability unable to close). A missing currency returns undefined so the caller still fails loud; there is deliberately no ?? CHF fallback, because a priced row without a currency is a data anomaly, not a CHF row.
  • bankMaps() replaces bankByIban() and returns both views over the same single bank read — no extra query, no new relation, and notably no dependency on fiatOutput, which openBuyFiatOwed's population may legitimately lack ("output already priced but not yet routed to a payout bank").
  • bankMark falls back to the currency mark for untracked or unmatched banks, mirroring the forward consumer's mark resolution. A tracked bank whose asset carries no mark at the snapshot date still fails loud — that branch intentionally has no fallback, matching the consumer.
  • alreadyOpenedInAnyNamespace runs at the top of all four per-row loops (buy_fiat and buy_crypto, received and owed), deriving exclusivity from what is already booked rather than from the mutable routing. This is correct regardless of when it deploys, including for rows that switch bucket beforehand, and it also stops an already-opened row from being re-pinned as unpriced.
  • The m6 error text drops its — retry when the mark feed is available suffix: after this fix a missing mark no longer indicates a feed outage but a structural gap (no tracked bank for the currency, an unmarked bank asset, or an unknown currency), so the operator-visible message must not suggest waiting it out.
  • The fail-loud rationale comments now state what the code does: which conditions actually resolve no mark, and that a failed run aborts with its openings committed rather than rolling back.

Tests

The existing R6-1 test seeded the mark cache and the outputAsset from the same invented id ({ id: 7, name: 'EUR' } as any), asserting the id collision as the specification — a mocked test of that shape cannot catch a cross-namespace confusion. It now uses a real bank asset id distinct from the Fiat id, without as any, and thereby also covers an owed row priced without a payout bank. Added: an unattributed EUR credit on an untracked bank; the namespace guard in both directions for buy_fiat, for paymentLink and for buy_crypto; an already-opened row not being re-pinned as unpriced; and a priced row without an output currency failing loud.

Ops note

After deploy the cutover completes idempotently against the already-pinned snapshot; the openings booked in the earlier runs are correct by construction and are skipped on the retry. Verified against production: all affected EUR rows route via Olkypay, whose bank asset carries a valid mark in the pinned snapshot, and the mark cache resolves it through the same representative the forward consumer picks.

…nk asset

openBuyFiatOwed looked a Fiat id up in the asset-keyed mark cache (fiat 2 =
EUR, asset 2 = DeFiChain/dBTC), so every foreign-currency owed opening missed
its mark and aborted the cutover. Resolve the mark through the currency's
representative tracked-bank asset instead — the same value the forward
consumers book — and let bankMark fall back to it for untracked or unmatched
banks, which would otherwise wedge the aggregated unattributed opening.

The three per-row buy_fiat opening namespaces are now mutually exclusive
across retries: their routing (outputAmount/fiatOutput) is mutable and
re-evaluated on every fail-loud retry while the snapshot stays pinned, so a
row that moved bucket between runs got a second opening under the new
sourceId and silently double-counted equity. Both directions are real paths,
so the guard checks all three namespaces before booking.

Also drop the claim that a failed run rolls the cutover back: per-row
openings commit in their own transactions and stay committed.
…ry re-opens

The buy_crypto path carries the same defect as buy_fiat: :buy_crypto: and
:buy_crypto-owed: are disjoint sourceId namespaces routed by the mutable
outputAmount, so a row that moves bucket between two fail-loud retries gets a
second opening. There it is worse than a plain double-count — the forward
consumer skips seq0 and seq1 once an owed opening exists, leaving the
received opening permanently unclosed.

Generalise the guard over a namespace list and run it at the top of all four
loops, so an already-opened row is never re-pinned as unpriced either.

Also correct the rationale comments: the cutover routes on outputAmount and
cryptoInput.paymentLinkPayment, never on fiatOutput, and the method that
clears outputAmount is resetAmlCheck, not resetSendingInfo.
Three sibling comments still claimed a missing mark rolls the cutover back —
the same false claim this branch already corrected on bookReceivedOwedOpening,
and the one that let the double-open gap go unnoticed. They also named a
no-bank-match as the miss condition, which the currency fallback now resolves.
…missing bank mark

The unattributed aggregate does not defer an unvaluable credit to
mark-to-market — it throws, and the CHF liability bucket it would have
written is never revalued. The miss condition also kept a live disjunct: a
tracked bank whose asset carries no mark at the snapshot date resolves no
mark either, and that branch has no currency fallback, mirroring the forward
consumer.
@TaprootFreak

Copy link
Copy Markdown
Collaborator Author

Review protocol: 5 review passes until zero findings.

Pass 1/2 surfaced the substantive one: the same double-open gap this PR closes on buy_fiat was still open on buy_crypto:buy_crypto: vs :buy_crypto-owed: share the identical mutable outputAmount routing, and there the consequence is worse (the forward consumer skips seq0 and seq1 once an owed opening exists, so the received opening would never close). Production exposure was 145 open rows, not 93. Fixed by generalising the guard over a namespace list and running it at the top of all four loops, which also stops an already-opened row from being re-pinned as unpriced. Two rationale comments were factually wrong and were corrected: the cutover routes on outputAmount and cryptoInput.paymentLinkPayment, never on fiatOutput, and the method clearing outputAmount is resetAmlCheck, not the non-existent resetSendingInfo.

Pass 3/4 were comment-only but load-bearing: three sibling comments still claimed a failed run rolls the cutover back — the same false claim this branch corrects on bookReceivedOwedOpening, and the reason the double-open gap stayed invisible in the first place. The follow-up fix then had to be corrected twice more: it kept a false consequence clause (an unvaluable unattributed credit does not defer to mark-to-market — it throws) and dropped a still-live disjunct from the miss condition (a tracked bank whose asset carries no mark at the snapshot date resolves no mark either, and that branch intentionally has no currency fallback, mirroring the forward consumer).

Pass 5 clean on both dimensions, including an enumeration confirming no third sibling path exists and that the guard blocks no legitimate first opening.

Verification: format/lint/type-check green, accounting suites green (34 suites, 760 tests). Mark resolution and the representative-asset pick were verified against production: fiat id 2 = EUR vs asset id 2 = DeFiChain/dBTC confirms the collision, and all affected EUR rows route via Olkypay, whose bank asset carries a valid mark in the pinned snapshot.

@TaprootFreak
TaprootFreak marked this pull request as ready for review July 16, 2026 21:53
@TaprootFreak TaprootFreak changed the title fix(accounting): resolve cutover fiat marks through the currency's bank asset fix(accounting): resolve cutover marks by currency and guard every opening namespace against retry re-opens Jul 16, 2026
@TaprootFreak
TaprootFreak merged commit fd60674 into develop Jul 16, 2026
7 checks passed
@TaprootFreak
TaprootFreak deleted the fix/ledger-cutover-mark-resolution branch July 16, 2026 22:04
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.

1 participant