From 322b58d1daec377bbfae858ac3944449d66ee957 Mon Sep 17 00:00:00 2001 From: akansnoro Date: Thu, 22 Oct 2026 14:29:37 +0100 Subject: [PATCH] feat: re-land issue 1683 (crypto payment resilience) Re-lands the part of the reverted #1703 squash that belongs to issue 1683 only. The remainder of the squash is re-landed by the other branches of the split, so this diff contains this issue's work exclusively. --- src/services/paymentMethodService.ts | 40 ++-- src/services/rpcProvider.ts | 150 ++++++++++++- src/services/transactionConfirmation.ts | 285 ++++++++++++++++++++++++ src/services/walletService.ts | 19 +- src/store/transactionQueueStore.ts | 222 +++++++++++++++--- src/store/walletStore.ts | 53 +++-- src/utils/decimalAmount.ts | 105 +++++++++ 7 files changed, 793 insertions(+), 81 deletions(-) create mode 100644 src/services/transactionConfirmation.ts create mode 100644 src/utils/decimalAmount.ts diff --git a/src/services/paymentMethodService.ts b/src/services/paymentMethodService.ts index c6e90bf5..d116c500 100644 --- a/src/services/paymentMethodService.ts +++ b/src/services/paymentMethodService.ts @@ -1,6 +1,8 @@ import { ethers } from 'ethers'; -import { getEvmRpcUrl } from '../config/evm'; +import { getEvmRpcUrls } from '../config/evm'; +import { getOrCreateResilientProvider } from './rpcProvider'; +import { exceedsLimit } from '../utils/decimalAmount'; import { CHAIN_IDS } from '../utils/constants/values'; import { PaymentMethod, @@ -195,7 +197,7 @@ export class PaymentMethodService { } try { - const provider = new ethers.providers.JsonRpcProvider(getEvmRpcUrl(method.chainId)); + const provider = getOrCreateResilientProvider(method.chainId, getEvmRpcUrls(method.chainId)); const erc20Abi = [ 'function decimals() view returns (uint8)', 'function symbol() view returns (string)', @@ -307,29 +309,35 @@ export class PaymentMethodService { chainId: number ): Promise<{ sufficient: boolean; balance: string; symbol: string }> { try { - const provider = new ethers.providers.JsonRpcProvider(getEvmRpcUrl(chainId)); + const provider = getOrCreateResilientProvider(chainId, getEvmRpcUrls(chainId)); const conn = this.walletManager?.getConnection(); if (!conn) { return { sufficient: false, balance: '0', symbol: method.tokenType }; } let balance: ethers.BigNumber; + // Read the token's own decimals instead of assuming "USDC = 6, + // everything else = 18". Assuming wrong under- or over-reports the + // balance, which triggers spurious fallbacks or lets a charge through + // that cannot settle. + let decimals = 18; if (method.tokenType === TokenType.NATIVE) { balance = await provider.getBalance(conn.address); } else { - const erc20Abi = ['function balanceOf(address) view returns (uint256)']; + const erc20Abi = [ + 'function decimals() view returns (uint8)', + 'function balanceOf(address) view returns (uint256)', + ]; const contract = new ethers.Contract(method.tokenAddress, erc20Abi, provider); + decimals = await contract.decimals(); balance = await contract.balanceOf(conn.address); } - const required = ethers.utils.parseUnits( - requiredAmount, - method.tokenType === TokenType.USDC ? 6 : 18 - ); + const required = ethers.utils.parseUnits(requiredAmount, decimals); return { sufficient: balance.gte(required), - balance: balance.toString(), + balance: ethers.utils.formatUnits(balance, decimals), symbol: method.tokenType.toString(), }; } catch { @@ -342,7 +350,7 @@ export class PaymentMethodService { maxGasPriceGwei: number ): Promise<{ acceptable: boolean; currentGasPrice: string }> { try { - const provider = new ethers.providers.JsonRpcProvider(getEvmRpcUrl(chainId)); + const provider = getOrCreateResilientProvider(chainId, getEvmRpcUrls(chainId)); const gasPrice = await provider.getGasPrice(); const gasPriceGwei = parseFloat(ethers.utils.formatUnits(gasPrice, 'gwei')); @@ -441,10 +449,7 @@ export class PaymentMethodService { continue; } - if ( - method.maxSpendPerInterval && - ethers.BigNumber.from(amount).gt(method.maxSpendPerInterval) - ) { + if (method.maxSpendPerInterval && exceedsLimit(amount, method.maxSpendPerInterval)) { attempt.status = 'failed'; attempt.failureReason = `Amount ${amount} exceeds max spend per interval ${method.maxSpendPerInterval}`; attempt.resolvedAt = new Date(); @@ -485,7 +490,7 @@ export class PaymentMethodService { } try { - const provider = new ethers.providers.JsonRpcProvider(getEvmRpcUrl(method.chainId)); + const provider = getOrCreateResilientProvider(method.chainId, getEvmRpcUrls(method.chainId)); const code = await provider.getCode(method.tokenAddress); const newHash = ethers.utils.keccak256(code); @@ -682,10 +687,7 @@ export class PaymentMethodService { continue; } - if ( - method.maxSpendPerInterval && - ethers.BigNumber.from(amount).gt(method.maxSpendPerInterval) - ) { + if (method.maxSpendPerInterval && exceedsLimit(amount, method.maxSpendPerInterval)) { fail(`Amount ${amount} exceeds max spend per interval ${method.maxSpendPerInterval}`); continue; } diff --git a/src/services/rpcProvider.ts b/src/services/rpcProvider.ts index 246a1436..eb69dc73 100644 --- a/src/services/rpcProvider.ts +++ b/src/services/rpcProvider.ts @@ -73,6 +73,8 @@ interface CircuitEntry { state: CircuitState; consecutiveFailures: number; openedAt: number | null; + /** Consecutive successes observed while HALF-OPEN, used to close the circuit. */ + halfOpenSuccesses: number; } // ───────────────────────────────────────────────────────────────────────────── @@ -88,6 +90,8 @@ export interface ResilientProviderConfig { failureThreshold?: number; /** Ms the circuit stays OPEN before transitioning to HALF-OPEN. Default: 30_000. */ recoveryTimeoutMs?: number; + /** Consecutive successes in HALF-OPEN before closing the circuit. Default: 2. */ + successThreshold?: number; } const DEFAULT_CONFIG: Required = { @@ -95,6 +99,7 @@ const DEFAULT_CONFIG: Required = { jitterMs: 500, failureThreshold: 5, recoveryTimeoutMs: 30_000, + successThreshold: 2, }; /** Returns a sensible timeout for EVM chains with higher RPC variance. */ @@ -132,6 +137,15 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { private readonly _config: Required; private readonly _circuits = new Map(); + /** + * The base ethers provider talks to a single mutable `connection.url`, so + * exactly one `send()` may be swapping that URL at a time. Without this + * chain, two concurrent calls (e.g. `getBalancesAcrossChains`, which fans + * out in parallel) interleave and one of them is sent to the other's + * endpoint — which may be a node on an entirely different chain. + */ + private _swapChain: Promise = Promise.resolve(); + constructor(urls: string[], chainId: number, config: ResilientProviderConfig = {}) { super(urls[0], chainId); this._urls = urls; @@ -141,6 +155,7 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { jitterMs: config.jitterMs ?? DEFAULT_CONFIG.jitterMs, failureThreshold: config.failureThreshold ?? DEFAULT_CONFIG.failureThreshold, recoveryTimeoutMs: config.recoveryTimeoutMs ?? DEFAULT_CONFIG.recoveryTimeoutMs, + successThreshold: config.successThreshold ?? DEFAULT_CONFIG.successThreshold, }; // Initialise circuit entries for every URL @@ -149,6 +164,7 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { state: 'closed', consecutiveFailures: 0, openedAt: null, + halfOpenSuccesses: 0, }); } } @@ -166,6 +182,7 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { const elapsed = Date.now() - (circuit.openedAt ?? 0); if (elapsed >= this._config.recoveryTimeoutMs) { circuit.state = 'half-open'; + circuit.halfOpenSuccesses = 0; } else { errors.push({ url, message: new RpcProviderCircuitOpenError(url).message }); continue; @@ -178,7 +195,17 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { return result; } catch (err) { const message = err instanceof Error ? err.message : String(err); - this._recordFailure(url); + + // Only transport-level problems indicate an unhealthy endpoint. A + // legitimate JSON-RPC failure (revert, insufficient funds, invalid + // params) means the node answered correctly, so counting it towards + // the failure threshold would evict a perfectly healthy URL. + if (isTransportFailure(err)) { + this._recordFailure(url); + } else { + this._recordSuccess(url); + } + errors.push({ url, message }); logger.warn( @@ -194,7 +221,12 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { private _getCircuit(url: string): CircuitEntry { if (!this._circuits.has(url)) { - this._circuits.set(url, { state: 'closed', consecutiveFailures: 0, openedAt: null }); + this._circuits.set(url, { + state: 'closed', + consecutiveFailures: 0, + openedAt: null, + halfOpenSuccesses: 0, + }); } return this._circuits.get(url)!; } @@ -202,13 +234,27 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { private _recordSuccess(url: string): void { const c = this._getCircuit(url); c.consecutiveFailures = 0; - c.state = 'closed'; c.openedAt = null; + + if (c.state === 'half-open') { + c.halfOpenSuccesses += 1; + // Require consecutive successes before trusting a recovered endpoint + // again, so a node that flaps does not ping-pong the circuit. + if (c.halfOpenSuccesses >= this._config.successThreshold) { + c.state = 'closed'; + c.halfOpenSuccesses = 0; + } + return; + } + + c.state = 'closed'; + c.halfOpenSuccesses = 0; } private _recordFailure(url: string): void { const c = this._getCircuit(url); c.consecutiveFailures += 1; + c.halfOpenSuccesses = 0; if (c.state === 'half-open') { // Failed probe — reopen @@ -237,6 +283,39 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), deadline); + // Serialise access to the shared `connection.url`. Each caller waits for + // the previous swap-and-restore to complete before touching it. + const run = this._swapChain.then( + () => this._sendToUrl(url, method, params, controller.signal, deadline), + () => this._sendToUrl(url, method, params, controller.signal, deadline) + ); + + this._swapChain = run.then( + () => undefined, + () => undefined + ); + + try { + return await run; + } finally { + clearTimeout(timer); + } + } + + private async _sendToUrl( + url: string, + method: string, + params: Array, + signal: AbortSignal, + deadline: number + ): Promise { + // The deadline starts when the caller enqueued, so a call that waited + // behind another may already be expired. The abort event has already + // fired in that case, so fail fast instead of waiting unbounded. + if (signal.aborted) { + throw new RpcProviderTimeoutError(url, deadline); + } + // We need to temporarily swap the underlying connection URL so the base // JsonRpcProvider sends to the right endpoint. const previousUrl = this.connection.url; @@ -247,7 +326,7 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { // Race the call against an abort-signal-aware timeout const timeoutPromise = new Promise((_, reject) => { - controller.signal.addEventListener( + signal.addEventListener( 'abort', () => { reject(new RpcProviderTimeoutError(url, deadline)); @@ -258,7 +337,6 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { return await Promise.race([callPromise, timeoutPromise]); } finally { - clearTimeout(timer); // Restore URL in case of future calls to this same provider instance (this as unknown as { connection: { url: string } }).connection.url = previousUrl; } @@ -281,11 +359,73 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { entry.state = 'closed'; entry.consecutiveFailures = 0; entry.openedAt = null; + entry.halfOpenSuccesses = 0; } logger.info(`[ResilientJsonRpcProvider] All circuits reset for chain ${this._chainId}`); } } +/** + * True when an error indicates the endpoint itself is unreachable rather than + * the request being rejected by a working node. + * + * This is what separates "this RPC URL is down" from "this transaction + * reverted", and it is the difference between a circuit breaker that protects + * the user and one that ejects healthy nodes during an incident. + */ +export function isTransportFailure(error: unknown): boolean { + if (error instanceof RpcProviderTimeoutError) return true; + if (error instanceof RpcProviderCircuitOpenError) return false; + + if (!error || typeof error !== 'object') return false; + + const err = error as { + code?: unknown; + error?: { code?: unknown }; + message?: unknown; + reason?: unknown; + }; + + // ethers wraps server responses in `error.code` as a numeric JSON-RPC code. + // Anything that reaches here with one of those is an application-level + // answer from a live node. + const codes = [err.code, err.error?.code]; + if (codes.some((code) => typeof code === 'number' || /^-?\d+$/.test(String(code)))) { + return false; + } + + const transportCodes = [ + 'TIMEOUT', + 'ETIMEDOUT', + 'ESOCKETTIMEDOUT', + 'ECONNREFUSED', + 'ECONNRESET', + 'EHOSTUNREACH', + 'ENETUNREACH', + 'EPIPE', + 'ERR_NETWORK', + 'NETWORK_ERROR', + ]; + if (codes.some((code) => typeof code === 'string' && transportCodes.includes(code))) { + return true; + } + + const text = [err.message, err.reason] + .filter((part): part is string => typeof part === 'string') + .join(' ') + .toLowerCase(); + + return ( + text.includes('failed to fetch') || + text.includes('network error') || + text.includes('fetch failed') || + text.includes('econnrefused') || + text.includes('econnreset') || + text.includes('socket hang up') || + text.includes('getaddrinfo') + ); +} + // ───────────────────────────────────────────────────────────────────────────── // Singleton registry — circuit state persists across calls in the same session // ───────────────────────────────────────────────────────────────────────────── diff --git a/src/services/transactionConfirmation.ts b/src/services/transactionConfirmation.ts new file mode 100644 index 00000000..2c666eb6 --- /dev/null +++ b/src/services/transactionConfirmation.ts @@ -0,0 +1,285 @@ +/** + * transactionConfirmation.ts + * + * Shared, defensive waiting for on-chain transaction receipts. + * + * Every on-chain payment path in SubTrackr previously called `tx.wait()` + * with no arguments and then only asserted that a hash came back. That is + * unsafe in three ways: + * + * 1. `tx.wait()` with no timeout can hang forever if a transaction is + * stuck in the mempool, which also stalls the offline retry queue. + * 2. `receipt.status === 0` means the transaction was *mined but reverted*. + * Treating "mined" as "succeeded" reports a failed payment as a success. + * 3. A single confirmation is not final. On fast chains a reorg can still + * drop the transaction, so users need a chain-appropriate depth. + * + * This module centralises those three concerns so all payment paths behave + * the same way and can be tested in isolation. + */ + +import type { ethers } from 'ethers'; + +import { logger } from './logging'; + +// ───────────────────────────────────────────────────────────────────────────── +// Errors +// ───────────────────────────────────────────────────────────────────────────── + +/** The transaction was mined but the receipt reports `status === 0`. */ +export class TransactionRevertedError extends Error { + readonly code = 'TRANSACTION_REVERTED' as const; + readonly transactionHash: string; + + constructor(transactionHash: string, detail?: string) { + super(`Transaction ${transactionHash} was reverted on-chain${detail ? `: ${detail}` : '.'}`); + this.name = 'TransactionRevertedError'; + this.transactionHash = transactionHash; + Object.setPrototypeOf(this, new.target.prototype); + } +} + +/** No receipt arrived within the allotted timeout. */ +export class TransactionTimeoutError extends Error { + readonly code = 'TRANSACTION_TIMEOUT' as const; + readonly transactionHash: string; + readonly timeoutMs: number; + + constructor(transactionHash: string, timeoutMs: number) { + super( + `Transaction ${transactionHash} was not confirmed within ${timeoutMs} ms. ` + + `It may still be pending on-chain — check your wallet before retrying.` + ); + this.name = 'TransactionTimeoutError'; + this.transactionHash = transactionHash; + this.timeoutMs = timeoutMs; + Object.setPrototypeOf(this, new.target.prototype); + } +} + +/** The transaction was confirmed but never reached the required depth. */ +export class TransactionInsufficientConfirmationsError extends Error { + readonly code = 'TRANSACTION_INSUFFICIENT_CONFIRMATIONS' as const; + readonly transactionHash: string; + readonly actual: number; + readonly required: number; + + constructor(transactionHash: string, actual: number, required: number) { + super( + `Transaction ${transactionHash} has ${actual} confirmation(s) but ${required} are required.` + ); + this.name = 'TransactionInsufficientConfirmationsError'; + this.transactionHash = transactionHash; + this.actual = actual; + this.required = required; + Object.setPrototypeOf(this, new.target.prototype); + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Confirmation policy +// ───────────────────────────────────────────────────────────────────────────── + +/** + * Minimum block confirmations before a payment is treated as settled. + * + * L2s reorg far less often than L1 and produce a block every ~250ms, so + * waiting many blocks would add latency without adding safety. Ethereum + * mainnet uses the deeper default. + */ +export const DEFAULT_CONFIRMATIONS = 2; + +const L2_CHAIN_IDS = new Set([10, 137, 42161, 8453, 43114, 56, 534352]); + +/** Confirmation depth appropriate for `chainId`. */ +export function requiredConfirmations(chainId: number, override?: number): number { + if (override !== undefined) { + return Math.max(1, Math.floor(override)); + } + return L2_CHAIN_IDS.has(chainId) ? 1 : DEFAULT_CONFIRMATIONS; +} + +/** + * Default receipt timeout. Generous enough for congested mainnet, short + * enough that a stuck mempool transaction does not block the queue forever. + */ +export const DEFAULT_CONFIRMATION_TIMEOUT_MS = 5 * 60 * 1000; + +// ───────────────────────────────────────────────────────────────────────────── +// Receipt classification +// ───────────────────────────────────────────────────────────────────────────── + +/** + * ethers surfaces a `wait()` timeout as a `TRANSACTION_REPLACED`-adjacent + * error, a plain `Error` carrying `code === 'TIMEOUT'`, or — depending on + * version — a rejected promise with a `timeout` field. Detection is done on + * shape rather than `instanceof` so that it stays robust across ethers v5/v6. + */ +function isWaitTimeout(error: unknown): boolean { + if (!error || typeof error !== 'object') return false; + + const err = error as { code?: unknown; message?: unknown; reason?: unknown; timeout?: unknown }; + if (err.code === 'TIMEOUT' || err.code === 'ETIMEDOUT' || err.code === 'ESOCKETTIMEDOUT') { + return true; + } + + const text = [err.message, err.reason] + .filter((part): part is string => typeof part === 'string') + .join(' ') + .toLowerCase(); + + return ( + text.includes('timeout') || + text.includes('timed out') || + text.includes('could not detect transaction') || + (typeof err.timeout === 'number' && text.includes('transaction')) + ); +} + +/** + * True when the error means "this transaction failed", as opposed to "we + * could not observe the outcome". Used to decide whether a queued item may + * be retried safely. + */ +export function isDefinitiveFailure(error: unknown): boolean { + return ( + error instanceof TransactionRevertedError || + error instanceof TransactionInsufficientConfirmationsError + ); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Wait +// ───────────────────────────────────────────────────────────────────────────── + +export interface ConfirmTransactionOptions { + chainId?: number; + /** Override the chain-derived confirmation depth. */ + confirmations?: number; + /** Override the receipt timeout. */ + timeoutMs?: number; + /** Human label used in error messages, e.g. `Sablier stream creation`. */ + operation?: string; +} + +/** The subset of a transaction response this module needs. */ +export interface AwaitableTransactionResponse { + hash: string; + wait: (confirmations?: number) => Promise; +} + +function label(options: ConfirmTransactionOptions): string { + return options.operation ? `${options.operation} ` : ''; +} + +/** + * Nodes expose revert reasons in several vendor-specific shapes and ethers v5 + * does not model them on `TransactionReceipt`. Read defensively so a missing + * reason never turns into a crash inside error handling. + */ +function extractRevertReason(receipt: ethers.providers.TransactionReceipt): string | undefined { + const candidate = receipt as unknown as { + revertReason?: unknown; + revertName?: unknown; + }; + + const reason = candidate.revertReason ?? candidate.revertName; + return typeof reason === 'string' && reason.length > 0 ? reason : undefined; +} + +/** + * Waits for `txResponse` to reach the required confirmation depth and proves + * it actually succeeded. + * + * Resolves with the confirmed receipt. Throws `TransactionRevertedError`, + * `TransactionTimeoutError` or `TransactionInsufficientConfirmationsError`. + */ +export async function confirmTransaction( + txResponse: AwaitableTransactionResponse, + options: ConfirmTransactionOptions = {} +): Promise { + const { + chainId, + confirmations = requiredConfirmations(chainId ?? 1), + timeoutMs = DEFAULT_CONFIRMATION_TIMEOUT_MS, + } = options; + + const txHash = txResponse.hash; + + let receipt: ethers.providers.TransactionReceipt | null = null; + try { + // The timeout is enforced here rather than delegated to ethers. In ethers + // v5 `ContractTransaction.wait(confirmations)` does not forward a timeout + // argument at all, so relying on it left every contract-based payment + // (ERC-20 approve, Sablier stream creation) able to hang indefinitely and + // to stall the offline retry queue queued behind it. + let timer: ReturnType | undefined = undefined; + + try { + receipt = await Promise.race([ + txResponse.wait(confirmations) as Promise, + new Promise((_resolve, reject) => { + timer = setTimeout(() => { + reject(new TransactionTimeoutError(txHash, timeoutMs)); + }, timeoutMs); + }), + ]); + } finally { + if (timer) clearTimeout(timer); + } + } catch (error) { + if (error instanceof TransactionTimeoutError) throw error; + if (isWaitTimeout(error)) { + throw new TransactionTimeoutError(txHash, timeoutMs); + } + throw error; + } + + if (!receipt) { + // ethers resolves `null` when the transaction was replaced/dropped rather + // than mined. Treated as a timeout: the outcome is genuinely unknown. + throw new TransactionTimeoutError(txHash, timeoutMs); + } + + // A mined receipt with status 0 is a reverted transaction. Reporting this + // as success is what previously let failed payments look confirmed. + if (receipt.status === 0) { + throw new TransactionRevertedError(txHash, extractRevertReason(receipt)); + } + + const actual = receipt.confirmations ?? confirmations; + if (actual < confirmations) { + throw new TransactionInsufficientConfirmationsError(txHash, actual, confirmations); + } + + return receipt; +} + +/** + * Resolves the transaction hash without failing the surrounding operation. + * + * Used by the offline queue: when a broadcast may have succeeded but the + * receipt could not be observed (app crash, dropped connection), the hash is + * still the durable handle the user needs in order to check on-chain. + */ +export async function bestEffortTransactionHash( + txResponse: AwaitableTransactionResponse | null | undefined, + options: ConfirmTransactionOptions = {} +): Promise { + if (!txResponse?.hash) return undefined; + + try { + await confirmTransaction(txResponse, options); + return txResponse.hash; + } catch (error) { + if (error instanceof TransactionRevertedError) { + // Definitive: the chain says it failed. Surface it, do not swallow it. + throw error; + } + logger.warn( + `${label(options)}receipt for ${txResponse.hash} could not be confirmed: ` + + (error instanceof Error ? error.message : String(error)) + ); + return txResponse.hash; + } +} diff --git a/src/services/walletService.ts b/src/services/walletService.ts index 85941144..86968774 100644 --- a/src/services/walletService.ts +++ b/src/services/walletService.ts @@ -2,10 +2,10 @@ import { ethers } from 'ethers'; import { Framework } from '@superfluid-finance/sdk-core'; import { logger } from './logging'; -import { PaymentMethodService } from './paymentMethodService'; import { ERC20__factory, getContractAddress } from '../contracts'; import { getEvmRpcUrl, getEvmRpcUrls } from '../config/evm'; import { getOrCreateResilientProvider } from './rpcProvider'; +import { confirmTransaction } from './transactionConfirmation'; import { ContractError, ContractErrorCode, NetworkError, NetworkErrorCode } from '../errors'; import { TIME_CONSTANTS, @@ -829,7 +829,10 @@ export class WalletServiceManager { ); const txResponse = await createOp.exec(signer); - const receipt = await txResponse.wait(); + const receipt = await confirmTransaction(txResponse, { + chainId, + operation: 'Superfluid stream creation', + }); if (!receipt?.transactionHash) { throw new Error('Transaction mined without a hash'); @@ -897,7 +900,10 @@ export class WalletServiceManager { ); if (currentAllowance.lt(amountBn)) { const txApprove = await erc20.approve(SABLIER_V2_LOCKUP_LINEAR, amountBn); - await txApprove.wait(); + await confirmTransaction(txApprove, { + chainId, + operation: 'Sablier token approval', + }); } // 3. Create the Sablier Stream @@ -926,7 +932,10 @@ export class WalletServiceManager { }; const txCreate = await sablierContract.createWithDurations(params); - const receipt = await txCreate.wait(); + const receipt = await confirmTransaction(txCreate, { + chainId, + operation: 'Sablier stream creation', + }); if (!receipt?.transactionHash) { throw new Error('Transaction mined without a hash'); @@ -1020,7 +1029,7 @@ export class WalletServiceManager { const erc20 = new ethers.Contract(token, erc20Abi, signer); try { const tx = await erc20.approve(spender, amount); - const receipt = await tx.wait(); + const receipt = await confirmTransaction(tx, { operation: 'Token approval' }); if (!receipt?.transactionHash) { throw new Error('Approval transaction mined without a hash'); } diff --git a/src/store/transactionQueueStore.ts b/src/store/transactionQueueStore.ts index 9d691a1a..7ec8bf9b 100644 --- a/src/store/transactionQueueStore.ts +++ b/src/store/transactionQueueStore.ts @@ -28,6 +28,17 @@ export interface QueuedTransaction { status: 'pending' | 'processing'; payload: QueuedTransactionPayload; lastError?: string; + /** + * Set immediately before the transaction is handed to the wallet. + * + * A broadcast can succeed on-chain even when the app dies before the + * receipt is observed. Persisting this marker first means the next queue + * pass can tell "never sent" apart from "sent, outcome unknown" and refuse + * to re-broadcast the latter — which would otherwise charge the user twice. + */ + broadcastedAt?: number; + /** Earliest time the next attempt may start (exponential backoff). */ + nextAttemptAt?: number; } export interface ExecuteOrQueueResult { @@ -59,7 +70,21 @@ const STORAGE_KEY = 'subtrackr-transaction-queue'; const STALE_TRANSACTION_TIMEOUT_MS = 30 * 60 * 1000; const MAX_ATTEMPTS = 3; +/** First retry delay after a failed attempt. Doubles each subsequent failure. */ +const RETRY_BASE_BACKOFF_MS = 5_000; +const RETRY_MAX_BACKOFF_MS = 5 * 60 * 1000; + +/** + * A queued item that was already handed to the wallet but whose receipt was + * never observed is *not* safe to re-broadcast. It is retired and surfaced + * to the user instead, because the on-chain outcome is genuinely unknown and + * a blind retry risks a second charge. + */ +const UNKNOWN_OUTCOME_TIMEOUT_MS = 10 * 60 * 1000; + let connectivitySubscription: NetInfoSubscription | null = null; +/** Reference count so one consumer unmounting does not silence every other. */ +let connectivitySubscriberCount = 0; const isOnlineState = (isConnected: boolean | null, isInternetReachable: boolean | null): boolean => Boolean(isConnected) && isInternetReachable !== false; @@ -94,6 +119,52 @@ const isLikelyConflictError = (error: unknown): boolean => { ); }; +/** + * Exponential backoff with a ceiling, so a flapping connection cannot drive + * the queue into a tight retry loop that burns through `MAX_ATTEMPTS` in a + * single pass. + */ +const backoffDelayMs = (attempts: number): number => { + const exponent = Math.max(0, attempts - 1); + return Math.min(RETRY_BASE_BACKOFF_MS * 2 ** exponent, RETRY_MAX_BACKOFF_MS); +}; + +/** + * Runs a fire-and-forget promise without producing an unhandled rejection, + * which in React Native terminates the app. + */ +const safely = (promise: Promise, context: string): void => { + void promise.catch((error: unknown) => { + console.warn(`[transactionQueue] ${context} failed`, error); + }); +}; + +/** + * True only when the error proves the transaction never left the wallet. + * + * The broadcast marker may be cleared (and the item retried) in exactly these + * cases. For every ambiguous failure — a dropped connection, an RPC timeout — + * the marker is kept, because the transaction may already be in the mempool + * and re-sending it would charge the user twice. + */ +const isDefinitelyNotBroadcast = (error: unknown): boolean => { + if (!error || typeof error !== 'object') return false; + + const err = error as { code?: unknown; userMessage?: unknown; message?: unknown }; + + const code = typeof err.code === 'string' ? err.code : ''; + if (code === 'USER_REJECTED' || code === 'WALLET_NOT_CONNECTED' || code === 'INVALID_PARAMS') { + return true; + } + + const text = [err.userMessage, err.message] + .filter((part): part is string => typeof part === 'string') + .join(' ') + .toLowerCase(); + + return text.includes('rejected in your wallet') || text.includes('user rejected'); +}; + const executeQueuedPayload = async ( payload: QueuedTransactionPayload ): Promise<{ streamId: string; txHash?: string }> => { @@ -135,10 +206,15 @@ export const useTransactionQueueStore = create()( lastError: null, initializeConnectivityListener: () => { + connectivitySubscriberCount += 1; + if (connectivitySubscription) { return () => { - connectivitySubscription?.(); - connectivitySubscription = null; + connectivitySubscriberCount = Math.max(0, connectivitySubscriberCount - 1); + if (connectivitySubscriberCount === 0) { + connectivitySubscription?.(); + connectivitySubscription = null; + } }; } @@ -148,27 +224,37 @@ export const useTransactionQueueStore = create()( set({ isOnline: online }); if (!wasOnline && online) { - void presentTransactionQueueNotification( - 'Back online', - 'Queued transactions are being processed now.' + safely( + presentTransactionQueueNotification( + 'Back online', + 'Queued transactions are being processed now.' + ), + 'reconnect notification' ); - void get().processQueue(); + safely(get().processQueue(), 'processQueue after reconnect'); } }); return () => { - connectivitySubscription?.(); - connectivitySubscription = null; + connectivitySubscriberCount = Math.max(0, connectivitySubscriberCount - 1); + if (connectivitySubscriberCount === 0) { + connectivitySubscription?.(); + connectivitySubscription = null; + } }; }, refreshConnectivity: async () => { - const state = await NetInfo.fetch(); - const online = isOnlineState(state.isConnected, state.isInternetReachable); - const wasOnline = get().isOnline; - set({ isOnline: online }); - if (!wasOnline && online) { - void get().processQueue(); + try { + const state = await NetInfo.fetch(); + const online = isOnlineState(state.isConnected, state.isInternetReachable); + const wasOnline = get().isOnline; + set({ isOnline: online }); + if (!wasOnline && online) { + safely(get().processQueue(), 'processQueue after connectivity refresh'); + } + } catch (error) { + console.warn('[transactionQueue] connectivity refresh failed', error); } }, @@ -204,11 +290,16 @@ export const useTransactionQueueStore = create()( }; }); - await presentTransactionQueueNotification( - 'Transaction queued', - replacedExisting - ? 'Updated a pending transaction with your latest request.' - : 'Transaction will run automatically once you are online.' + // A failed notification must not lose the queued transaction: the item is + // already persisted above, so this only downgrades to a warning. + safely( + presentTransactionQueueNotification( + 'Transaction queued', + replacedExisting + ? 'Updated a pending transaction with your latest request.' + : 'Transaction will run automatically once you are online.' + ), + 'queued notification' ); return { transactionId, replacedExisting }; @@ -262,19 +353,58 @@ export const useTransactionQueueStore = create()( for (const tx of sortedQueue) { if (!get().isOnline) break; - const age = now() - tx.createdAt; + // Staleness is measured from the last attempt, not creation, so a + // transaction that keeps making progress is not expired out from + // under itself on a long-lived queue. + const reference = tx.lastAttemptAt ?? tx.createdAt; + const age = now() - reference; if (age > STALE_TRANSACTION_TIMEOUT_MS) { set((state) => ({ queuedTransactions: state.queuedTransactions.filter((q) => q.id !== tx.id), })); - await presentTransactionQueueNotification( - 'Queued transaction expired', - 'A pending transaction was removed because it became stale.' + safely( + presentTransactionQueueNotification( + 'Queued transaction expired', + 'A pending transaction was removed because it became stale.' + ), + 'stale notification' + ); + continue; + } + + // Respect exponential backoff so a flapping connection cannot + // burn every attempt in a single pass. + if (tx.nextAttemptAt !== undefined && now() < tx.nextAttemptAt) { + continue; + } + + // A transaction that was already handed to the wallet but whose + // receipt was never observed must not be re-broadcast: the + // on-chain outcome is unknown and a retry would double-charge. + // Retire it and tell the user to verify manually. + if ( + tx.broadcastedAt !== undefined && + now() - tx.broadcastedAt > UNKNOWN_OUTCOME_TIMEOUT_MS + ) { + set((state) => ({ + queuedTransactions: state.queuedTransactions.filter((q) => q.id !== tx.id), + lastError: + 'A queued transaction may have been submitted before its result could be confirmed.', + })); + + safely( + presentTransactionQueueNotification( + 'Queued transaction needs review', + 'It may already have been submitted on-chain. Please check your wallet before retrying.' + ), + 'unknown-outcome notification' ); continue; } + // Mark the broadcast BEFORE handing the payload to the wallet so + // a crash between submission and receipt cannot cause a replay. set((state) => ({ queuedTransactions: state.queuedTransactions.map((queued) => queued.id === tx.id @@ -283,6 +413,8 @@ export const useTransactionQueueStore = create()( status: 'processing', attempts: queued.attempts + 1, lastAttemptAt: now(), + broadcastedAt: queued.broadcastedAt ?? now(), + nextAttemptAt: undefined, updatedAt: now(), lastError: undefined, } @@ -299,9 +431,12 @@ export const useTransactionQueueStore = create()( ), })); - await presentTransactionQueueNotification( - 'Queued transaction sent', - 'A pending transaction has been executed successfully.' + safely( + presentTransactionQueueNotification( + 'Queued transaction sent', + 'A pending transaction has been executed successfully.' + ), + 'success notification' ); } catch (error) { if (isLikelyNetworkError(error)) { @@ -334,9 +469,12 @@ export const useTransactionQueueStore = create()( ), })); - await presentTransactionQueueNotification( - 'Queued transaction skipped', - 'A pending transaction conflicted with another on-chain transaction and was removed.' + safely( + presentTransactionQueueNotification( + 'Queued transaction skipped', + 'A pending transaction conflicted with another on-chain transaction and was removed.' + ), + 'conflict notification' ); continue; } @@ -352,21 +490,32 @@ export const useTransactionQueueStore = create()( lastError: error instanceof Error ? error.message : 'Queued transaction failed.', })); - await presentTransactionQueueNotification( - 'Queued transaction failed', - error instanceof Error - ? `Dropped after retries: ${error.message}` - : 'Dropped after retry attempts.' + safely( + presentTransactionQueueNotification( + 'Queued transaction failed', + error instanceof Error + ? `Dropped after retries: ${error.message}` + : 'Dropped after retry attempts.' + ), + 'dropped notification' ); continue; } + // Schedule the next attempt with exponential backoff. The broadcast marker is + // cleared only when the failure proves nothing was submitted; + // otherwise it is kept so the queue never re-sends a payment + // that may already be in the mempool. + const safeToRetry = isDefinitelyNotBroadcast(error); + set((state) => ({ queuedTransactions: state.queuedTransactions.map((queued) => queued.id === tx.id ? { ...queued, status: 'pending', + broadcastedAt: safeToRetry ? undefined : queued.broadcastedAt, + nextAttemptAt: now() + backoffDelayMs(attempts), updatedAt: now(), lastError: error instanceof Error ? error.message : 'Execution failed.', } @@ -397,7 +546,10 @@ export const useTransactionQueueStore = create()( storage: createJSONStorage(() => asyncStorageAdapter), partialize: (state) => ({ queuedTransactions: state.queuedTransactions }), onRehydrateStorage: () => () => { - void useTransactionQueueStore.getState().refreshConnectivity(); + safely( + useTransactionQueueStore.getState().refreshConnectivity(), + 'rehydrate connectivity refresh' + ); }, } ) diff --git a/src/store/walletStore.ts b/src/store/walletStore.ts index fb64c5bc..fc5b1b8f 100644 --- a/src/store/walletStore.ts +++ b/src/store/walletStore.ts @@ -32,6 +32,30 @@ export interface NetworkMismatch { preferredNetwork: Network; } +/** + * Adapts stored `PaymentAttempt` records to the shape the fallback-chain + * health monitor consumes. + * + * The two shapes previously disagreed: this code read `attempt.success` and + * `attempt.createdAt`, neither of which exists on `PaymentAttempt` (it has + * `status` and `attemptedAt`). Every field resolved to `undefined`, so the + * monitor's look-back window discarded *all* attempts and every payment + * method scored a 100% success rate. Smart fallback ordering was therefore + * permanently inert. + */ +function buildRecentAttempts(attempts: PaymentAttempt[]): Array<{ + paymentMethodId: string; + success: boolean; + timestamp: Date; + latencyMs?: number; +}> { + return attempts.map((attempt) => ({ + paymentMethodId: attempt.paymentMethodId, + success: attempt.status === 'success', + timestamp: attempt.attemptedAt, + })); +} + interface WalletState { // Connection state from service connection: WalletConnection | null; @@ -438,7 +462,7 @@ export const useWalletStore = create()( const updatedMethods = paymentMethods.map((m) => m.id === id ? { ...m, isVerified: true, updatedAt: new Date() } : m ); - set({ paymentMethods: updatedMethods, isLoading: false }); + set({ paymentMethods: updatedMethods }); } return verified; } catch (error) { @@ -449,9 +473,13 @@ export const useWalletStore = create()( : error instanceof Error ? error.message : 'Failed to verify payment method', - isLoading: false, }); throw error; + } finally { + // Cleared unconditionally: previously `isLoading` was only reset + // inside the `if (verified)` branch and the catch, so a `false` + // result left the UI spinning forever. + set({ isLoading: false }); } }, @@ -796,15 +824,11 @@ export const useWalletStore = create()( const chain = get().fallbackChains.find((c) => c.id === chainId); if (!chain) return null; - // Build lightweight attempt objects from stored PaymentAttempts. - const recentAttempts = get().paymentAttempts.map((a) => ({ - paymentMethodId: a.paymentMethodId, - success: a.success, - timestamp: new Date(a.createdAt), - latencyMs: undefined as number | undefined, - })); - - return _chainHealthMonitor.snapshotChainHealth(chainId, chain.methodIds, recentAttempts); + return _chainHealthMonitor.snapshotChainHealth( + chainId, + chain.methodIds, + buildRecentAttempts(get().paymentAttempts) + ); }, getSmartFallbackSelection: (chainId: string) => { @@ -847,6 +871,7 @@ export const useWalletStore = create()( applyAllRotationPolicies: () => { const { rotationPolicies, fallbackChains, paymentAttempts } = get(); + const recentAttempts = buildRecentAttempts(paymentAttempts); const updatedPolicies: PaymentMethodRotationPolicy[] = []; for (const policy of rotationPolicies) { @@ -856,12 +881,6 @@ export const useWalletStore = create()( continue; } - const recentAttempts = paymentAttempts.map((a) => ({ - paymentMethodId: a.paymentMethodId, - success: a.success, - timestamp: new Date(a.createdAt), - })); - const snapshot = _chainHealthMonitor.snapshotChainHealth( chain.id, chain.methodIds, diff --git a/src/utils/decimalAmount.ts b/src/utils/decimalAmount.ts new file mode 100644 index 00000000..fd57d2e8 --- /dev/null +++ b/src/utils/decimalAmount.ts @@ -0,0 +1,105 @@ +/** + * decimalAmount.ts + * + * Exact comparison for fixed-point decimal amounts expressed as strings. + * + * Crypto amounts are user-entered decimals like `"12.5"`. `ethers.BigNumber` + * only accepts integer base units, so `BigNumber.from("12.5")` throws. That + * throw used to be swallowed by the surrounding `catch`, which meant *every* + * fractional amount failed the per-method spend cap and was reported as a + * generic payment error. + * + * Comparing via `parseFloat` instead would reintroduce float error, so the + * comparison below is done on normalised integer coefficient/exponent pairs. + */ + +/** Splits a decimal string into a sign, a digit string and a decimal scale. */ +interface NormalizedAmount { + negative: boolean; + /** Digits with the decimal point removed. */ + digits: string; + /** Number of implied decimal places in `digits`. */ + scale: number; +} + +const DECIMAL_PATTERN = /^[+-]?(\d+(\.\d*)?|\.\d+)$/; + +/** + * Parses a decimal string, or returns `null` when it is not a finite decimal. + * Rejects exponents (`1e18`), blanks, and anything non-numeric. + */ +export function normalizeDecimal( + value: string | number | null | undefined +): NormalizedAmount | null { + if (typeof value === 'number') { + if (!Number.isFinite(value)) return null; + value = value.toString(); + } + + if (typeof value !== 'string') return null; + + const trimmed = value.trim(); + if (trimmed === '' || !DECIMAL_PATTERN.test(trimmed)) return null; + + const negative = trimmed.startsWith('-'); + const unsigned = trimmed.replace(/^[+-]/, ''); + const dotIndex = unsigned.indexOf('.'); + + const whole = dotIndex === -1 ? unsigned : unsigned.slice(0, dotIndex); + const fraction = dotIndex === -1 ? '' : unsigned.slice(dotIndex + 1); + const rawDigits = `${whole}${fraction}`.replace(/^0+(?=\d)/, ''); + + if (rawDigits === '') return null; + + return { + negative: negative && !/^0+$/.test(rawDigits) ? true : false, + digits: rawDigits, + scale: fraction.length, + }; +} + +/** + * Returns -1, 0 or 1 as `a` is less than, equal to, or greater than `b`. + * + * @throws when either side is not a finite decimal string. + */ +export function compareDecimalAmounts( + a: string | number | null | undefined, + b: string | number | null | undefined +): number { + const left = normalizeDecimal(a); + const right = normalizeDecimal(b); + + if (!left || !right) { + throw new Error( + `Cannot compare non-decimal amounts: ${JSON.stringify(a)} vs ${JSON.stringify(b)}` + ); + } + + if (left.negative !== right.negative) { + return left.negative ? -1 : 1; + } + + // Align both sides to the same scale so a plain lexicographic compare works. + const scale = Math.max(left.scale, right.scale); + const leftAligned = left.digits.padEnd(left.digits.length + (scale - left.scale), '0'); + const rightAligned = right.digits.padEnd(right.digits.length + (scale - right.scale), '0'); + + const magnitude = compareDigits(leftAligned, rightAligned); + + return left.negative ? -magnitude : magnitude; +} + +function compareDigits(a: string, b: string): number { + if (a.length !== b.length) return a.length < b.length ? -1 : 1; + if (a === b) return 0; + return a < b ? -1 : 1; +} + +/** True when `amount` exceeds `limit`. Both must be finite decimals. */ +export function exceedsLimit( + amount: string | number | null | undefined, + limit: string | number | null | undefined +): boolean { + return compareDecimalAmounts(amount, limit) > 0; +}