diff --git a/README.md b/README.md index 77aadfe..591baea 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,32 @@ pending button behavior, duplicate-submission prevention, Transfers page filter sync (search, status, and date-range presets such as last 7/30/90 days), and landscape tablet layout integration. +### Transfer recovery storage + +Before submitting, Send Money must read the session operation journal and save +the safe recovery reference successfully. Unavailable storage, malformed JSON +or a journal that is not an object map prevents that attempt from reaching the +transfer API; existing stored bytes are left untouched. The form reports that +the attempt was not submitted and permits a retry after storage recovers. + +Only the opaque fingerprint, idempotency key, status and transfer reference are +stored. The same in-memory intent is retained after a failed pre-submit save. +Status updates after submission and mount-time recovery remain best-effort, so +a late journal failure cannot hide an accepted transfer. The existing demo API +still requires its separate localStorage record to be saved before reporting +creation. Session recovery does not replace a production backend idempotency +store or provide cross-tab or live-provider guarantees. + +The following captures show the built local-demo app under a controlled journal +write failure (2026-10-03). The earlier version accepted the transfer without a +recovery reference; the repaired version explains that this attempt was not +submitted. Restoring storage permits one successful retry and receipt recovery. + +| Viewport width | Earlier accepted state | Repaired refusal | +| -------------- | ----------------------------------------------------------------------- | ------------------------------------------------------------------------ | +| 1280 | [Desktop before](docs/screenshots/transfer-recovery-parent-desktop.png) | [Desktop after](docs/screenshots/transfer-recovery-repaired-desktop.png) | +| 390 | [Phone before](docs/screenshots/transfer-recovery-parent-phone.png) | [Phone after](docs/screenshots/transfer-recovery-repaired-phone.png) | + ## Accessibility All interactive elements (buttons, links, inputs, selects, icon buttons, and diff --git a/docs/screenshots/transfer-recovery-parent-desktop.png b/docs/screenshots/transfer-recovery-parent-desktop.png new file mode 100644 index 0000000..c5d805c Binary files /dev/null and b/docs/screenshots/transfer-recovery-parent-desktop.png differ diff --git a/docs/screenshots/transfer-recovery-parent-phone.png b/docs/screenshots/transfer-recovery-parent-phone.png new file mode 100644 index 0000000..cdd49f9 Binary files /dev/null and b/docs/screenshots/transfer-recovery-parent-phone.png differ diff --git a/docs/screenshots/transfer-recovery-repaired-desktop.png b/docs/screenshots/transfer-recovery-repaired-desktop.png new file mode 100644 index 0000000..675e3ed Binary files /dev/null and b/docs/screenshots/transfer-recovery-repaired-desktop.png differ diff --git a/docs/screenshots/transfer-recovery-repaired-phone.png b/docs/screenshots/transfer-recovery-repaired-phone.png new file mode 100644 index 0000000..000d06c Binary files /dev/null and b/docs/screenshots/transfer-recovery-repaired-phone.png differ diff --git a/docs/validation/crypto-recovery.md b/docs/validation/crypto-recovery.md new file mode 100644 index 0000000..6b7ae35 --- /dev/null +++ b/docs/validation/crypto-recovery.md @@ -0,0 +1,140 @@ +# Transfer fingerprint fallback and legacy recovery + +## Failure and reproduction + +The Send Money page already hashes the canonical transfer payload before saving +its recovery reference. The correction here is not a raw-recipient storage fix. +When `crypto.subtle` was absent, `idempotencyKeyFor` instead used 32-bit FNV-1a. +These two different valid recipient addresses produced the same recovery +fingerprint, `idem_489b5d95`: + +- `recipient-103w456@example.com` +- `recipient-1p5osr6@example.com` + +For both, the other canonical fields are `USD`, `NGN`, `25`, `36642.38`, `0.25`, +and `1480.5` (source, destination, sent, received, fee, rate). The canonical +input is the JSON array of the recipient followed by those six strings. +The pair was found in a bounded local search and is fixed in the regression; +the regression does not run a brute-force search. + +Saving an unknown operation for one old fingerprint lets a lookup for the +other recipient select that same operation. The mock API's existing independent +full-payload comparison still rejects a conflicting replay. This demonstrates +incorrect recovery binding and a false conflict, not a wrong payment, live +provider exploit, or actual money movement. Changing SubtleCrypto availability +also formerly changed the fingerprint of identical details. + +## Repair + +The existing SubtleCrypto SHA-256 path is unchanged. Without SubtleCrypto, the +utility lazily loads `hash` from the existing `@stellar/stellar-sdk` dependency. +Both paths hash UTF-8 and retain the first 32 hexadecimal characters, preserving +the existing 128-bit SHA-256-derived `idem_` representation. No new dependency, +canonical-payload format, nonce behavior, or API comparison is introduced. + +The two corrected fingerprints are: + +```text +idem_c1c4fbc87f8298fbaaa08f2abf483014 +idem_292b27e8347f595af102c42d966ae47c +``` + +## Existing browser sessions + +An old `idem_` fingerprint followed by eight lowercase hexadecimal characters +cannot be safely rebound to an entered payload: the old fingerprint may collide. +A strict pre-submit lookup therefore refuses choosing that weak record or +minting a new intent while an unresolved weak record remains. It leaves the +stored bytes, original idempotency key, and operation status unchanged and shows +a specific reconciliation message before calling `addTransfer`. + +Normal recovery reads still expose the original key. An accepted transfer can +be found by that key or its saved transfer ID and acknowledged through the +existing success dialog. A separately saved exact strong match may still be +retried. Failed or explicitly dismissed legacy records do not block a new +intent. No record is automatically deleted, rehashed, marked failed, or assumed +unsubmitted. Do not clear an unknown operation merely to bypass the message; +resolve its outcome through the existing transfer history/provider first. + +## Focused check + +From the repository root, using the existing locked dependencies: + +```sh +npx vitest run test/integration/send-money-crypto-recovery.test.jsx +``` + +The file checks native/fallback UTF-8 key equivalence, the real collision pair, +legacy states and unchanged journal bytes, independent strong retry, and the +actual Send Money block/reconciliation UI using the maintained mock API. +It does not establish live backend idempotency or cross-tab persistence. + +## Executed results — October 4, 2026 + +The isolated [hosted run 37193994776](https://github.com/woahwhattheheck/RemitFlow-Frontend/actions/runs/37193994776) +checked out exact product `fe08b70814f2a3e9994d457e60f487832680d389`, installed +the unchanged lockfile, and used Node 24.21.0, Vitest 4.1.10, and Stellar SDK +12.3.0. It executed only this ten-case file: nine passed, one failed. The failure +was the exact-text selector omitting the warning-icon prefix from the existing +`ErrorMessage` component. All crypto, collision, legacy state, strong retry, and +accepted-legacy reconciliation cases passed. This run is not reported as green. +Its original log is retained in [artifact 11299594080](https://github.com/woahwhattheheck/RemitFlow-Frontend/actions/runs/37193994776/artifacts/11299594080). + +The subsequent [composed source e6405a32](https://github.com/woahwhattheheck/RemitFlow-Frontend/commit/e6405a32e9c68df8281dcf17dbd5bcde068412d2) +preserves the crypto repair and adds the separate wallet-admission contribution. +It corrects only that assertion's displayed-text selector, retaining the +no-transfer and unchanged-journal assertions. Its recorded focused rerun passes +all ten cases in 4.13 seconds on Node 24.19.0 and Vitest 4.1.10. The same commit +records a successful composed Vite production build, including the deferred SDK +fallback chunk. These are separate local composition results, not a relabeling +of the failed hosted run or an additional run in that workflow. + +No second hosted run was launched after the completed composition evidence +became available. The previously recorded full-suite failures were still +outstanding at that stage. The subsequent combined package check below resolves +those local failures without relabeling the earlier hosted run. No live wallet, +provider, settlement, or payment outcome was exercised. + +## Combined package acceptance — October 4, 2026 + +The combined acceptance pass composed +the wallet-admission and crypto-recovery repairs with the existing +offline, form, precision, and quote fixture corrections. The tests follow the +actual Review, Confirm, and View Transfers flow and assert canonical decimal +strings and minimum-fee arithmetic. PR295 retains its session-storage resets; +the two other precision setups now also start with independent recovery state. +No test cases were removed, skipped, or added by this acceptance pass. + +One existing product mismatch also required correction: when quote construction +returns null after input validation, the review handler now displays the same +pricing error as confirmation instead of misreporting a nonpositive amount. +The existing no-submission regression assertion is preserved. + +The exact 352-file tracked source snapshot used the unchanged package manifest, +lockfile, test configuration, formatter configuration, and ignore rules. Logs +and the dependency cache were outside that source snapshot. The existing +Prettier 3.9.6 formatter corrected only its 13 flagged files. + +| Required local gate | Result | +| ---------------------------------- | --------------------------------------------------------------------------------------------------- | +| `prettier --check .` | Passed after formatting; unchanged formatter rules. | +| `npm audit --audit-level=critical` | Exit 0; zero critical findings. The report still contains 20 high, 12 moderate, and 2 low findings. | +| Full package test selection | **522 passed across 51 files**, 65.64 seconds, Vitest 4.1.10, two workers. | +| `vite build` | Passed in 4.79 seconds, Vite 5.4.21, 122 modules transformed. | + +The full suite used the package's maintained selection: + +```sh +node node_modules/vitest/vitest.mjs run \ + test/integration test/unit test/components test/services \ + test/lighthouse-config.test.js test/security --maxWorkers=2 +``` + +This execution used Node 24.19.0 and a writable temporary directory. The +repository's Node 22 hosted CI remains a separate result. These local checks +do not establish merge, sponsor acceptance, or payment. + +The build's main JavaScript chunk is 229.36 kB minified (74.37 kB gzip). The +deferred Stellar SDK fallback is 903.71 kB minified (234.96 kB gzip), which +continues to produce the existing large-chunk warning. This is a build-size +measurement, not a browser-load or live-provider performance benchmark. diff --git a/docs/validation/generated-transfer-records.md b/docs/validation/generated-transfer-records.md new file mode 100644 index 0000000..4de248a --- /dev/null +++ b/docs/validation/generated-transfer-records.md @@ -0,0 +1,26 @@ +# Generated transfer receipt identity + +`createTransfer` now applies caller transfer details before its generated `id`, initial `pending` status and creation timestamp. Those fields are owned by creation, not by the request fingerprint. A caller-supplied record ID can no longer create a second stored row with a seed record's ID, and a supplied `completed` status or historical timestamp does not become the initial receipt metadata. + +The change is limited to object assembly in the local mock API. The transfer contract, input amount normalization, idempotency fingerprint, in-flight sharing, saved-key replay and strict storage behavior are unchanged. This does not claim a server or live payment-provider vulnerability. + +## Reproduction and result + +Base: `91ccb2cb1071cae139f9be2d503395cb888d9f86`. +Tested source commit: `11a898caf3a1f05f985159bad387a7722f33c0e2`. +Source blob: `20606eda4770274d6876f038bd76383a8e6de7bd`. +Test blob: `de01c9bba5ca24ba0481440b3e700d96fb97b1b5`. +Runtime: Node `v24.21.0`, Linux, the repository's maintained Vitest/jsdom configuration and unchanged locked dependencies. + +The regression submits valid transfer details together with `id: 'tx_1001'`, `status: 'completed'` and `createdAt: '2000-01-01T00:00:00.000Z'`, both with an idempotency key and through the legacy keyless path. The old assembly accepts the supplied ID, colliding with the existing seed receipt. The repaired assembly creates a distinct pending receipt with the current timestamp and preserves the original seed row. + +On the original source, the new selection returned **2 failed / 1 passed**, exit 1; the 9 pre-existing cases were not selected in that baseline. The complete focused file on the repaired source returned **12 passed / 0 failed / 0 skipped**, exit 0, with 13.843 seconds recorded for the file. The replay control confirms that ignored metadata cannot alter an existing keyed receipt, including an in-flight replay. + +```sh +node node_modules/vitest/vitest.mjs run \ + test/unit/create-transfer-idempotency.test.js --maxWorkers=1 +``` + +[Execution and retained artifact](https://github.com/woahwhattheheck/RemitFlow-Frontend/actions/runs/37192132664). Artifact `11299641642`, ZIP SHA-256 `70edbc63bb9c3c79060adcc9a99d0384a633b050a82497b0b8d4b454557f4058`, contains the before/after JSON reports, commands, source hashes, exact changed files and patch. The isolated runner only published a validation ref; the original contribution branch was refreshed separately before integration. + +No full suite, production build, real browser, backend, chain or live-funds execution was performed for this repair. Earlier PR results remain evidence for their own source revisions, not a rerun on this change. diff --git a/src/context/AppContext.jsx b/src/context/AppContext.jsx index 336af1b..452bf9f 100644 --- a/src/context/AppContext.jsx +++ b/src/context/AppContext.jsx @@ -1,4 +1,3 @@ - import { createContext, useContext, useEffect, useState } from 'react'; import { connectWallet, diff --git a/src/hooks/useTransfers.js b/src/hooks/useTransfers.js index dea1e61..a3ebcea 100644 --- a/src/hooks/useTransfers.js +++ b/src/hooks/useTransfers.js @@ -1,4 +1,3 @@ - import { useCallback, useEffect, useState } from 'react'; import { listTransfers, createTransfer } from '../services/api.js'; import { ContractViolationError } from '../services/contracts/schema.js'; @@ -47,10 +46,18 @@ export function useTransfers() { const addTransfer = useCallback(async (payload) => { const created = await createTransfer(payload); - setTransfers((prev) => [created, ...prev]); + setTransfers((prev) => { + if (prev.some((t) => t.id === created.id)) return prev; + return [created, ...prev]; + }); return created; }, []); + const getTransferById = useCallback( + (id) => transfers.find((t) => t.id === id) ?? null, + [transfers], + ); + // Existing consumers use reload for both pull-to-refresh and the error-state // retry action. Withhold it only while a non-retryable error is displayed. const safeReload = error && !retryable ? undefined : reload; @@ -62,5 +69,6 @@ export function useTransfers() { retryable, reload: safeReload, addTransfer, + getTransferById, }; } diff --git a/src/pages/SendMoney.jsx b/src/pages/SendMoney.jsx index c1e03f0..41c88c1 100644 --- a/src/pages/SendMoney.jsx +++ b/src/pages/SendMoney.jsx @@ -9,7 +9,11 @@ import Modal from '../components/Modal.jsx'; import { buildQuote } from '../services/quote.js'; import { ContractViolationError } from '../services/contracts/schema.js'; import { getUserErrorMessage, normalizeError } from '../services/errors.js'; -import { formatAmount, formatCurrencyInput, parseCurrencyInput } from '../utils/format.js'; +import { + formatAmount, + formatCurrencyInput, + parseCurrencyInput, +} from '../utils/format.js'; import { isPositiveAmount, validateRecipient, @@ -17,6 +21,15 @@ import { } from '../utils/validate.js'; import { useWallet } from '../hooks/useWallet.js'; import { useTransfers } from '../hooks/useTransfers.js'; +import { + fingerprintTransferPayload, + idempotencyKeyFor, + newTransferIntentNonce, + saveTransferOperation, + getLatestRecoverableOperation, + TransferOperationStorageError, + LegacyTransferOperationError, +} from '../utils/transferIntent.js'; import { useOnlineStatus } from '../hooks/useOnlineStatus.js'; import { useApp } from '../context/AppContext.jsx'; import { useDebouncedValue } from '../hooks/useDebouncedValue.js'; @@ -36,7 +49,7 @@ import './SendMoney.css'; export default function SendMoney() { const navigate = useNavigate(); const { wallet, isConnected, connect } = useWallet(); - const { addTransfer } = useTransfers(); + const { addTransfer, getTransferById, transfers } = useTransfers(); const { locale } = useApp(); const isOnline = useOnlineStatus(); @@ -48,6 +61,8 @@ export default function SendMoney() { const [submitting, setSubmitting] = useState(false); const [submitError, setSubmitError] = useState(null); const submissionLock = useRef(false); + const intentKeyRef = useRef(null); + const intentFingerprintRef = useRef(null); const wasOffline = useRef(false); // True when the form just recovered from a disconnected state. Used to @@ -59,6 +74,44 @@ export default function SendMoney() { const [phase, setPhase] = useState(null); const [pendingQuote, setPendingQuote] = useState(null); const [submittedTransfer, setSubmittedTransfer] = useState(null); + + // Restore a recoverable transfer intent after navigation/refresh so a retry + // cannot mint a second transfer for the same payload. Succeeded intents show + // status once; dismissing the dialog marks them acknowledged. + useEffect(() => { + const recoverable = getLatestRecoverableOperation(); + if (!recoverable) return; + intentKeyRef.current = recoverable.idempotencyKey; + intentFingerprintRef.current = recoverable.fingerprint; + const existing = + (recoverable.transferId && getTransferById(recoverable.transferId)) || + transfers.find( + (transfer) => transfer.idempotencyKey === recoverable.idempotencyKey, + ); + if (existing) { + if ( + recoverable.status !== 'succeeded' || + recoverable.transferId !== existing.id + ) { + saveTransferOperation({ + ...recoverable, + transferId: existing.id, + status: 'succeeded', + }); + } + setSubmitError(null); + setSubmittedTransfer(existing); + setPhase('success'); + } else if ( + recoverable.status === 'unknown' || + recoverable.status === 'submitting' + ) { + setSubmitError( + 'Transfer status is unknown. Check Transfers, or re-enter the same details to retry safely.', + ); + } + }, [getTransferById, transfers]); + const submitButtonRef = useRef(null); // Debounce the amount so the quote isn't rebuilt on every keystroke. @@ -167,7 +220,9 @@ export default function SendMoney() { } const finalQuote = buildQuote(parsedAmount.value, from, to); if (!finalQuote) { - applyErrors({ amount: 'Enter an amount greater than zero.' }); + setSubmitError( + 'We could not price this transfer. Check the amount and the selected currencies.', + ); return; } @@ -187,18 +242,28 @@ export default function SendMoney() { submissionLock.current = true; setSubmitting(true); try { - if (!isConnected) { - await connect(); + const transferWallet = isConnected ? wallet : await connect(); + if (!transferWallet) { + setSubmitError( + 'Wallet connection did not complete. Connect your wallet and try again.', + ); + setPendingQuote(null); + setPhase(null); + return; } // Rebuild at confirmation time so the committed amounts match the note: // rates are indicative and update at confirmation. - const parsedAmount = parseCurrencyInput(amount, { currency: from, locale }); + const parsedAmount = parseCurrencyInput(amount, { + currency: from, + locale, + }); if (!parsedAmount.ok) { setSubmitError(parsedAmount.error); return; } - const finalQuote = pendingQuote ?? buildQuote(parsedAmount.value, from, to); + const finalQuote = + pendingQuote ?? buildQuote(parsedAmount.value, from, to); if (!finalQuote) { setSubmitError( 'We could not price this transfer. Check the amount and the selected currencies.', @@ -206,10 +271,22 @@ export default function SendMoney() { return; } + // A wallet connected here was unavailable during the form's balance check. + if ( + !isWithinBalance(finalQuote.sendAmount, transferWallet.balance, { + currency: finalQuote.from, + }) + ) { + setSubmitError('Amount exceeds your wallet balance.'); + setPendingQuote(null); + setPhase(null); + return; + } + // Record the fee, rate and expiry alongside the amounts so the receipt // can reproduce exactly what was quoted rather than re-deriving it from // a rate that may since have moved. - const created = await addTransfer({ + const payload = { recipient, from, to, @@ -218,6 +295,38 @@ export default function SendMoney() { fee: finalQuote.fee, rate: finalQuote.rate, expiresAt: finalQuote.expiresAt, + }; + // Persist only an opaque fingerprint, never recipient or quote details. + const fingerprint = await idempotencyKeyFor( + fingerprintTransferPayload(payload), + ); + if ( + intentFingerprintRef.current !== fingerprint || + !intentKeyRef.current + ) { + const recoverable = getLatestRecoverableOperation(fingerprint, { + strict: true, + }); + intentKeyRef.current = + recoverable?.idempotencyKey ?? + (await idempotencyKeyFor(fingerprint, newTransferIntentNonce())); + intentFingerprintRef.current = fingerprint; + } + const idempotencyKey = intentKeyRef.current; + saveTransferOperation( + { + idempotencyKey, + fingerprint, + status: 'submitting', + }, + { strict: true }, + ); + const created = await addTransfer({ ...payload, idempotencyKey }); + saveTransferOperation({ + idempotencyKey, + fingerprint, + transferId: created?.id, + status: 'succeeded', }); setSubmittedTransfer(created ?? finalQuote); setPendingQuote(null); @@ -226,27 +335,56 @@ export default function SendMoney() { } catch (err) { setPendingQuote(null); setPhase(null); - if (err instanceof ContractViolationError) { + if (err instanceof LegacyTransferOperationError) { + // Preserve the old operation and its key until its outcome is known. + // Neither addTransfer nor a replacement journal write has occurred. + setSubmitError(err.message); + } else if (err instanceof TransferOperationStorageError) { + // addTransfer was not called. Keep the in-memory intent for a safe + // retry, and do not attempt another write to unavailable storage. + setSubmitError( + 'This attempt was not submitted because your browser could not preserve its recovery information. Check browser storage and try again.', + ); + } else if (err instanceof ContractViolationError) { // The full field-by-field diff goes to the console; the user gets a // message that distinguishes "we rejected this" from "try again". console.error(err.message); + // Nothing was submitted — drop the intent so a corrected payload can + // start a fresh one, and a retry of the same payload gets a new key + // only if the user edits (same fingerprint reuses the key below). + if (intentKeyRef.current) { + saveTransferOperation({ + idempotencyKey: intentKeyRef.current, + fingerprint: intentFingerprintRef.current, + status: 'failed', + }); + } setSubmitError( 'This transfer was rejected before it was sent because the details did not match the expected format. Nothing was submitted.', ); } else { const normalized = normalizeError(err, { source: 'api' }); - // A transfer can be interrupted mid-signature by a connection drop. - // The honest message here is "unknown", not "failed": the backend may - // have accepted the transfer even though the response never arrived. - // The transfers page reconciles real status on reconnect. - // Read the current connectivity directly (not from the render closure) - // so that a mid-flight disconnect produces the correct message. - const connectedNow = typeof navigator !== 'undefined' && navigator.onLine; - setSubmitError( - connectedNow - ? getUserErrorMessage(normalized) - : 'Connection lost while sending. Reconnect to check your transfer status.', - ); + // A transfer can be interrupted mid-signature by a connection drop or + // timeout. The honest message here is "unknown", not "failed": the + // backend may have accepted the transfer even though the response never + // arrived. Keep the intent as `unknown` so refresh/retry reuses the same + // idempotency key instead of minting a duplicate. + // Read the current connectivity directly (not from the render closure) + // so that a mid-flight disconnect produces the correct message. + const connectedNow = + typeof navigator !== 'undefined' && navigator.onLine; + if (intentKeyRef.current) { + saveTransferOperation({ + idempotencyKey: intentKeyRef.current, + fingerprint: intentFingerprintRef.current, + status: 'unknown', + }); + } + setSubmitError( + connectedNow + ? getUserErrorMessage(normalized) + : 'Connection lost while sending. Reconnect to check your transfer status.', + ); } } finally { submissionLock.current = false; @@ -254,6 +392,20 @@ export default function SendMoney() { } } + function dismissSuccess() { + if (intentKeyRef.current) { + saveTransferOperation({ + idempotencyKey: intentKeyRef.current, + fingerprint: intentFingerprintRef.current, + transferId: submittedTransfer?.id, + status: 'dismissed', + }); + } + intentKeyRef.current = null; + intentFingerprintRef.current = null; + setPhase(null); + } + const errorCount = Object.keys(errors).length; return ( @@ -292,8 +444,8 @@ export default function SendMoney() { role="status" aria-live="polite" > - ✓ Back online. Your form was not submitted while you were - offline — review it and send when ready. + ✓ Back online. Your form was not submitted while you were offline + — review it and send when ready. )} @@ -397,7 +549,7 @@ export default function SendMoney() { )} {phase === 'success' && submittedTransfer && ( - setPhase(null)} title="Transfer submitted"> +

Your transfer was submitted successfully. Track its progress under Transfers. @@ -433,7 +585,7 @@ export default function SendMoney() {

-