Release: develop -> main - #4343
Merged
Merged
Conversation
* Fix Firo Open CryptoPay minimum fee rejecting Spark payments The Firo customer-facing Open CryptoPay minimum was taken from DFX's own payout fee rate (estimateSmartFee times the CPFP/default margin, ~2.068 sat/vB). Firo Spark transactions carry a protocol-fixed fee at the network relay minimum (~1 sat/vB) that the user cannot raise, so valid Spark payments were rejected. Use Firo's relay floor as the customer minimum (configurable via FIRO_MIN_FEE_RATE, default 1 sat/vB); the payout margin stays on DFX's own payout path. Bitcoin keeps its margin-based minimum because its fees are user-adjustable. * Decouple Bitcoin Open CryptoPay minimum from payout fee margin For consistency with the Firo fix, the customer-facing Open CryptoPay minimum for Bitcoin no longer derives from DFX's payout send rate (which carries a CPFP/default margin meant only for DFX's own outbound spends). Use the network's recommended next-block rate, floored at the relay minimum so the advertised minimum stays relayable. The payout margin remains on DFX's own payout/payin/dex paths, unchanged. * Derive Firo Open CryptoPay minimum from node rate instead of hardcoded floor The current OCP Firo deposit address is transparent, so a Stack Wallet payment is a Spark-spend to it whose fee sits at the relay floor and cannot be raised. Mirror the Bitcoin approach: use Firo's own estimatesmartfee(1) without the payout CPFP margin, floored at the relay minimum, instead of a hardcoded FIRO_MIN_FEE_RATE constant. On a quiet Firo node estimatesmartfee returns null (the normal state) and degrades to the relay floor; a genuine node/RPC error propagates so Firo fails closed (drops out of the fee cache) like Bitcoin rather than being advertised at 1. Removes the now-unused FIRO_MIN_FEE_RATE config and adds PayoutFiroService tests for the estimate/null/error branches.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
July 23, 2026 15:40
* fix(scrypt): self-heal withdrawal completion after a missed WS event (#4310) A Scrypt withdrawal whose BalanceTransaction completion event is missed (a WS drop at the wrong moment) could never complete: getWithdrawalStatus read only the in-memory balance-transaction cache that the subscription fills, and getAllTransactions (the 5-minute EXCHANGE_TX_SYNC) read the same cache. A missed event therefore left the liquidity-management order polling forever and its in-flight amount double-counted in the balance snapshot until a full process restart. Give getWithdrawalStatus the same fresh-fetch fallback getOrderStatus already has — fetch from the Scrypt API on a cache miss, then cache the result — and make getAllTransactions fetch balance transactions fresh instead of reading the cache, reusing the existing connection.fetch(BALANCE_TRANSACTION) transport the constructor warm-up already uses. Either path now heals a missed event within minutes instead of never. Also reorder checkWithdrawCompletion so a FAILED/REJECTED status fails the order before the missing-txHash early return: a rejected withdrawal carries no txHash, so it previously returned false and polled forever instead of failing the order. * fix(scrypt): heal a missed withdrawal completion via the fresh 5-minute sync (#4310) Rework the self-heal after review. getWithdrawalStatus's cache-miss fetch could never fire: withdrawFunds() seeds a non-terminal balance-transaction cache entry via the permanent subscriber, so the cache is never empty for a stuck withdrawal, and a getOrderStatus-style cache-miss fallback is dead code for the exact "missed completion event" case this targets. Heal through the 5-minute EXCHANGE_TX_SYNC instead: getAllTransactions now fetches balance transactions fresh via fetchAll (full pagination, not a truncated single page), refreshes the cache conditionally so a non-terminal fetched record never overwrites a terminal cached one (a fetch cannot regress a completed withdrawal it raced against a live event), and on a fetch failure degrades loudly to the last-known-good cache instead of throwing (which would also discard the caller's concurrent trade fetch). getWithdrawalStatus reverts to a pure cache read; the FAILED/REJECTED reorder in the adapter is unchanged. * fix(scrypt): drop the fetch self-heal; keep only the rejected-withdrawal reorder (#4310) Adversarial review showed the fetch-based cache self-heal is unsound for balance transactions: the BalanceTransaction stream's schema forbids a StartDate filter (additionalProperties: false), so a since-filtered fetch is invalid and a wider window would still miss older stuck withdrawals; and fetch/fetchAll open an uncancelled server-side subscription, so running it per poll or every 5 minutes leaks streams. Revert scrypt.service.ts (getWithdrawalStatus and getAllTransactions) to their original cache-based form and drop the corresponding tests. This PR now carries only the completion-check reorder: evaluate FAILED/REJECTED before the missing-txHash early return, so a rejected withdrawal (which has no txHash) fails the order via OrderFailedException instead of polling forever. The heal for a missed completion event is handled correctly by a fetchAll catch-up on WS reconnect (unfiltered, as the constructor warm-up already does), which lands in the reconnect-resilience PR. * test(scrypt): use Object.assign entity and add a FAILED-without-txHash regression test (#4310) pr-ready review: build the order via Object.assign(new LiquidityManagementOrder(), ...) matching the repo convention, and add a dedicated FAILED-without-txHash case so both terminal statuses are pinned against the missing-txHash early return.
#4310) (#4338) * fix(log): clamp the unfiltered pending legs to match the filtered ones (#4310) The filtered pending legs (fromKraken/toKraken/fromScrypt/toScrypt) are each clamped to 0 when negative, but their unfiltered counterparts were not. When useUnfilteredTx is true, an unclamped negative unfiltered leg (observed: asset 405 approx -5.0M) drove totalPlusPending negative and fired the 'totalPlusPending < 0' verbose log every minute. Clamp the four unfiltered legs symmetrically, placed after the filtered/unfiltered discrepancy comparisons so those still compare raw values. The root cause of a persistently-negative unfiltered leg is a separate follow-up. * fix(log): log the unfiltered clamp components and cover the useUnfilteredTx path (#4310) pr-ready review: the four new unfiltered-leg <0 verbose logs now include their constituent pending-amount breakdown (matching the filtered siblings), which the separate follow-up into the persistently-negative unfiltered leg will need; and add a test exercising the useUnfilteredTx=true clamp path (previously entirely untested). * test(log): isolate the per-leg unfiltered-toKraken clamp from the aggregate clamp (#4310) pr-ready round 2: the negative-leg clamp test was vacuous — the downstream aggregate totalPlusPending<0 clamp floors the same scenario, so reverting the per-leg clamp still left it green. Assert (via the verbose log) that the per-leg 'toKrakenUnfiltered balance < 0' path fires and the aggregate 'totalPlusPending < 0' path does NOT, so the test now fails if the per-leg clamp is removed.
…ckoff (#4310) (#4331) * fix(scrypt): resubscribe on every WebSocket reconnect and retry with bounded backoff (#4310) The Scrypt WS reconnect had two gaps that let a dropped connection silently stop delivering events for the rest of the process lifetime: - The scheduled reconnect was single-shot: one setTimeout -> connect(), whose .catch only logged. After a single failed reconnect (e.g. a 401) nothing ever retried. - resubscribeToStreams ran only from that dead single-shot path. Any implicit reconnect via ensureConnected() (a business call such as a price fetch) rebuilt the raw socket without restoring subscriptions, so the socket healed while the BalanceTransaction stream stayed unsubscribed. Move resubscription into connect()'s success path so every re-connect restores the streams, guarded by hasEverConnected so the first connect (whose subscriptions are sent directly by the initial subscribe() calls) does not double-send. Replace the single-shot reconnect with a bounded-backoff loop (5s doubling, capped at 60s) that retries indefinitely and logs reconnect success, guarded by isReconnecting against overlapping loops. resubscribeToStreams no longer clears activeStreams up front, so a transient per-stream subscribe failure leaves the stream queued for the next reconnect instead of dropping it permanently. An explicit fetchAll catch-up on reconnect (to recover events missed during the outage) and the fetch/fetchAll subscription-cancel fix follow in a separate PR. * fix(scrypt): share reconnect readiness and fail loud on a mid-resubscribe drop (#4310) Rework the reconnect after review found two concurrency blockers: - A caller joining an in-flight reconnect (connect()'s CONNECTING branch) received the raw handshake promise, so it could send a money-path request as soon as the socket opened — before the streams were resubscribed, risking a missed confirmation push. connectionPromise now covers full readiness via establishConnection (handshake -> revalidate -> resubscribe on a re-connect -> revalidate -> CONNECTED); connectionState stays CONNECTING until the end, so a joiner waits for resubscription instead of proceeding on a half-ready socket. - resubscribeToStreams swallowed all errors and connect() never revalidated the socket after it, so a drop mid-resubscribe resolved connect() as a false "reconnected" and left the connection dead with no retry. establishConnection now asserts the socket is open after resubscription, so a mid-resubscribe drop rejects and the backoff loop retries. resubscribeToStreams sends on the current socket directly (sendSubscriptionOnSocket, no ensureConnected) so it cannot recurse into connect(). Also: a 15s handshake timeout so a black-hole connect cannot wedge the loop; full jitter on the backoff with an escalation log every 10 attempts; and disconnect() now cancels a pending reconnect timer. * fix(scrypt): close a mid-establish reconnect race that could spawn duplicate sockets (#4310) A socket drop DURING establishConnection (state CONNECTING) previously flipped connectionState to DISCONNECTED, letting a concurrent connect() start a second establishConnection while the first was still in flight; the stale first attempt's rejection then clobbered the second's freshly-installed state, spawning duplicate sockets and inconsistent connection state. handleDisconnection now only resets connectionState when the drop hit an already CONNECTED socket, so a mid-establish close leaves state CONNECTING and concurrent callers keep joining the in-flight connectionPromise instead of starting a second establish. connect()'s catch additionally only resets state if it still owns the in-flight promise, so a stale establishConnection's rejection can never clobber a newer attempt. Strengthen the drop-mid-resubscribe test to emit a real close event (so the real handleDisconnection runs) and add a concurrency test asserting no second establishConnection is started; both were mutation-checked (they fail when the fix is reverted). * fix(scrypt): tie reconnect attempts to a generation token so stale socket events cannot corrupt a newer connection (#4310) A review found a residual class of races: a socket event (open/close) or an establishConnection step from an abandoned connection attempt could still mutate the shared connection state that a newer attempt (or disconnect()) had moved on to — e.g. a superseded socket's delayed close nulling a newer healthy this.ws and scheduling a bogus reconnect, or disconnect() failing to cancel an in-flight attempt or its backoff. Give every connection attempt a generation (connectionGeneration): connect() mints a new one, the socket's open/close listeners and establishConnection capture it, and they only mutate shared state while it is still current — a superseded socket's open terminates instead of being adopted, its close is ignored, and establishConnection aborts. disconnect() bumps the generation to supersede any in-flight attempt, and the backoff loop short-circuits on isReconnecting. This closes the class at its root (a missing attempt identity) instead of patching individual interleavings. These were only reachable via disconnect() (no production callers today) or an extremely narrow handshake-timeout timing, so this is latent hardening — but it makes the state machine correct if disconnect() is ever wired into a shutdown path. * fix(scrypt): reset connection state in disconnect() before the pre-open early return (#4310) disconnect() bumped the generation and cleared the reconnect timer, but during a pre-open handshake (this.ws still undefined) it early-returned without resetting connectionState/connectionPromise — so a later connect() joined the superseded, doomed promise via the single-flight guard instead of starting a fresh attempt. Reset connectionState = DISCONNECTED and connectionPromise = undefined unconditionally, before the `if (!this.ws) return`. * fix(scrypt): address pr-ready review — log the error object, full disconnect cleanup, doc/naming (#4310) - scheduleReconnect passes the error as the logger's second argument so the reconnect-failure class keeps its stack trace and span.recordException. - disconnect() clears pendingRequests/subscriptions/activeStreams unconditionally (only ws.close() is gated), so a pre-open-handshake disconnect fully resets state. - correct the establishConnection comment (close-before-open is bounded by the handshake timeout, not a direct reject); drop the now-unnecessary Array.from in resubscribeToStreams; rename 'full jitter' -> 'equal jitter'; document the latent subscribe-while-disconnected double-send on subscribeToStream. * fix(exchange): scope scrypt reconnect loop to an epoch so a superseded attempt cannot disturb a newer loop (#4310) pr-ready round 2: the reconnect loop guarded its timer/then/catch continuations on the bare isReconnecting flag, so a disconnect() that supersedes an in-flight attempt and is followed by a fresh drop+loop could let the stale attempt's late continuation clear the live loop's isReconnecting or overwrite its reconnectTimer. Scope every scheduleReconnect continuation to a reconnectEpoch (bumped on disconnect and at each new loop start) so a superseded attempt no-ops. Also reword the subscribeToStream JSDoc to the real already-active-stream invariant and correct a stale flushPromises test comment. * fix(exchange): clear reconnect state on full connect and reset reuse flag on disconnect (#4310) pr-ready round 3: two confirm-review findings. (1) isReconnecting/reconnectTimer were reset only in scheduleReconnect's own success continuation, so a business call that healed the socket via ensureConnected()->connect() during the backoff window left them stale — clear both once establishConnection reaches CONNECTED, whichever path connected. (2) disconnect() did not reset hasEverConnected, so reusing the connection (disconnect then subscribeToStream) double-sent the SUBSCRIBE frame — reset it for a clean full-reset. Both pinned by mutation-checked tests.
Author
ℹ️ New TODOs/FIXMEs (1)+ address: yapealCHF.bic.padEnd(11, 'XXX'), |
Prod-only data migration lowering the global lmActivationDelay runtime setting from 15 to 10. This is the debounce window a liquidity-management rule's deficit/redundancy condition must persist before a fund-moving pipeline starts; it is global (affects all liquidity rules). Guarded to ENVIRONMENT === 'prd' (the setting row may be absent on dev/loc/CI). up() is fail-loud on row existence and the '10' post-state; the prior value is intentionally not asserted because the setting is runtime-mutable. down() best-effort restores 15.
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