Skip to content

feat(spark): self-custodial payments and transaction history #3758

Open
esaugomez31 wants to merge 76 commits intofeat--spark-backup-and-recoveryfrom
feat--spark-payments-and-transaction-history
Open

feat(spark): self-custodial payments and transaction history #3758
esaugomez31 wants to merge 76 commits intofeat--spark-backup-and-recoveryfrom
feat--spark-payments-and-transaction-history

Conversation

@esaugomez31
Copy link
Copy Markdown
Collaborator

@esaugomez31 esaugomez31 commented Apr 14, 2026

PR Summary — Self-Custodial Payments and Transaction History

Feature: #647 — Payments and Transaction History
Epic: 4 — Bitcoin Payments & History

This PR delivers the full send, receive, routing, transaction-history and offline-blocking behavior for self-custodial accounts across Lightning, on-chain and Spark flows. It sits on top of feat--spark-backup-and-recovery and closes every task in #647.

Task checklist

Task Description Status
4.1 Self-custodial transaction normalizer Done
4.2 Lightning payment adapters — Receive Done
4.2 Lightning payment adapters — Send Done
4.3 On-chain receive: deposit address, claim flow, below-minimum guidance Done
4.4 On-chain send: fee tier selector + withdrawal Done
4.5 Spark payment adapters Done
4.6 Payment routing and destination wrapping Done
4.6 Offline blocking (FR37) Done
4.7 All Transactions feed integration Done

What's delivered

4.1 — Transaction normalizer

  • Enriched NormalizedTransaction with memo, lnAddress, tokenTicker, isConversion and sourceAccountType.
  • mappers/transaction-mapper.ts normalizes Breez Payment into NormalizedTransaction, with extractors for Lightning, Spark and Token details, plus currency inference (mapCurrency), direction, status and fee handling.
  • amounts.ts gains tokenBaseUnitsToCents and toSatsAmount helpers used across receive/send flows.
  • PaymentMethod.Deposit and PaymentMethod.Withdraw map to PaymentType.Onchain.

4.2 — Lightning send + receive

  • bridge/receive.tscreateReceiveLightning, createReceiveOnchain.
  • bridge/send.tsprepareSend, executeSend, lightning / onchain fee extractors.
  • hooks/use-payment-request.ts — SC receive hook with Lightning invoice + on-chain address generation, memo/amount, payment detection via provider events, URI formatting through shared buildLightningUri / buildBitcoinUri. Generation is gated on active-wallet readiness and wrapped in try/catch so adapter throws surface as Error state.
  • payment-details/lightning.ts + payment-details/send-helpers.ts — SC lightning send payment-detail factory consuming the bridge.
  • receive-screen.tsx and send-bitcoin-* screens wired through useOnchainResolver, use-send-wallets, use-onchain-fee-tier-options and wrap-destination.
  • Apollo wiring: use-payments.ts registers SC adapters (receiveLightning, receiveOnchain, sendLightning, sendOnchain, deposits, convert). use-display-currency.ts and graphql/client.tsx fall through cleanly for SC users.

4.3 — On-chain receive, claim and below-minimum guidance

  • bridge/deposits.ts — list / claim / refund unclaimed deposits, with error classification for below-minimum and other failure states.
  • adapters/deposit-adapter.ts — adapter exposing claim/refund to the UI.
  • components/unclaimed-deposit-banner/ + integration in home-screen.tsx.
  • screens/unclaimed-deposits/ — full flow: list screen, use-deposit-actions, use-recommended-fee-tiers, deposit-error-message for below-min / fee-estimation errors.

4.4 — On-chain send: fee tier selector

  • screens/send-bitcoin-screen/fee-tier-selector.tsx — 3-tier UI (Slow / Medium / Fast).
  • hooks/use-onchain-fee-tiers.ts, use-onchain-fee-tier-options.ts, format-fee-tier-options.ts — fee quoting and formatting.
  • payment-details/onchain.ts — SC onchain payment-detail factory with selected tier propagated through confirm/send.

4.5 — Spark payment adapters

  • bridge/parse.ts + bridge/convert.ts — input parsing and stable-balance conversions.
  • payment-destination/spark.ts and payment-details/spark.ts — Spark destination resolver and payment-detail factory.
  • payment-details/wrap-destination.ts — routes a resolved destination to the matching SC factory (lightning / onchain / spark).
  • adapters/payment-adapter.ts — unifies send adapter behind the same shape the custodial flow uses.

4.6 — Payment routing

  • navigation/navigation-container-wrapper.tsxcanHandlePayments = isAuthed || isSelfCustodial, so deep links open for SC users.
  • navigation/root-navigator.tsx + stack-param-lists.ts — unclaimed-deposits route registered.
  • send-bitcoin-destination-screen.tsx — destination wrapping applied for SC accounts before navigation.

4.6 — Offline blocking (FR37)

Delivered in this PR's final 5 commits:

  • bridge/status.ts — wraps Breez SDK's getSparkStatus() returning the worst status across Spark Operators + SSP. No new deps; no third-party pings.
  • providers/is-online.ts — maps ServiceStatus: Operational / Degraded → online, Partial / Unknown / Major or SDK throw → offline.
  • providers/use-sdk-lifecycle.ts:
    • OFFLINE_EXEMPT_STATUSES (Error, Unavailable) never transition to Offline.
    • refreshWallets performs an online check before hitting the SDK; if offline, sets Offline without touching local cache.
    • Polling every 10s while mounted triggers refreshWallets so offline detection does not require user interaction.
    • AppState → active also triggers a refresh for immediate recovery when returning from background.
  • hocs/with-offline-gate.tsx — memoized HOC (WeakMap cache per Component → stable reference required by React Navigation, no intermediate constants). Renders ScPaymentOfflineNotice when isSelfCustodial && status === Offline; passes through otherwise. Custodial users are untouched.
  • components/sc-payment-offline-notice/ — title + description + "Try again" button wired to refreshWallets.
  • navigation/root-navigator.tsx — 9 payment routes wrapped inline (scanningQRCode, sendBitcoinDestination, sendBitcoinDetails, sendBitcoinConfirmation, receiveBitcoin, redeemBitcoinDetail, conversionDetails, conversionConfirmation, unclaimedDepositsScreen).
  • Reverted getWalletInfo to ensureSynced:false so startup doesn't hang when Spark operators are unreachable — the polling detects offline instead.

4.7 — All Transactions feed

  • mappers/to-transaction-fragment.ts — maps NormalizedTransaction[] into Apollo TransactionFragment[] for cache compatibility.
  • mappers/transaction-description.ts — human-readable description resolver.
  • transaction-history-screen.tsx — SC and custodial transactions render in the same feed.
  • transaction-detail-screen.tsx — hides "Blink Internal Id" for SC transactions.
  • wallet-snapshot.ts — paginated fetch, isKnownPayment filter, getStableBalance accepts both Map and Record, plus appendTransactions helper used by loadMore.

SDK lifecycle hardening

  • providers/sdk-events.ts — extracted REFRESH_EVENTS, PAYMENT_RECEIVED_EVENTS and extractPaymentId.
  • providers/validate-network.ts — extracted network-match guard (legacy wallets without a stored network are allowed).
  • providers/use-sdk-lifecycle.ts — refactored around the extracted helpers; getUserSettings is deferred off the critical init path, refreshWallets is exposed, loadMore now appends via appendTransactions.
  • logging.ts — suppresses the noisy "Received empty event" SDK log line.
  • config.tsSparkNetworkLabel exported and reused in bridge + validator.
  • Fixes landed here: infinite loading on first init, receive screen re-triggering Paid on re-entry, onchain fee-estimation error classification surfaced to the UI.

Infrastructure and shared utilities

  • app/utils/bitcoin-uri.ts — shared buildBitcoinUri, buildLightningUri, satsToBtc.
  • app/screens/receive-bitcoin-screen/hooks/use-onchain-resolver.ts — resolves on-chain data from either SC or custodial hooks transparently.
  • i18n — new keys for fee tiers, unclaimed deposits, SC transaction descriptions, and SelfCustodialOffline notice copy across every supported language (28 locales).
  • __mocks__/@breeztech/breez-sdk-spark-react-native.js — mock kept in sync with new enum values (ServiceStatus), payment-detail tags and getSparkStatus.

Testing

~300 suites passing with full coverage added for every domain touched:

  • Normalization: transaction-mapper, to-transaction-fragment, transaction-description.
  • Bridge: bridge.spec.ts, bridge/deposits, bridge/parse, bridge/send, bridge/wallet (ensureSynced:false contract), bridge/status (delegation + error propagation), bridge-mainnet-guard.
  • Adapters: adapters/deposit-adapter, adapters/payment-adapter.
  • Payment details: payment-details/lightning, payment-details/onchain, payment-details/send-helpers, payment-details/wrap-destination.
  • Providers:
    • providers/sdk-events, providers/validate-network.
    • providers/wallet-snapshotappendTransactions filtering + immutability.
    • providers/is-online — one test per ServiceStatus variant + error path.
    • providers/wallet-provider — refresh coalescing, payment-id propagation, loadMore, Ready↔Offline transitions, Error/Unavailable preservation under offline, Loading→Offline initial path, 10s polling interval (fake timers), polling cleanup on unmount, AppState active triggers refresh / background does not.
  • HOC: hocs/with-offline-gate — custodial pass-through, SC+Offline notice, SC+Ready pass-through, prop forwarding, displayName, cache stability per component.
  • Component: components/sc-payment-offline-notice — renders i18n copy, retry button calls refreshWallets (fireEvent, idempotent).
  • Hooks: hooks/use-payment-request (create, error, adapter-throw, ready-gate, re-entry, full cycle), hooks/use-payments, hooks/use-send-wallets, hooks/use-onchain-fee-tiers, hooks/format-fee-tier-options.
  • Screens / utils: transaction-history-dates, payment-destination/spark, receive-bitcoin/helpers, utils/bitcoin-uri, types/to-sats-amount.
  • Cross-cutting: config.spec.ts (network label + storage scoping), logging.spec.ts (level routing + empty-event suppression).

Known follow-ups (out of scope for this PR)

  • Balance staleness indicator (FR44): when getSparkStatus reports Degraded/Partial but the user remains online enough to reach services, the balance may be computed as 0 because the wallet-tree sync is incomplete. This affects multiple users and is independent of this PR. Tracked separately with analysis in ~/Desktop/sc-balance-stale-issue.txt. Will land as a dedicated ticket.
  • A few environment-level files remain uncommitted on purpose (feature-flags-context.tsx defaults, graphql/generated.ts codegen drift, settings-screen dev section, dev-reset-wallet) — those are not part of this feature's scope.

@esaugomez31 esaugomez31 changed the title feat: add transaction metadata fields to NormalizedTransaction feat(spark): self-custodial payments and transaction history Apr 14, 2026
@esaugomez31 esaugomez31 marked this pull request as ready for review April 14, 2026 02:33
@esaugomez31 esaugomez31 marked this pull request as draft April 14, 2026 02:33
@esaugomez31 esaugomez31 marked this pull request as ready for review April 16, 2026 14:00
@esaugomez31 esaugomez31 force-pushed the feat--spark-payments-and-transaction-history branch from be1bb8e to ef45a9a Compare April 17, 2026 02:28
@esaugomez31 esaugomez31 requested a review from grimen April 17, 2026 05:21
@esaugomez31 esaugomez31 force-pushed the feat--spark-backup-and-recovery branch from 69ec015 to 73cbca0 Compare April 17, 2026 15:10
@esaugomez31 esaugomez31 force-pushed the feat--spark-payments-and-transaction-history branch 2 times, most recently from eefb0dc to a1f58d0 Compare April 17, 2026 15:37
Copy link
Copy Markdown
Contributor

@grimen grimen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: Self-Custodial Payments and Transaction History

Overall this is a solid implementation delivering all spec requirements. The architecture is well-separated and test coverage is comprehensive.


Issues (6)

1. Potential race condition in deposit claiming

Problem: UnclaimedDepositBanner fetches deposits in useEffect without subscribing to wallet refresh. If a deposit is claimed elsewhere, the list becomes stale.

Solution: Invalidate deposit list on wallet refresh events or use shared state.


2. Missing i18n key validation across 28 locales

Problem: 28 locale files updated with identical +64/-1 changes. High likelihood of missing keys, typos, or mismatched placeholders ({count}, {sats}).

Solution: Add CI validation comparing all locales against en.json as source of truth.


3. WeakMap cache without size limit

Problem: withOfflineGate uses WeakMap to cache wrapped components. Grows unboundedly during session.

