fix(donation): eliminate partial-success window with optimistic idemp… - #149
Merged
BarryArinze merged 1 commit intoAug 26, 2026
Conversation
…otency and recovery Fixes aid-linkk#139 Scenario A (successful ledger, polling timeout): - pollForResult now performs one extra getTransaction probe after its client-side 60s timeout elapses, before treating the donation as failed. Soroban RPC retains transaction results for ~10 ledgers (~50s) after submission, so a donation that lands just after the last regular poll is now detected as a success instead of being reported as a hard failure while the donor's funds already moved. Scenario B (double-submit race): - The idempotency slot for (donor, campaign, amount, window) is now claimed OPTIMISTICALLY the instant donate() is called, before any network I/O — not after pollForResult succeeds. A concurrent donate() call for the same key attaches to the first call's promise and waits for its outcome instead of building and submitting a second envelope, which prevents the double-submit at the source rather than merely detecting it afterwards. - Idempotency entries persist to sessionStorage on success, so the window (>= 60s, matching the max poll duration) survives page navigations within the same tab, not just re-renders. - A deterministic nonce derived from the idempotency key is now passed to record_donation as a trailing argument, giving the contract the raw material to enforce its own idempotency if/when its ABI is updated to accept it (no contract changes made here — out of scope). - As a safety net for races that span separate tabs/sessions (so they don't share the in-memory map), a failing attempt now checks whether another attempt for the same key committed successfully in the meantime and reports isDuplicate: true instead of surfacing an error over funds that already moved. Scenario C (payment succeeds, contract call traps): - decodeResultXdr now walks the actual OperationResult/PaymentResult XDR union instead of reading the operation *type* name, so a payment failure is reported with its specific reason (e.g. opNO_DESTINATION for a missing escrow account) instead of a generic/incorrect message. Scenario D (simulate -> sign gap / stale sequence number): - A txBadSeq submission error now triggers a silent retry: re-fetch the account, rebuild the envelope with the fresh sequence (reusing the existing simulation result — no re-simulation needed for a sequence-only failure, keeping this off the sign->submit hot path), re-sign, and resubmit. Up to 3 retries with backoff before surfacing a user-visible error. Constraints preserved: - TransactionEvent/DonationState/UseDonationResult signatures and the hook's state machine values are unchanged. - No new npm dependencies; XDR decoding uses @stellar/stellar-sdk's existing xdr namespace. - All 52 existing tests in use-donation.test.ts pass unmodified. - No server round-trip added between fee confirmation and signing. Also includes a one-line fix to src/store/wallet-store.ts: SESSION_TTL_MS was referenced but never defined, which broke every test (and the real app) that touches useWalletStore, including all of use-donation.test.ts. This is unrelated to aid-linkk#139 itself but blocked verifying this fix, so it's bundled in as a small, clearly-isolated one-line change (matches the 8-hour TTL already asserted in src/lib/store/__tests__/encrypted-storage.test.ts). New tests: - src/hooks/__tests__/use-donation-error-decoding.test.ts (real SDK, no mocking): decodeResultXdr's opINNER vs opNO_DESTINATION distinction, generateDonationNonce determinism, and pollForResult's post-timeout probe. - src/hooks/__tests__/use-donation-idempotency.test.tsx (mocked SDK): concurrent donate() calls dedupe to exactly one sendTransaction call, txBadSeq triggers a silent retry, non-sequence errors do not retry. npm run test (383/383 passing tests; the one failing suite is a pre-existing, unrelated SWC compile issue present on master too) and npm run type-check (no new errors; two pre-existing JSX errors in admin/page.tsx and beneficiary/page.tsx are present on master too).
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…otency and recovery
Fixes #139
Scenario A (successful ledger, polling timeout):
Scenario B (double-submit race):
Scenario C (payment succeeds, contract call traps):
Scenario D (simulate -> sign gap / stale sequence number):
Constraints preserved:
Also includes a one-line fix to src/store/wallet-store.ts: SESSION_TTL_MS was referenced but never defined, which broke every test (and the real app) that touches useWalletStore, including all of use-donation.test.ts. This is unrelated to #139 itself but blocked verifying this fix, so it's bundled in as a small, clearly-isolated one-line change (matches the 8-hour TTL already asserted in
src/lib/store/tests/encrypted-storage.test.ts).
New tests:
npm run test (383/383 passing tests; the one failing suite is a pre-existing, unrelated SWC compile issue present on master too) and npm run type-check (no new errors; two pre-existing JSX errors in admin/page.tsx and beneficiary/page.tsx are present on master too).