Release: develop -> main - #4465
Merged
Merged
Conversation
…er posting (#4452) * fix(ledger): add the missing Binance custody assets for ONDO and ADA 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. * fix(ledger): check the target row before the price source in the custody 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. * docs(ledger): say which environments actually seed the custody rows 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.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
July 29, 2026 16:17
…hots (#4464) * perf(ledger): project asset prices in SQL instead of loading whole snapshots LedgerMarkService.preload loaded complete log rows and parsed each message in Node, only to keep three values per asset: created, assetId and priceChf. A snapshot carries 200 assets in an average 42 KB JSON document. Measured on the real two-day preload window in production: 930 rows and 38 MB transferred, of which 7.3 MB is the data actually used. The JSON.parse over those 38 MB runs on the event loop and is a direct cause of the API container sitting at a saturated core, which slows every request. The new repository method projects the prices in SQL via LATERAL jsonb_each and returns flat rows. The LIMIT and the keyset cursor stay on the log rows, not on the expanded result, so pagination semantics are unchanged — the cursor therefore carries the originating log id. getLatestMarks keeps using getFinancialLogs: it runs on the dailySample path, where the row count is already small and the added complexity would not pay for itself. Mark map contents, ordering and the handling of missing or non-finite prices are unchanged. * fix(ledger): keep every read log row countable and tighten price validation Review follow-ups on the SQL price projection. A log row whose assets yield no usable price used to vanish from the projection entirely. Overflow detection and keyset pagination count log rows, so a vanished row made them undercount: preload could miss that more data exists, and paginate could stop early — a silent loss of marks. The projection now uses LEFT JOIN LATERAL, so every read log row comes back even when it carries no usable price, and the counting is correct again. This also removes a related false end-of-data path in the stale-cursor guard. Price conversion now matches the previous Number.isFinite semantics: a price is only taken when the JSON value really is a number, so a string "1.25" is skipped as before rather than silently accepted by the float cast. Non-numeric asset keys no longer abort the whole query; they yield null and are skipped. Malformed message JSON now fails loudly instead of being swallowed — deliberate, and documented on the method. buildMarkMap keeps Number.isFinite as a second line of defence. The repository already nulls NaN and Infinity, but a NaN mark would corrupt a valuation silently, so that must not rest on a single guard. The test fake mirrors the SQL semantics instead of masking it, and three tests cover the cases: a log row without usable prices between two valid ones, a string price, and a non-numeric asset key.
…since hand-off (#4455) * test(scrypt): cover the cooldown interval formula and the since hand-off Both properties were provably untested. Replacing `ageMs / 10` and the 30-minute cap with the flat one-minute floor left the suite green, and so did making getOrderStatus ignore its `since` argument and fetch the fixed 30-day window again — every existing cooldown test uses a freshly created order, where the interval is always the floor, and the adapter tests mock getOrderStatus away entirely. - pipeline service: a 100-minute-old order still owes its 10-minute wait after 9 minutes, and an 8-hour-old one is asked again after 31 because the cap holds it at 30 rather than the uncapped 48 - scrypt service: a caller-supplied `since` reaches the venue request as StartDate, and omitting it still yields the 30-day window * test(scrypt): pin the cooldown rate and cap from both sides Review follow-up. Both cooldown tests asserted only the lower side of the interval, so a mutation that ENLARGES it stayed green: with `ageMs / 5` the order simply remains in cooldown and a "still waiting" assertion keeps passing, and the cap test is dominated by the cap whatever the divisor. Each test now brackets its exact threshold. For a 100-minute-old order the wait is satisfied once elapsed >= (100 min + elapsed) / 10, i.e. at 11 min 6.7 s — hence one call at 11 minutes and two at 11 min 20 s. The cap is bracketed at 29 min 59 s and 30 min 1 s, which also pins the boundary against a cap set too low. * test(scrypt): land the cap check on the boundary itself The cap test bracketed 29 min 59 s and 30 min 1 s but never the boundary between them, which is the only place `<` and `<=` differ: at exactly 1_800_000 ms elapsed the production check admits the lookup, a `<=` variant would hold it back, and neither assertion point could tell them apart. Moving the second check onto 30 minutes exactly pins that too. * test(scrypt): pin the cap to the millisecond Checking at 29 min 59 s left the cap a whole second of room: any value between 1_799_001 and 1_799_999 ms produced the same two observations as the real 1_800_000, so a cap set slightly too low passed unnoticed. Checking one millisecond before the boundary closes that gap entirely. Also corrects the comment's arithmetic: 48 minutes is the uncapped wait at the first pass; by the boundary check the order has aged another 30 minutes and its uncapped wait is 51. Both are far past the cap, so the conclusion held, but the number did not. * test(scrypt): say what the cap check proves, more carefully The comment claimed the bracket leaves the cap "no room to sit anywhere else". That holds for every whole millisecond, which is every value the constant can actually take, but the test alone does not rule out a fractional one — so say whole-millisecond and mean it. Also fixes the grammar of the preceding clause.
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
1 participant
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.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist