Skip to content

feat(stellar): validated network profile, pre-sign review, and idempotent submission - #1003

Merged
llinsss merged 3 commits into
DogStark:mainfrom
JoyAdah:feat/stellar-network-review-idempotency
Aug 28, 2026
Merged

feat(stellar): validated network profile, pre-sign review, and idempotent submission#1003
llinsss merged 3 commits into
DogStark:mainfrom
JoyAdah:feat/stellar-network-review-idempotency

Conversation

@JoyAdah

@JoyAdah JoyAdah commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Description

Four assigned Stellar payment issues. They share one spine — the network profile from #943 is what #945 checks against and what #946/#947 submit to — so they are proposed together.

Reviewer note: the contributor notes ask for narrowly scoped PRs. #943 (config), #945 (review UI) and #946+#947 (submission) split cleanly into three if you'd prefer; say the word and I'll break it up.

Issue What landed
#943 One immutable, validated network profile
#945 Decode-and-review the real envelope before signing, with stale-quote expiry
#946 Hash-keyed idempotent submission + Horizon reconciliation
#947 Durable pending records, reconciled on launch

Linked Issue

Closes #943
Closes #945
Closes #946
Closes #947


⚠️ Two pre-existing blockers had to be cleared first

Neither is cosmetic — without them nothing in this repo can be verified at all.

1. No test in the repo currently runs. jest.setup.js loads src/__mocks__/server.tsmsw/node, whose dependency graph ships ESM. transformIgnorePatterns allowlisted only otplib/expo-*, and the transform pattern ^.+\.[jt]sx?$ doesn't match .mjs — so Babel never touched it and every suite threw Cannot use import statement outside a module before executing a single test.

$ npx jest --ci          # on untouched main
Test Suites: 181 failed, 181 total
Tests:       0 total

Fixed by widening transformIgnorePatterns and extending the transform to .mjs/.cjs.

2. tsc was broken project-wide. src/utils/sanitize.ts had a literal */ inside a JSDoc code span listing SQL metacharacters, which closed the block comment early — everything after it parsed as code. Fixed by rewording the doc line; no logic touched.


#943 — Validate network and asset configuration as one unit

The bug

Network selection was made independently in two places:

Module How it chose
services/blockchainService.ts const STELLAR_NETWORK: string = 'TESTNET' — a hard-coded literal
services/stellarPathPaymentService.ts config.env === 'production' ? Networks.PUBLIC : Networks.TESTNET

With APP_ENV=production those disagree: signTransactionXdr signed with the public passphrase while every Horizon call went to testnet. A passphrase is part of what a Stellar signature commits to, so this is the exact "mixing testnet/mainnet Horizon and passphrase" failure the issue describes. blockchainService also computed a passphrase into const _networkPassphrase and threw it away.

The fix

src/config/stellarNetwork.ts resolves network, Horizon URL and passphrase together, validates them as a unit, and freezes the result. Both services now read it; there are no remaining ad-hoc derivations.

Rejected combinations: public network outside production (without explicit opt-in), testnet in production, a Horizon host belonging to the other network, non-https Horizon, malformed URL, and a passphrase that doesn't match. Every problem is reported at once rather than one per run, and resolution throws rather than defaulting — a wrong-network default is the irreversible mistake here.

#945 — Simulation and review before signing

The bug

The confirm screen rendered fields from the server-supplied PathPaymentQuote, then signed preparedPayment.transactionXdr. Two different objects, never cross-checked — a wrong or tampered XDR would be approved against a reassuring summary. And quote.expiresAt already existed but was never read, so a user could leave the screen open and sign at a long-stale rate.

The fix

src/services/transactionSimulation.ts decodes the envelope that will actually be signed and reports destination, asset, amount, fee, memo, network and per-operation effects. compareQuoteToSimulation flags disagreement between the quote and the envelope; a blocking discrepancy prevents signing. evaluateQuoteFreshness implements stale-quote expiry — an absent or unparseable expiry counts as expired, because refusing to sign is the safe direction.

PaymentScreen gains a live countdown, a "Refresh quote" action once lapsed, and re-evaluates the gate at the moment of signing rather than trusting the last render, since the ticker may not have fired since the quote lapsed.

Accessibility: the confirm button carries accessibilityRole, accessibilityLabel, accessibilityState (disabled/busy) and an accessibilityHint that explains why it's disabled. Discrepancies render as accessibilityRole="alert". The expiry countdown uses accessibilityLiveRegion="polite" only when actually expiring, to avoid a once-a-second announcement. The review block is a single accessible node with one composed sentence, so a screen-reader user gets the whole review rather than a stream of fragments.

#946 — Prevent duplicate submissions

Two distinct hazards, handled differently:

  1. Rapid taps — concurrent calls share one in-flight promise keyed by the envelope hash.
  2. Ambiguous timeouts — a signed envelope has a deterministic hash, so the registry asks Horizon whether that hash already landed rather than rebuilding with a fresh sequence number. Rebuilding is what pays twice.

A bug the test-first work caught

The acceptance criteria ask for behaviour to be characterised by test before implementation. That's what found this: my first implementation did

const existing = inFlight.get(hash);
if (existing) return existing;
const stored = await getTransactionByHash(hash);   // ← first await
// ... register in inFlight only afterwards

An async function runs synchronously only up to its first await, so three taps queued in the same tick all passed the inFlight check before any of them registered — the guard did nothing for exactly the case it existed for. collapses concurrent submissions of the same envelope into one network call failed, and registration now happens before the first await.

An unreachable Horizon leaves a record pending, never failed — an outage is not evidence a payment failed, and reporting failure would invite a duplicate rebuild.

sendPayment and storeDataOnStellar now route through submitStellarTransactionOnce. PaymentScreen also holds a synchronous useRef lock, because submitting is React state and a second tap in the same frame can beat the re-render.

#947 — Persist and reconcile

Records are persisted before the network call, so a termination mid-flight leaves a durable trace; App.tsx reconciles on launch. Reconciliation never throws — a failure must not block startup.

Only non-secret data is stored. A signed envelope contains the source public key and a signature: the same bytes the network sees. Secret keys stay in secure storage and never reach the registry. A test asserts no strkey-shaped secret ever appears in the persisted blob. Storage is capped at 50 records, unresolved first, so trimming can't discard live work.


Test Plan

71 new tests across 4 suites, all passing.

src/config/__tests__/stellarNetwork.test.ts            21 passed
src/services/__tests__/transactionSimulation.test.ts   24 passed
src/services/__tests__/stellarTransactionRegistry.test.ts  21 passed
src/services/__tests__/stellarStartup.test.ts           5 passed

Because the Jest fix changes what runs at all, the honest comparison is against main with only that fix applied, so the source changes are isolated:

untouched main main + Jest fix only this branch
Tests executed 0 2477 2548
Passing 0 2067 2138
Failing — (181 suites crashed) 406 406
Failing suites 181 88 88

The failing set is identical — all 406 are pre-existing and unrelated, merely revealed by the setup fix. This branch adds 4 suites and 71 tests and introduces no new failure.

  • Typecheck: 17 errors, unchanged from baseline, none in files this PR adds or rewrites (verified by diffing error lists against a clean worktree).
  • Lint: eslint clean on all changed files — 0 errors.
  • transactionSimulation.test.ts calls jest.unmock('@stellar/stellar-sdk') and builds real XDR envelopes; a mocked decoder would prove nothing about the review path.
  • Synthetic data only: every key is an SDK-generated throw-away keypair.

Checklist

  • Current behaviour characterised by a focused test before implementation — this is how the [Mobile] Prevent duplicate Stellar submissions across taps and retries #946 in-flight race and a memo rendered as hex were both found.
  • Success, offline, timeout/cancellation, malformed-input and retry paths covered (ambiguous-vs-definite failure classification, unreachable Horizon, corrupt stored data, undecodable XDR, expired/missing/unparseable quote expiry).
  • No secrets, wallet material or raw tokens added to logs, fixtures or storage — asserted by test.
  • Configuration, API contracts and contributor documentation updated — new docs/STELLAR_PAYMENTS.md; the sendPayment/storeDataOnStellar return-type change is documented there.
  • Unit/integration tests pass.