Solution: Replace HOC with wrapper component (see Code Smells #6) - eliminates need for cache entirely.


4. BigInt to Number conversion risk

Problem: Bridge layer converts SDK BigInt to Number:

// SDK returns
amountSats: BigInt(5000)
// App uses
result.amountSats // number

Could lose precision for values > Number.MAX_SAFE_INTEGER.

Solution: Keep as BigInt through the stack, or add runtime validation that values are within safe range.


5. Inconsistent error handling pattern

Problem: Bridge throws, adapters return result objects:

// Bridge - throws
await claimDeposit({ sdk, txid, vout })  // throws Error

// Adapter - returns
const result = await adapter.claimDeposit(...)  // { status: 'failed', errors }

Solution: Standardize on one pattern. Recommend adapters catch bridge errors and always return result objects.


6. Duplicate PaymentType import source

Problem:

// Some files
import { PaymentType } from "@blinkbitcoin/blink-client"

// Other files  
import { PaymentType } from "@app/types/..."

Solution: Use single source. Prefer internal types with re-export if needed.


Code Smells (6)

Issue Example Proposed Fix
Long function chains wrapDestinationForSC nested conditionals Extract payment-type handlers to separate functions
Excessive test mocking wallet-provider.spec.tsx has 20+ mocks Create shared createMockSelfCustodialContext() fixture
String-based error classification if (err.message.includes("SdkError.InsufficientFunds")) Use typed error classes or error codes
Inconsistent "SC" abbreviation ScPaymentOfflineNotice vs useSelfCustodialWallet Use full SelfCustodial consistently
Utility in hooks directory hooks/format-fee-tier-options.ts is pure function Move to utils/
HOC pattern hocs/with-offline-gate.tsx with WeakMap cache Replace with <OfflineGate> wrapper component

Test Gaps (3)

  1. Edge cases: Network switching mid-transaction, concurrent payments, app backgrounding during execution
  2. Missing scenarios: Accessibility, 0-fee tiers, malformed SDK responses, duplicate transaction IDs
  3. Uncovered files: deposit-error-message.tsx, use-onchain-resolver.ts

@esaugomez31
Copy link
Copy Markdown
Collaborator Author

esaugomez31 commented Apr 22, 2026

@grimen

Review Response — Self-Custodial Payments and Transaction History


Issues (6)

1. Potential race condition in deposit claiming — Addressed

UnclaimedDepositBanner now re-fetches:

  • On useFocusEffect (covers the user returning from the unclaimed-deposits screen after claiming).
  • Whenever useSelfCustodialWallet().wallets changes identity (i.e. after any ClaimedDeposits / NewDeposits SDK event that triggers refreshWallets).

Commit: fix(unclaimed-deposits): refetch banner on wallet refresh and screen focus.

2. Missing i18n key validation across 28 locales — Partially addressed

The missing keys and placeholders for this PR are aligned across the 28 locales. The broader CI validation tool you suggested we'll land in a dedicated PR together with the cleanup of the ~1.1k drifts that already exist in legacy translations — adding it to check-code today would block every merge on pre-existing issues.

3. WeakMap cache without size limit — Addressed

Replaced via Smell #6 (see below). The HOC + WeakMap are gone; the <OfflineGate> wrapper component has no memoization and relies on the 9 module-level gated-screen constants in root-navigator.tsx for stable references.

Commit: refactor(self-custodial): replace withOfflineGate HOC with OfflineGate wrapper component.

4. BigInt to Number conversion risk — Addressed

toNumber now records a Crashlytics error when a bigint input exceeds Number.MAX_SAFE_INTEGER, so any precision loss surfaces in telemetry.

if (!Number.isSafeInteger(parsed) && typeof value === "bigint") {
  crashlytics().recordError(
    new Error(`toNumber: bigint value ${value} exceeds Number.MAX_SAFE_INTEGER`),
  )
}

In practice all Bitcoin (≤ 21M × 10⁸ sats) and USDB base-unit amounts stay well within MAX_SAFE_INTEGER, but the hook is there if that ever shifts.

Commit: fix(utils): record crashlytics error when toNumber loses BigInt precision.

5. Inconsistent error handling pattern — Acknowledged, intentional

The adapter/bridge split is intentional and consistent within each layer:

  • Bridge wraps the SDK and throws (raw layer).
  • Adapter composes bridge calls and returns PaymentAdapterResult (UI-facing layer).

The 5 hooks/screens that call bridge directly (use-create-wallet, use-restore-wallet, send-bitcoin-destination-screen, use-onchain-fee-tiers, use-recommended-fee-tiers) are one-off call sites that don't have an adapter equivalent. Happy to unify them in a follow-up PR if you want the boundary enforced uniformly, but treating them as an exception in this PR keeps the scope focused.

6. Duplicate PaymentType import source — Won't fix (not duplicates)

They model two different stages of a payment:

Source Purpose Values
@blinkbitcoin/blink-client Parsed destination intent Lightning, Intraledger, IntraledgerWithFlag, Onchain, Lnurl, NullInput, Unified, Unknown (8)
@app/types/transaction.types Normalized transaction history classification Lightning, Onchain, Spark, Conversion (4)

Merging would drop information (parsing needs Intraledger, NullInput, Unified, Unknown; history needs Spark, Conversion). The aliasing pattern PaymentType as SelfCustodialPaymentType already disambiguates in the couple of files that reference both.


Code Smells (6)

1. Long function chains in wrapDestinationForSCAddressed

Replaced the three sibling if-returns with a switch statement and three named, type-narrowed handlers (buildLightningDetail, buildSparkDetail, buildOnchainDetail) that accept a single BuildArgs<T, DestinationSubtype> context object.

Commit: refactor(self-custodial): replace wrapDestinationForSC if-chain with switch and typed handlers.

2. Excessive test mocking in wallet-provider.spec.tsxAddressed

Extracted setupConnectedWallet() and getWalletSnapshotMocks() into wallet-provider.fixtures.ts. Ten tests that previously repeated 15 lines of init/snapshot/listener boilerplate now share a 5-line setup. Spec dropped from 765 → 685 lines; new tests are one-liners.

Commit: test(self-custodial): extract shared fixture for SelfCustodialWalletProvider setup.

3. String-based error classification — Addressed

use-onchain-fee-tiers.ts now prefers the typed path:

if (SdkError.instanceOf(err)) {
  return SDK_ERROR_TAG_MAP[err.tag] ?? SdkFeeError.Generic
}
// string fallback kept for non-typed rejections

A tag-to-code map (SDK_ERROR_TAG_MAP) handles the typed case; the original string matcher is kept as a fallback for non-typed rejections (and existing tests that throw plain Error). Two new tests cover the typed branch.

Commit: refactor(send): classify SDK fee errors by SdkError tag instead of message string.

4. Inconsistent "SC" abbreviation — Addressed

Converged on the full SelfCustodial prefix across the codebase:

  • ScPaymentOfflineNoticeSelfCustodialPaymentOfflineNotice
  • SCPaymentRequestStateSelfCustodialPaymentRequestState
  • createSCLightningPaymentDetails / Onchain / SparkcreateSelfCustodialLightningPaymentDetails / …
  • PaymentType as SCPaymentTypePaymentType as SelfCustodialPaymentType
  • usePaymentRequest as useSCPaymentRequestusePaymentRequest as useSelfCustodialPaymentRequest
  • Directory app/components/sc-payment-offline-notice/app/components/self-custodial-payment-offline-notice/
  • testIDs sc-payment-offline-notice / sc-payment-offline-retryself-custodial-payment-offline-notice / self-custodial-payment-offline-retry

Commit: refactor(self-custodial): use full SelfCustodial prefix consistently across components, types and testIDs.

5. Utility in hooks directory — Addressed

format-fee-tier-options.ts and its spec moved from app/screens/send-bitcoin-screen/hooks/ to app/utils/. Both importers updated.

Commit: refactor: move format-fee-tier-options from send-bitcoin hooks to shared utils.

6. HOC pattern — Addressed

Rewritten as a wrapper component. app/self-custodial/hocs/ is gone. New <OfflineGate> component lives at app/self-custodial/components/offline-gate.tsx. The 9 payment routes in root-navigator.tsx are now wrapped once at module level via a local helper that composes <OfflineGate> with each screen — no WeakMap, no cache, stable references. Dedicated spec added (offline-gate.spec.tsx, 4 tests).

Commit: refactor(self-custodial): replace withOfflineGate HOC with OfflineGate wrapper component.


Test Gaps (3)

1. Edge cases — Deferred

Network switching mid-transaction, concurrent payments and app-backgrounding during execution are valuable but need concrete failure cases to anchor them. We'll open follow-up specs as we reproduce each scenario; no additions in this PR to avoid shallow, coincidence-dependent tests.

2. Missing scenarios — Deferred

Accessibility, 0-fee tiers, malformed SDK responses, duplicate transaction IDs — same reasoning as #1. Concrete cases first, tests after.

3. Uncovered files — Addressed

  • deposit-error-message.tsx — new spec, 7 tests covering every error-reason branch and fallbacks.
  • use-onchain-resolver.ts — new spec, 6 tests covering SC vs custodial routing, loading, and memo/amount forwarding.

Commit: test: add specs for deposit-error-message and use-onchain-resolver.


Summary

Test suite: 299 → 301 suites, 3,141 → 3,153 tests, all passing. TSC clean, ESLint clean.

Copy link
Copy Markdown
Contributor

@grimen grimen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — bugs + test coverage

Important

This PR (and its stack) needs a major refactor pass. Findings include:

  • SOLID violationsusePayments returns a mode-dependent shape (optional fields per mode); adapter contracts leak isSelfCustodial: boolean in their return types; the bridge layer mixes pass-through wrappers with real abstraction.
  • High cyclomatic complexity in the screen layer — 60+ isSelfCustodial branches across 11 files; home-screen.tsx alone has 15 mode branches and a 17-hook prelude; send-bitcoin-destination-screen.tsx has a 30-hook prelude.
  • Incomplete adapter pattern — mode leaks into call sites instead of being hidden behind a uniform Port. SC code fakes Apollo error shapes (__typename: "GraphQLApplicationError" in send-helpers.ts:91, 116); shared types carry Apollo- and Breez-flavoured fields.
  • ~50 naming smells — mode prefixes baked into every export (selfCustodialCreateWallet, mapSelfCustodialTransaction, etc.), container nouns (Info, Result, Fn, helpers, utils), lying constants (LIGHTNING_FEE_SATS = 0 is the actual fee bug too), and inconsistent vocabulary (memo/description/message; fee/feeAmount/feeSats/feeRateSatPerVb).

Why this is not in scope for this PR: with 6 stacked PRs above this one (#3761#3769), doing the refactor here would force a multi-day restack across all of them, break the stack's momentum, and create combinatorial conflict pain — particularly the rename passes. Refactoring against main after the stack settles is materially cheaper for everyone.

Decision: this review covers correctness only. The structural / SOLID / complexity / naming review will be shared as a separate cleanup ticket once both:

  1. The full stack is approved for functionality, and
  2. All Critical bugs and missing tests below are resolved.

Cleanup will then land as a sequence of small, focused PRs against main — no restack pain, each independently mergeable. Production rollout (#3768) should be gated on the cleanup ticket reaching at least the adapter-pattern unification milestone, otherwise the structural debt ships and gets normalised.


Real strengths first: bridge-layer DI on the Breez SDK, the selfCustodialCreateWallet rollback (bridge/lifecycle.ts:91-109), wallet-provider.spec.tsx driving real timers + AppState, is-online.spec.ts walking the full enum, the use-payment-request.spec.ts "full cycle" baseline-ref test, validate-network with proper Sentry instrumentation. The shape of the integration is right.

Severity:

  • Critical — fund-loss, mis-attribution, or silent failure that misleads the user. Must fix before merge.
  • Important — correctness/reliability that doesn't lose funds but degrades UX or hides incidents.
  • Test — coverage gap or test that passes while a confirmed bug is alive.

Critical

1. Hardcoded LIGHTNING_FEE_SATS = 0 in SC Lightning send

app/self-custodial/payment-details/send-helpers.ts:16, 43-57

createGetFee calls prepareSend then discards the quoted fee, returning the constant 0 regardless. Compare app/self-custodial/adapters/payment-adapter.ts:74-79, which correctly uses extractLightningFee(prepared). User authorises a Lightning send shown as free; SDK takes the actual fee.

Fix: delete the constant; read fee from prepared via extractLightningFee.

2. Refund accepts arbitrary fee rate, including 0

app/screens/unclaimed-deposits/hooks/use-deposit-actions.ts:119-150 + use-recommended-fee-tiers.ts:14-18, 29-47

useRecommendedFeeTiers defaults to feeSats: 0 and silently keeps zeros on fetch failure (catch { /* keep defaults */ }). getFeeRateSatPerVb returns those zeros. handleRefund passes them straight to claimDeposit.refundDeposit({ feeRateSatPerVb }) with no validation.

Fix: validate feeRateSatPerVb > 0 before submission; surface fetch failure as an error state rather than silently keeping zeros.

3. extractOnchainFees returning null silently shows 0-sat fees

app/screens/send-bitcoin-screen/hooks/use-onchain-fee-tiers.ts:79-82 + app/self-custodial/payment-details/send-helpers.ts:69

When prepare result isn't a BitcoinAddress tag, extractOnchainFees returns null and the hook returns silently with tiers = DEFAULT_TIERS (zeros) and error = null.

Fix: treat null as an error state; never default fee tiers to zero in user-visible code.

4. isOnline() collapses every SDK error into "offline"

app/self-custodial/providers/is-online.ts:5-12 + caller at use-sdk-lifecycle.ts:60-66 + app/self-custodial/components/offline-gate.tsx

Boolean conflates three states: confirmed online, confirmed offline, and "we couldn't tell because the SDK threw." Auth errors, malformed responses, mnemonic decryption errors all become Offline, gated behind a Retry button that re-runs the broken code. Also: Degraded is treated as online.

Fix: return a three-state result ("online" | "offline" | "unknown"); let the lifecycle hook decide what unknown means; surface Degraded distinctly so the UI can warn.

5. usePayments defaults to AccountType.Custodial during SC startup

app/hooks/use-payments.ts:46-72

accountType = activeAccount?.type ?? AccountType.Custodial. During SC startup activeAccount can be momentarily undefined; the hook hands back the custodial adapter shape (no sendPayment / getFee / receiveLightning / receiveOnchain). Consumers either no-op or take the wrong code path.

Fix: return undefined adapters in the loading window; don't default to a mode that doesn't match.

6. useOnchainFeeAlert always fires custodial GraphQL on SC

app/screens/send-bitcoin-screen/send-bitcoin-details-screen.tsx:220-224, 731-793

useOnChainTxFeeLazyQuery runs whenever isOnchainPayment, including when isSelfCustodial && paymentType === Onchain. Fires an authenticated mutation that doesn't apply to SC; the high-fee modal logic is meaningless because SC fees come from useOnchainFeeTiers.

Fix: gate on !isSelfCustodial.

7. refreshWallets lifecycle race + recursive finally re-call

app/self-custodial/providers/use-sdk-lifecycle.ts:51-88, 114, 122

  • TOCTOU between pendingRefreshRef check (line 53) and clear (line 84).
  • Recursive refreshWallets() invocation in finally (line 85) is unawaited and uncaught.
  • Three independent triggers (refresh chain, AppState, 10s setInterval) all fire-and-forget.
  • Status-fold logic at line 79 can transition Loading → Ready on a refresh failure → an empty wallet looks like a successful empty wallet.
  • First-sync .catch(() => {}) (line 114) and getUserSettings(...).catch(() => {}) (line 122) silently latch failures with no Sentry record.

Fix: model the lifecycle as a state machine (reducer or xstate); replace empty .catch(() => {}) with structured failure handling that updates status and records to crashlytics.

8. Mapper silent fallbacks mis-attribute transactions

app/self-custodial/mappers/transaction-mapper.ts:22-37, 45-48

  • mapPaymentMethod: any unknown SDK enum value falls through to PaymentType.Lightning.
  • mapDirection: any non-Send value becomes Receive.
  • mapCurrency: only checks details.tag === Token; future detail types silently default to WalletCurrency.Btc.

A future SDK enum or unanticipated direction silently mis-classifies tx history. No log, no Sentry.

Fix: make exhaustive (TypeScript never check on unhandled tag); on unknown input throw or record an error.

9. BIP-21 URI uses default JS number stringification

app/utils/bitcoin-uri.ts:27

${satsToBtc(amountSats)} produces "1e-8" for 1 sat — many BIP-21 wallet parsers reject exponential. Memo encoded via URLSearchParams, which percent-encodes spaces as + (BIP-21 expects %20).

Fix: format as fixed-decimal (.toFixed(8) then strip trailing zeros) and use encodeURIComponent for the memo.

10. Pagination hasMore over-counts after isKnownPayment filter

app/self-custodial/providers/wallet-snapshot.ts:50, 97, 120

hasMore = transactions.length >= TRANSACTIONS_PER_PAGE is computed after isKnownPayment filtering. If a page returns 20 raw items and 5 are filtered out, transactions.length === 15 and hasMore is wrongly false. Pagination silently truncates for token-heavy histories.

appendTransactions (lines 101-111) also doesn't dedupe by tx id; concurrent refresh between loadMore calls can duplicate entries.

Fix: track raw response length for hasMore; dedupe by id in appendTransactions (or move to cursor-based pagination).

11. Fund-recovery surface has zero tests

app/screens/unclaimed-deposits/unclaimed-deposits-screen.tsx (259 LOC), use-deposit-actions.ts (160 LOC), use-recommended-fee-tiers.ts (64 LOC), unclaimed-deposit-banner.tsx, utils.ts — all have no test files. This is the riskiest user flow added; broken refund logic costs users sats.

Fix: test files for each, prioritising use-deposit-actions (claim/refund + error classification + busy state) and use-recommended-fee-tiers (the source of Critical #2).


Important

# Item Location
I1 String-matched error classification (locale + version fragile) bridge/deposits.ts:49, use-deposit-actions.ts:55-57, use-onchain-fee-tiers.ts:45-49
I2 OfflineGate only blocks on OfflineError/Unavailable fall through with divergent in-screen guards app/self-custodial/components/offline-gate.tsx; consumers in receive-screen.tsx:42-54, send-bitcoin-destination-screen.tsx:730-748
I3 Conversion screens are OfflineGated but conversion is custodial-only; SC users can press the Transfer button on home-screen root-navigator.tsx:177-178 + home-screen.tsx:449-463
I4 useDisplayCurrency lost skip: !isAuthed — fires for unauthed/SC sessions app/hooks/use-display-currency.ts:119
I5 Race in unclaimed-deposit-banner between useFocusEffect and useEffect; cancelled flag is per-call and doesn't coordinate app/components/unclaimed-deposit-banner/unclaimed-deposit-banner.tsx:29-46
I6 ReceiveScreen invokes both usePaymentRequest() and useSelfCustodialPaymentRequest() on every render; cast strips type-safety app/screens/receive-bitcoin-screen/receive-screen.tsx:46-65
I7 use-payment-request.ts fire-and-forget adapter().then(...) with no .catch; onchain address never refreshes after first set even if SDK reconnects app/self-custodial/hooks/use-payment-request.ts:107-113
I8 Send mutations log raw SDK English text into the GraphQL error path; no crashlytics().recordError — failed sends never reach Sentry send-helpers.ts:80-98, 100-121, bridge/{convert,receive}.ts
I9 extractOnchainFees always picks fees.fast, ignoring feeTier declared on SendPaymentParams app/self-custodial/adapters/payment-adapter.ts:58-72
I10 parseDepositId has no validation; malformed id → vout: NaN reaches SDK app/self-custodial/adapters/deposit-adapter.ts:55-61
I11 getClaimFee always returns feeAmount: 0 (placeholder pretending to be feature-complete) app/self-custodial/adapters/deposit-adapter.ts:91-94
I12 useOnchainFeeTiers doesn't abort in-flight prepareSend on dep change — race where stale tiers overwrite fresh app/screens/send-bitcoin-screen/hooks/use-onchain-fee-tiers.ts
I13 transaction-history-screen writes every SC fragment to Apollo cache on every change of selfCustodialFragments; depends on LL so locale changes trigger render storms app/screens/transaction-history/transaction-history-screen.tsx:187-196
I14 selfCustodialRestoreWallet has no equivalent rollback or BIP39 validation (compare selfCustodialCreateWallet) app/self-custodial/bridge/lifecycle.ts:112-117
I15 parseSparkAddress swallows all errors as null — caller cannot distinguish "not a Spark address" from "SDK threw" app/self-custodial/bridge/parse.ts:36-38

Test coverage

Estimated behavioural coverage of the new SC layer: ~62%, concentrated on bridge / mappers / wallet-provider; sparse on the screen layer and adapters.

Tier 1 — production code with no test file

Verified against __tests__/:

File LOC Notes
app/screens/unclaimed-deposits/hooks/use-deposit-actions.ts 160 All claim/refund error classification + dust heuristic + busy state
app/screens/unclaimed-deposits/unclaimed-deposits-screen.tsx 259 Refund flow: address input, fee tier selection, mempool linking, reset
app/screens/unclaimed-deposits/hooks/use-recommended-fee-tiers.ts 64 Source of Critical #2
app/screens/unclaimed-deposits/utils.ts 12 Mempool URL builder + placeholder
app/components/unclaimed-deposit-banner/unclaimed-deposit-banner.tsx 94 Race in I5; only entry point for unclaimed deposits
app/screens/send-bitcoin-screen/fee-tier-selector.tsx 160 UI users tap when choosing a fee tier
app/screens/send-bitcoin-screen/hooks/use-onchain-fee-tier-options.ts 120 Owns setFeeTier, rebuild-detail-on-tier-change logic
app/self-custodial/bridge/convert.ts 43 No direct test; only transitively via payment-adapter.spec.ts happy path
app/self-custodial/bridge/lifecycle.ts 117 createWallet rollback path is security-critical
app/navigation/navigation-container-wrapper.tsx (modified) canHandlePayments change for SC deep links
app/screens/receive-bitcoin-screen/receive-screen.tsx (modified) Dual-hook invocation + cast logic in I6

Tier 2 — tests that pass while a Critical bug is alive

These create false confidence. Each should be made to fail on the current code first, then fixed alongside the production fix.

Test file What it should catch but doesn't Bug
__tests__/utils/bitcoin-uri.spec.ts Smallest tested amount is 1000 sats. Never tests 1 sat (1e-8) where JS flips to exponential. Memo encoding (+ vs %20) unverified. #9
__tests__/self-custodial/payment-details/lightning.spec.ts Lines 9-16 mock send-helpers — including createGetFee. The LIGHTNING_FEE_SATS = 0 bug is invisible. Mocks return bare jest.fn() with no implementation. #1
__tests__/self-custodial/adapters/payment-adapter.spec.ts createGetFee onchain branch (feeTier, confirmationEtaMinutes, totalDebited) completely unasserted. Hardcoded feeTier: Fast survives. #6, I9
__tests__/self-custodial/payment-details/send-helpers.spec.ts Verify whether returned fee value matches prepareSend quote, or only call-count. #1
__tests__/self-custodial/mappers/transaction-mapper.spec.ts Silent fallback branches (unknown SDK enum → Lightning/Receive/Btc) not tested with unknown enum values. #8
__tests__/self-custodial/mappers/to-transaction-fragment.spec.ts Mixed-unit-fee bug (fee path always uses currency: WalletCurrency.Btc) not exercised on a token tx. (mappers)
__tests__/self-custodial/providers/is-online.spec.ts Walks every ServiceStatus enum but never tests the caller's mis-classification of "SDK threw" as "offline." #4
__tests__/self-custodial/providers/wallet-snapshot.spec.ts Verify: does it test post-filter hasMore over-counting and appendTransactions dedupe? If not, #10 uncovered. #10
__tests__/self-custodial/providers/wallet-provider.spec.tsx Recursive refreshWallets() in finally not directly tested. The suite acknowledges (lines 463-497) that Ready→Error via refresh is unreachable; the OFFLINE_EXEMPT_STATUSES Error preservation is effectively untested. #7
__tests__/self-custodial/hooks/use-payment-request.spec.ts Onchain receive adapter().then(...) with no .catch — rejected adapter throws unhandled rejection; not asserted. I7
__tests__/self-custodial/providers/sdk-events.spec.ts expect(REFRESH_EVENTS.size).toBeGreaterThanOrEqual(5) is tautological. Should assert specific tag membership. (test quality)

Tier 3 — Critical bugs with no test coverage anywhere

For each, no existing test exercises the behaviour. Add a regression test alongside the fix.

Critical Recommended test
#1 Direct createGetFee test (un-mocked) asserting fee value comes from extractLightningFee(prepared)
#2 useRecommendedFeeTiers rejection → tiers stay zero AND surface error / refuse use
#3 useOnchainFeeTiers with non-onchain prepared → tiers untouched, error explicit
#4 Caller-level test: SDK throws non-status error → caller distinguishes unknown from offline
#5 usePayments with activeAccount: undefined and SC mode → no SC operation routed to custodial
#6 send-bitcoin-details-screen test: SC user + onchain → custodial GraphQL not called
#7 wallet-provider test: pendingRefreshRef = true + recursive throw → outer error capture
#8 Mapper tests with unknown SDK enum → assertion the fallback is explicit (logged/classified/thrown)
#10 Full page that filters down → hasMore: true; back-to-back loadMore with overlapping ids → unique entries
Refund (#2) use-deposit-actions test: feeRateSatPerVb: 0 → throws or refuses, NOT calls SDK

Suggested order of operations

  1. Identify originating PR per Critical bug and route fixes via gt absorb into the right PR in the stack (several Critical items likely originated in #3746 "shared wallet abstractions" — specifically #5, #7, possibly #4 and #10).
  2. In #3758: land the Critical bugs that originated here (#1, #2, #3, #6, #8, #9, #11) plus their regression tests.
  3. Write the missing test files (Tier 1) and the regression tests (Tier 3) alongside the production fixes.
  4. Un-mock send-helpers in lightning.spec.ts and replace tautological cardinality assertions in sdk-events.spec.ts (Tier 2 quality fixes).
  5. Important items I1–I15 — most are small; some can fold into the same commits as the Critical fixes.

Structural and naming feedback (adapter-shape unification, mode-leakage cleanup, ~50 rename suggestions) will follow as a separate note, scoped as post-stack work against main so it doesn't restack-bomb your downstream PRs (#3761#3769).

@esaugomez31 esaugomez31 force-pushed the feat--spark-backup-and-recovery branch from 5affa3c to 9205cc7 Compare May 6, 2026 02:05
@esaugomez31 esaugomez31 force-pushed the feat--spark-payments-and-transaction-history branch from d129a05 to 5e2acfa Compare May 6, 2026 02:05
esaugomez31 added 23 commits May 6, 2026 11:56
…stodial shape and report onchain adapter failures
@esaugomez31
Copy link
Copy Markdown
Collaborator Author

esaugomez31 commented May 7, 2026

Hi @grimen - the feedback is applied. Summary below per item with the commit hash where it landed; details follow.


Critical

C1 - LIGHTNING_FEE_SATS = 0 hardcoded

Skipped (resolved upstream). Replaced with extractLightningFee(prepared) ?? 0 in upstream commit dcbdc65a5 on feat--spark-stable-receive-and-send-fixes. No change needed on this branch.

C2 - Refund fee gating with bogus 0 sat/vB

Applied - c550ff0be. useRecommendedFeeTiers now returns { tiers, error }. useDepositActions.handleRefund rejects when feeRateSatPerVb <= 0. UI disables "Refund now" when fee is unavailable. Added feeRateUnavailable i18n key across 29 locales.

C3 - extractOnchainFees returning null silently

Applied - 9c38f764b. useOnchainFeeTiers sets SdkFeeError.Generic when fees are null. createGetFeeOnchain returns { amount: undefined } instead of using a stale 0.

C4 - isOnline collapses unknown into offline

Applied - 25d152588. New getOnlineState(): "online" | "offline" | "unknown" (OnlineState const). use-sdk-lifecycle preserves status during unknown. isOnline kept as a boolean wrapper for backward compat.

C5 - usePayments defaulting to Custodial during loading

Applied - d621add4a. Removed the ?? AccountType.Custodial fallback. accountType is now optional on PaymentsResult, with three explicit branches (self-custodial ready / Custodial / loading).

C6 - useOnchainFeeAlert firing for self-custodial users

Applied - 4b5d262bc. Hook extracted to app/screens/send-bitcoin-screen/hooks/use-onchain-fee-alert.ts. Now takes an options object with isSelfCustodial; gate is !isSelfCustodial && walletId && paymentDetail && paymentType === "onchain".

C7 - refreshWallets recursion in finally

Applied - bd9a7ca0b. Replaced recursion with an internal do/while loop (avoids TOCTOU and unhandled rejections). Switched failure logging from crashlytics().log to recordError. Removed dead .catch(() => {}). On init failure the status now folds from Loading to Error (previously it landed on Ready).

C8 - Mapper exhaustiveness (payment method / direction / currency)

Applied - a06f7a89c. mapPaymentMethod, mapDirection, mapCurrency are now exhaustive switch statements with satisfies never guards. Helper reportUnhandledEnum logs to crashlytics with a documented fallback.

C9 - BIP-21 URI generation

Applied - 7c050798e. formatBip21Amount uses .toFixed(8) with trailing-zero strip (BIP-21 compliant). Memo encoded with encodeURIComponent so spaces become %20, not +.

C10 - hasMore heuristic + duplicate transactions

Applied (partial; balance from upstream). Our commit 70e765a5a adds id-based dedupe via new Map(entries) in appendTransactions. Upstream f53331622 already exposes rawTransactionCount so hasMore is now derived from the raw response length rather than a heuristic.

C11 - Unclaimed-deposits test gaps

Applied - 9f86691f7. New unclaimed-deposit-banner.spec.tsx and utils.spec.ts. Extended unclaimed-deposits-screen.spec.tsx to cover empty state, claim, mempool, immature, processing, busy, cancel, and refund modes.


Important

I1 - String-matched error classification (dust limit / fee tier / deposits)

Applied - 0911609d2 + 51923b9fd. Removed MESSAGE_TAG_MATCHERS from use-onchain-fee-tiers, the .includes("dust limit") heuristic from bridge/deposits.ts, and the raw.includes("dust") branch in use-deposit-actions. Only typed SdkError.instanceOf(err) instances are classified; everything else falls through as generic. Trade-off acknowledged: dust-limit messages will surface raw SDK English copy until the SDK exposes a typed tag - we preferred that over fragile string matching.

I2 - OfflineGate only blocking on Offline

Applied - 1427babcc. SC_BLOCKED_STATUSES now includes Offline, Error, and Unavailable, so self-custodial users see the offline notice on Receive/Send whenever the SDK is unhealthy.

I3 - Transfer button hidden for self-custodial

Skipped (resolved upstream). The upper branch integrates a self-custodial Transfer flow via Spark stable balance (BTC and USDB). The hide we had on this branch was reverted; users see Transfer everywhere again. No change here.

I4 - useDisplayCurrency running while unauthed

Reverted. Tried gating useCurrencyListQuery with { skip: !isAuthed }, but it broke self-custodial: the currencyList query is public and we rely on it to populate the language list in self-custodial settings. Without it, self-custodial users got an empty currency dictionary, "Currency issue. Refresh needed" on every wallet card, and a broken language picker. Reverted to keep the list available; if there's a real concern about stale auth headers on a public query, the right place to fix it is the Apollo link layer, not the hook.

I5 - unclaimed-deposit-banner concurrent-fetch race

Applied - b7ea9e061. Replaced the per-call cancelled flag with a fetchGenerationRef token; only the latest fetch is allowed to commit state.

I6 - ReceiveScreen cast hiding type mismatch

Applied - 426068b4f. Eliminated structurally rather than casting. SelfCustodialPaymentRequestState was aligned with the custodial shape: optional state: PaymentRequestStateType, optional expirationTime, expiresInSeconds: number | null, convertMoneyAmount: NonNullable<...>, nullable pr, optional info.data, optional username on InvoiceData. Self-custodial hook updates: useState<PaymentRequestStateType>, early return when convertMoneyAmount is missing, expiresInSeconds: null (was undefined). Five consumer hooks now use RequestState = SelfCustodialPaymentRequestState. useLnurlWithdraw signature narrowed to LnurlWithdrawablePr. useDisplayPaymentRequest guards info.data.username before calling getLightningAddress. The cast on receive-screen.tsx is gone.

I7 - use-payment-request fire-and-forget onchain fetch

Applied - 426068b4f. The onchain adapter useEffect now uses a cancelled flag, a .catch that calls crashlytics().recordError("Self-custodial receive onchain adapter failed: ..."), and depends on [sdk] so it re-fetches on reconnect.

I8 - Send mutations swallow errors with console.error

Applied - 2d9639574. Introduced reportSendFailure(scope, err) that returns a GraphQLApplicationError-shaped error and records to crashlytics with a scoped prefix. createSendMutation and createSendMutationOnchain use it; messages are prefixed Self-custodial Lightning send failed: / Self-custodial onchain send failed: for non-Error throws.

I9 - extractOnchainFees ignored the requested feeTier

Applied - cfa8e0e68. createGetFee destructures feeTier from SendPaymentParams, reads onchainFees[tier], and returns the matching tier and confirmationEtaMinutes from FEE_TIER_ETA_MINUTES[tier]. Removed two identity-map lookup tables (ONCHAIN_TIER_TO_FEE_KEY in payment-adapter, TIER_TO_FEE_KEY in send-helpers) and dropped the app/screens/... import that the adapter had for the ETA constant - FEE_TIER_ETA_MINUTES now lives in app/types/payment.types.ts.

I10 - parseDepositId accepting malformed ids

Applied - f30844b8e. parseDepositId validates lastColon > 0, non-empty txid, decimal vout (\d+), Number.isSafeInteger, vout >= 0, and returns { txid, vout } | null. Now exported. getClaimFee, claimDeposit, and refundDeposit short-circuit when parsing fails, so the SDK is never called with NaN.

I11 - getClaimFee returning a fake 0-sat placeholder

Applied - f30844b8e. getClaimFee now returns null. Removed the wasted listDeposits call (both branches collapsed to null/0). UI tolerates the null. We'll re-introduce a real fee read once the SDK exposes one.

I12 - useOnchainFeeTiers racing concurrent fetches

Applied - 51923b9fd. Added requestTokenRef; a token is captured before each prepareSend and the resolution is discarded if the token has changed by the time the promise settles (after the await and inside the catch).

I13 - Transaction history Apollo cache thrash

Skipped (resolved upstream). The polish branch already uses InteractionManager.runAfterInteractions and cache.batch for transaction history updates. No change needed here.

I14 - selfCustodialRestoreWallet storing invalid mnemonics

Applied - b00a66e5c. Now runs bip39.validateMnemonic (after trim + whitespace collapse) before storing. SDK init runs as a smoke test post-store; on failure we delete the stored mnemonic and call crashlytics().recordError, leaving the user no worse than before.

I15 - parseSparkAddress collapsing parse errors and not-Spark

Applied - da956c293. Added parseSparkAddressDetailed returning a discriminated union (Match | NotSparkAddress | ParseError) with ParseSparkAddressOutcome const + type. parseSparkAddress is preserved as a backward-compat wrapper.


Test coverage

Tier 1 - production code without a test file

Applied - 0bf7644e5 + 9f86691f7. New specs cover use-deposit-actions, use-recommended-fee-tiers, unclaimed-deposits-screen, unclaimed-deposit-banner, utils.ts (mempool URL), fee-tier-selector, use-onchain-fee-tier-options, bridge/convert.ts, bridge/lifecycle.ts (rollback paths), navigation-container-wrapper, and receive-screen.

Tier 2 - tests passing while a confirmed bug is alive

Applied - 4ca6e7ee3. Tightened the existing specs so they would fail on the original buggy code:

  • bitcoin-uri.spec.ts - exercises 1-sat amounts and verifies %20-encoded memo (catches Critical non scannable QR code #9).
  • lightning.spec.ts - replaced bare jest.fn() stubs with spies that delegate to the real send-helpers via jest.requireActual; the actual fee value computed by prepareSend is now asserted (catches Critical testPR CI #1).
  • payment-adapter.spec.ts - asserts the onchain branch (feeTier, confirmationEtaMinutes, totalDebited) instead of relying on Fast slipping through (catches Critical add a "do you like the app? you can review" #6, I9).
  • transaction-mapper.spec.ts - exercises unknown SDK enum values and asserts the explicit fallback + crashlytics record (catches Critical add bitcoin: and lightning: as compatible link #8).
  • to-transaction-fragment.spec.ts - adds the mixed-unit guard test for token tx (BTC fee + USD settlement).
  • is-online.spec.ts - caller-level test that "SDK threw" stays unknown and doesn't collapse to Offline (catches Critical MISSING_INSTANCEID_SERVICE issue #4).
  • wallet-snapshot.spec.ts - covers post-filter hasMore and appendTransactions dedupe (catches Critical make it possible to try to copy graphql error from RN for easier support #10).
  • wallet-provider.spec.tsx - adds the pendingRefreshRef recursive-throw scenario and asserts OFFLINE_EXEMPT_STATUSES preservation (catches Critical Qrcode scanning of username doesn’t work for coldstart #7).
  • use-payment-request.spec.ts - asserts unhandled-rejection capture on adapter failure (catches I7).
  • sdk-events.spec.ts - replaces the tautological cardinality check with explicit tag-membership assertions.

Tier 3: regression tests alongside each Critical fix

Each Critical fix above lands with its regression test in the same commit (called out per item):


Refactor (not flagged, surfaced during the work)

FeeTier map deduplication

  • Added FEE_TIER_ETA_MINUTES to app/types/payment.types.ts (neutral location).
  • Removed ONCHAIN_TIER_TO_FEE_KEY and TIER_TO_FEE_KEY - they were identity maps; replaced with direct lookups.
  • app/self-custodial/adapters/payment-adapter.ts no longer imports from app/screens/... (fixes layering smell).
  • use-onchain-fee-tiers re-exports ETA_MINUTES = FEE_TIER_ETA_MINUTES to keep use-recommended-fee-tiers unchanged.

@esaugomez31 esaugomez31 requested a review from grimen May 7, 2026 16:01
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.

2 participants