Release: develop -> main - #4450
Merged
Merged
Conversation
* fix(kyc): skip the Sumsub file sync for non-Sumsub ident steps reviewIdentSteps selects every ident step in internal review, regardless of its type, and syncs the ident files before persisting the new status. syncIdentFilesInternal throws for any step that is not a Sumsub one, so a manual ident lost its manual-review transition and stayed in internal review - where the every-minute review picked it up and failed it again, indefinitely. Only call the sync for Sumsub steps: a manual ident uploads its document in updateIdentManual and never has an IDENT_REPORT to sync. The ordering stays as it is, so a failing sync still keeps a Sumsub step in internal review to be retried. * fix(kyc): document the intentional sync-before-save ordering, tighten the tests - state in the comment that running the sync before the save is deliberate, so the guard is not mistaken for a workaround and the ordering is not "cleaned up" later - cover that ordering: a Sumsub step whose sync fails must stay unsaved for a retry - assert the status the repo actually receives; the entity is already mutated in memory before the sync, so the previous assertion held with the bug present - use the SumSubLevelName enum instead of a level name that cannot occur - name the spy after the method it replaces, add the dep-scope comment the other blocks in this file carry, and drop the unused Config bootstrap * fix(kyc): cover both Sumsub ident types and make the retry test self-validating - run the two Sumsub cases over SumsubAuto and SumsubVideo: a guard narrowed to one of them passed the previous tests, while the other type would have completed without any file and without an error - assert in the retry case that the sync was reached, so its two absence assertions cannot hold because the iteration never got there - type savedStatus as optional, since undefined is the "never saved" sentinel - state in the dep-scope comment which stubs the short dep list depends on * fix(kyc): pin the remaining legs of the ident file-sync condition Mutation-checking the previous tests showed four weakenings of the condition that no test noticed: replacing the type guard with !isManual, dropping the already-has-a-report check, matching a different file subType, and dropping the isCompleted leg. Each of them re-breaks a real path. - run the non-Sumsub case over Manual and both legacy IdNow types - cover a step that already carries its ident report, i.e. the normal case in which the webhook downloaded it before the review runs - cover a completing step, which needs its files even more than one in manual review: it leaves internal review for good and is never revisited * fix(kyc): pin the ident file-sync condition against its last three weakenings Three more mutations of the condition passed the previous cases: collapsing the report check to "the account has no files at all", and replacing the whole status clause with true or with !isFailed. The first would disable the review-time sync for nearly every account, the other two would pull documents for an ignored ident. - give the no-report cases an unrelated file, so an empty file list cannot stand in for a missing ident report - cover the ignored outcome, the third one the review can reach - separate the auto and video IdNow fixtures, which are told apart by companyid * fix(kyc): use a synthetic step id and an exhaustive fixture map - the fixture carried a real step id from the incident; fixtures stay synthetic - map every ident type without the optional modifier, so a sixth type fails to compile here instead of silently reaching the review without a result
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
July 29, 2026 13:03
* feat(custody): expose order timestamps in the account history The order history DTO carried no timestamp at all, so a client had no way to date an entry. Pass both the creation date and the valuta timestamp separately rather than collapsing them: an order only gets completedAt once it completes, and the distinction between when an order was recorded and when it was valuated is exactly what a reader needs. * test(custody): pin the history mapper timestamps The valuta timestamp is only ever set once an order completes. A later refactoring that collapses it onto the creation date would claim a valuta that never happened, so assert the undefined case explicitly.
* fix(scrypt): back off quarantined-order venue lookups and release the never-sent order 127664 A quarantined order whose reference the venue does not know pays a full 30-day execution-report fetch on every reconciliation pass — several per 10-second tick via the pipeline loop, measured at ~300 heavy fetches per hour against the same connection whose failure usually caused the quarantine in the first place. - resolveUncertainOrders: per-order cooldown, a tenth of the order's age clamped to [1 min, 30 min], stamped when the lookup FINISHES (success, inconclusive and thrown alike); a pending manual release bypasses it - getOrderStatus: optional 'since' lower bound for the fallback fetch; the uncertain-order path passes order.created minus one day, never wider than the previous fixed 30-day window - migration: release order 127664 — provably never sent (its stored failure is thrown before any bytes leave the process, the venue never knew the reference across 8 h of lookups, the EUR position never drained and was manually converted at the venue). Compare-and-set guards make it a no-op if the admin endpoint releases it first; correlationId + created window pin the exact production row. * fix(scrypt): drop the release migration and scope cooldown stamps to one quarantine episode Review follow-ups: - the data migration for order 127664 is removed: the order was released via the admin endpoint on 2026-07-28 (row is Failed), so the status compare-and-set makes the migration a guaranteed no-op — and the only state it could ever match again is a blockConfirmedOrder re-quarantine, which must never be flipped to Failed - a cooldown stamp now lives for exactly one quarantine episode: it is cleared when the order's exit write lands (leaveQuarantine), so an order quarantined anew gets its first venue lookup immediately instead of inheriting up to 30 minutes of old wait - two mutation-killing tests: the cooldown measures from the END of a slow lookup (a start-stamp mutant fails), and a re-quarantined order starts a fresh cooldown (removing the exit-write deletion fails)
…4446) * fix(ledger): bound financial-log reads in SQL instead of after loading LedgerMarkService paginated financial-log snapshots through windows, but getFinancialLogs had no upper bound and no limit in SQL. Every window re-read all rows from its start date to today - including the message column at ~5.8 KB per row - and discarded the excess in JavaScript. In production that filter matches 32'757 rows carrying 311 MB of JSON, while the filter scan itself costs only 134 ms. Push `to` and `limit` into the query and replace the window loop with a keyset cursor. The cursor is the id of the last row of the previous page; its `created` value is resolved in a correlated subquery instead of round-tripping through a JS Date. log.created is timestamp(6) and 4999 of 5000 sampled production rows carry a microsecond remainder that a Date truncates - comparing against the truncated value would re-include the cursor row on every page and, at markPreloadMaxRows = 1, never terminate. A missing cursor row fails loud rather than yielding an empty page that callers would misread as end-of-data. On overflow the probe read is reused as the first page instead of being discarded and read again. The returned row set is unchanged for identical parameters. * fix(ledger): validate the preload row cap and detect a stale cursor reliably Review follow-ups on the bounded financial-log reads. markPreloadMaxRows comes from an env var and was never validated. At 0 the probe read sliced to an empty first page and LIMIT 0 returned nothing, so preload() silently built an empty mark cache while rows existed. It is now required to be a positive integer and fails loud otherwise. The cursor existence check ran before the main query, which left a window in which cleanup() could delete the cursor row: the query then saw a NULL cursor, returned no rows, and pagination read that as end-of-data. The check now runs only after an empty result with a cursor set, which closes that window and drops the extra round-trip on every non-empty page. The pagination fake filtered on id alone while the query compares (created, id) lexicographically, so a later row with a lower id would have been dropped in the test but returned in production. It now resolves the cursor row and compares the same way, with a test covering non-monotonic ids. Also bounds the cutover snapshot read at `to` in SQL rather than filtering in JavaScript afterwards, now that the parameter exists. * test(ledger): annotate the cutover snapshot mock return type
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
2 participants
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