Not verified — needs a reviewer with devices

  • iOS/Android evidence. I have no simulator or device in this environment, so I cannot attach screenshots or confirm platform behaviour. PaymentScreen changes are visible, so this needs a pass on both platforms before merge.
  • Accessibility, RTL, reduced motion, dynamic type, viewport. The a11y props are in place and the review summary is composed for screen readers, but I could not verify with VoiceOver/TalkBack, in an RTL locale, or at large dynamic type. The countdown line is the most likely to need layout attention at large text sizes.

API change

sendPayment and storeDataOnStellar now resolve to SubmitResult ({ hash, status, deduplicated, ledger?, resultCode? }) rather than a raw Horizon SubmitTransactionResponse. Neither had callers outside blockchainService.ts. The raw submitStellarTransaction primitive is unchanged, and its existing circuit-breaker tests still pass untouched.

…tent submission

Closes DogStark#943
Closes DogStark#945
Closes DogStark#946
Closes DogStark#947

DogStark#943 — Validate Stellar network and asset configuration as one unit
- Adds src/config/stellarNetwork.ts: one immutable, validated profile pairing
  network, Horizon URL and passphrase, resolved together and frozen.
- Fixes a real hazard: blockchainService hard-coded STELLAR_NETWORK='TESTNET'
  while stellarPathPaymentService derived its signing passphrase from
  config.env. With APP_ENV=production those disagree, so transactions were
  signed for PUBLIC and submitted to testnet Horizon.
- Rejects public-outside-production, testnet-in-production, cross-network
  Horizon hosts, non-https Horizon and mismatched passphrases, reporting all
  problems at once. Resolution throws rather than defaulting.

DogStark#945 — Require transaction simulation and review before signing
- Adds src/services/transactionSimulation.ts, which decodes the envelope that
  will actually be signed rather than trusting the server-supplied quote, and
  reports destination, asset, amount, fee, memo, network and per-operation
  effects.
- compareQuoteToSimulation flags any field where the displayed quote and the
  real envelope disagree; a blocking discrepancy prevents signing.
- Implements stale-quote expiry from PathPaymentQuote.expiresAt, which already
  existed and was never read. An absent or unparseable expiry counts as expired.
- PaymentScreen re-evaluates the gate at the moment of signing, adds a live
  countdown, a refresh action, and accessibility roles/labels/state including a
  single spoken review summary.

DogStark#946 — Prevent duplicate Stellar submissions across taps and retries
- Adds src/services/stellarTransactionRegistry.ts keyed by the envelope's
  deterministic hash: concurrent taps share one request, a confirmed
  transaction is never re-sent, and an ambiguous timeout is reconciled against
  Horizon instead of rebuilt with a fresh sequence number.
- A test-first check caught a real bug in the first implementation: the
  in-flight guard was registered after an await, so every tap in the same tick
  slipped past it. Registration now happens before the first await.
- An unreachable Horizon leaves a record pending, never failed — an outage is
  not evidence a payment failed.
- sendPayment and storeDataOnStellar now route through
  submitStellarTransactionOnce. PaymentScreen adds a synchronous ref lock,
  since `submitting` state can lose a same-frame double tap.

DogStark#947 — Persist and reconcile pending Stellar transactions
- Records are persisted before the network call and reconciled on launch via
  App.tsx, so a termination mid-flight is recoverable.
- Only non-secret data is stored: a signed envelope holds the source public key
  and a signature, the same bytes the network sees. Secrets stay in secure
  storage. Storage is capped, unresolved records first.

Also fixes two pre-existing blockers, without which none of the above could be
verified:
- jest.config.js: MSW and its ESM dependency graph were never transformed, so
  jest.setup.js threw and every suite in the repo failed before running a single
  test. On main, `npx jest` runs 0 tests across 181 failing suites.
- src/utils/sanitize.ts: a literal */ inside a JSDoc code span closed the
  comment early, breaking `tsc` project-wide.

Verification: 71 new tests across 4 suites, all passing. Against main with only
the jest fix applied, the failure set is byte-identical (88 suites / 406 tests,
all pre-existing); passing tests go 2067 -> 2138. Typecheck errors unchanged at
17, none in files this PR adds or rewrites. eslint clean on all changed files.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@drips-wave

drips-wave Bot commented Aug 25, 2026

Copy link
Copy Markdown

@JoyAdah Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@llinsss
llinsss merged commit 3ac38e9 into DogStark:main Aug 28, 2026
0 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants