Skip to content

40947f62 - Create the missing Binance custody assets that wedged ledger posting - #4452

Merged
TaprootFreak merged 3 commits into
developfrom
fix/ledger-binance-custody-assets-and-manual-ident-sync
Jul 29, 2026
Merged

40947f62 - Create the missing Binance custody assets that wedged ledger posting#4452
TaprootFreak merged 3 commits into
developfrom
fix/ledger-binance-custody-assets-and-manual-ident-sync

Conversation

@TaprootFreak

@TaprootFreak TaprootFreak commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Problem

Ledger posting for exchange_tx has been wedged in production since 2026-07-28 20:40 UTC.

exchangeAssetByCcy resolves the ledger account by name and fail-closes when it is missing:

Failed to book exchange_tx 145948: Error: Ledger account Binance/ONDO not found (CoA bootstrap missing)

processForward catches that and breaks out of the batch, deliberately leaving the watermark
untouched so the row is retried. With a permanently missing account the retry never succeeds, so
ledgerWatermark.exchange_tx froze at lastProcessedId: 145947 and every later row stopped posting —
regardless of which exchange it belongs to. The content-change scan breaks on the same row, so
status flips of already-booked rows are no longer reversed either.

At the time of writing that is 163 unposted exchange_tx rows worth about CHF 358k, growing by
roughly one every few minutes, plus one error per minute from each of the two scans.

Root cause

Every asset held on an exchange needs a second asset row with type='Custody' and
blockchain=<ExchangeName>, next to its on-chain row — Monero/XMR has MEXC/XMR and Kraken/XMR,
Solana/SOL has Binance/SOL, and so on. LedgerBootstrapService.bootstrapAssetAccounts() iterates
over asset rows, so it can only create a CoA account once that row exists.

Ethereum/ONDO has no Binance/ONDO counterpart, so no account was ever created. The recurring
LEDGER_COA_BOOTSTRAP cron cannot heal this: it creates accounts for assets, not assets themselves.
ONDO traded on Binance before (May, June) without a wedge because those trades predate the ledger
cutover and are covered by the aggregate opening — 145948 is the first one after it.

Checking every symbol and deposit/withdrawal currency traded over the last 120 days against the
existing custody assets turned up exactly one more gap: Binance/ADA. It has not wedged yet only
because its last Binance trade also predates the cutover. Both are created here so the second one
does not reproduce this incident on the next ADA trade.

Change

A data migration creating the two missing custody rows. Everything else already exists.

  • Idempotent per uniqueName, so a re-run is a no-op.
  • Advisory lock first. uniqueName is not DB-unique — the unique index covers
    (dexName, type, blockchain) — and this migration has no ENVIRONMENT guard, so two instances
    starting at once could both pass the idempotency check and have the second INSERT crash on the
    index. Same approach as AddSavingZchfAsset.
  • No ENVIRONMENT guard. Unlike AddBankFrickCustodyAssets these rows carry no
    LiquidityManagementRule, no bank link and no external side effect; they are correct in every
    environment. migration/seed/asset.csv mirrors them so a freshly seeded local database matches
    (the seed runs on LOC only; CI never touches it).
  • Prices via subquery from each coin's on-chain row. All rows of one coin share a price rule, so
    Binance/ONDO inherits from Ethereum/ONDO and Binance/ADA from Cardano/ADA. A fail-loud guard
    rejects a missing or renamed source row rather than inserting an unpriced asset. It deliberately does
    not require the source to carry a priceRuleId: an empty one is a legitimate state that the subquery
    carries over verbatim — migration/seed/asset.csv ships Ethereum/ONDO exactly like that.
  • No LiquidityManagementRule. Neither asset is held as a balance — both are pass-through, and
    neither has a liquidity_balance row. Adding a feed would be a product decision, not part of this fix.
  • refundEnabled is false, matching every live custody asset (the TRUE values on the older
    Binance rows in the seed CSV are a stale snapshot).

Effect

Once deployed, the LEDGER_COA_BOOTSTRAP cron creates both accounts within five minutes, the
watermark advances past 145948 and the backlog posts through. All backlogged rows already carry a
persisted amountChf, so they book at their real CHF values rather than needing a mark.

Verification

  • format:check, lint, type-check and the affected suites run clean.
  • Migration spec covers the emitted SQL (advisory lock ordering, idempotency, both fail-loud guards)
    plus a real-Postgres block behind MIGRATION_TEST_PG for create / idempotency / rollback, including
    the case where a target row already exists but its price source is gone — up() resolves instead of
    throwing, and each asset is handled independently of the other.
  • Watermark state, the missing rows, the two price sources and the backlog size were all read back
    from production before writing this.

Every asset held on an exchange needs a second `asset` row with type='Custody'
and blockchain=<ExchangeName>, next to its on-chain row. Both were missing for
ONDO and ADA on Binance, so LedgerBootstrapService — which iterates over `asset`
rows — could never create their CoA accounts.

Without the account, exchangeAssetByCcy fail-closes with "Ledger account
Binance/ONDO not found (CoA bootstrap missing)" and processForward breaks out of
the batch, leaving the exchange_tx watermark frozen: every later row stays
unposted, whatever exchange it belongs to. The recurring CoA bootstrap cron
cannot heal this — it only creates accounts for `asset` rows that exist.

ADA has the same gap and only escaped the wedge because its last Binance trade
predates the ledger cutover.

The migration is idempotent per uniqueName and takes a transaction-scoped
advisory lock first: uniqueName is not DB-unique (the unique index covers
(dexName, type, blockchain)) and there is no ENVIRONMENT guard, so two instances
starting at once could otherwise both pass the idempotency check and have the
second INSERT crash on the index. Prices come from each coin's on-chain row via
subquery, with a fail-loud guard instead of a silent NULL priceRuleId.
…ody migration

up() claimed to be idempotent per uniqueName but evaluated the price-source guard
first, so a re-run against an already-created asset still threw once its source
had been renamed or removed. Checking the target first — as AddSavingZchfAsset
does — makes the claim true and keeps the guard on the path that actually inserts.
Both assets stay independent: an existing ONDO row no longer skips ADA.

Also narrows the guard's doc comment to what it really rejects, a missing or
renamed source row. It does not require the source to carry a priceRuleId, and an
empty one is legitimate — the seed ships Ethereum/ONDO exactly like that, and the
subquery carries the NULL over on purpose.
@Danswar
Danswar self-requested a review July 29, 2026 15:02
The header claimed dev, loc and CI all mirror the two rows from
migration/seed/asset.csv. Only LOC does: main.ts runs the seed exclusively for
Environment.LOC and seed.js blocks everything else outright, so dev, CI and prod
receive the rows from this migration alone.
@TaprootFreak

Copy link
Copy Markdown
Collaborator Author

Three review passes were needed to reach zero findings.

Pass 1 raised three points, all on the migration: the price-source guard was documented as preventing a silent NULL priceRuleId when it only checks that the source row exists, up() claimed to be idempotent per uniqueName while evaluating that guard before the target check, and the description of which environments read the seed CSV was wrong.

The guard itself was left alone on purpose — the seed ships Ethereum/ONDO with an empty priceRuleId, so tightening it would break a freshly seeded database over a state that is legitimate, and both sources carry a price rule where it matters. The doc was corrected instead. The target check now runs before the guard, matching AddSavingZchfAsset, with a new real-Postgres test for the case it repairs: an existing target whose price source is gone resolves instead of throwing, and each asset stays independent of the other.

Pass 2 found one more inaccuracy in the same area — the header still claimed dev and CI mirror the rows from the seed CSV, which only LOC does. Pass 3 came back clean on both lenses.

@TaprootFreak
TaprootFreak marked this pull request as ready for review July 29, 2026 15:57
@TaprootFreak
TaprootFreak merged commit 95bf197 into develop Jul 29, 2026
13 checks passed
@TaprootFreak
TaprootFreak deleted the fix/ledger-binance-custody-assets-and-manual-ident-sync branch July 29, 2026 16:16
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