You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(transfers): prevent a double payment on a repeated transfer - #8138
The warning in #8137 does not stop a double payment. Each send gets a new created_at_time, so the ledger treats a repeated send as a new transfer. If the repeated send reuses the timestamp, the ledger rejects it as a duplicate.
Changes
Added transfer-attempts.utils, which saves the timestamp of each transfer in local storage before the ledger call, for all tabs and for at most 24 hours.
Reused the timestamp in transferICP and transferTokens if the user repeats the same transfer.
Handled the duplicate rejection of a repeated transfer as a success, with a new "already sent" toast.
Removed the saved timestamp after a success, a duplicate, a "too old" rejection, or a rejected first attempt, so a real second payment stays possible.
Screenshots
Tests
Added unit tests for the attempts, the repeated transfers, and the toast in the ICP, ICRC, and ckBTC modals.
Ran npm run check, the unit tests, and npm run build.
The screenshot shows the real toast, triggered from a local dev server. No test used a real ledger or a real lost reply.
Save the timestamp of a transfer with an unknown outcome. If the user
repeats the same transfer, reuse the timestamp, so the ledger rejects
the retry as a duplicate if the first attempt executed.
Show the duplicate rejection as "already sent", and forget the transfer.
The user can then repeat the transfer to make a real second payment.
► frontend/src/lib/types/i18n.d.ts Expose transaction_already_sent in accounts translations
Enhancement
► frontend/src/lib/modals/accounts/CkBTCTransactionModal.svelte Handle alreadySent flag for success toasts
Enhancement
► frontend/src/lib/modals/accounts/IcpTransactionModal.svelte Handle alreadySent flag for success toasts
Enhancement
► frontend/src/lib/modals/accounts/IcrcTokenTransactionModal.svelte Handle alreadySent flag for success toasts
Enhancement
► frontend/src/lib/services/icp-accounts.services.ts Integrate transfer attempts logic; support alreadySent flow; pass createdAt in calls
Enhancement
► frontend/src/lib/services/icrc-accounts.services.ts Integrate transfer attempts logic; support alreadySent flow; pass createdAt in calls
Enhancement
► frontend/src/tests/lib/modals/accounts/CkBTCTransactionModal.spec.ts Add test for already sent toast
Enhancement
► frontend/src/tests/lib/modals/accounts/IcpTransactionModal.spec.ts Add test for already sent toast
Enhancement
► frontend/src/tests/lib/modals/accounts/IcrcTokenTransactionModal.spec.ts Add test for already sent toast
Enhancement
► frontend/src/tests/lib/routes/Accounts.spec.ts Expect createdAt in icp/sendICP calls
Enhancement
► frontend/src/tests/lib/routes/Wallet.spec.ts Expect createdAt in icrcTransfer calls
Enhancement
► frontend/src/tests/lib/services/icp-accounts.services.spec.ts Include tests for transfer attempts unknown outcome flow; expect createdAt in calls; duplicates handling
Enhancement
► frontend/src/tests/lib/services/icrc-accounts.services.spec.ts Include tests for transfer tokens with createdAt; duplicates handling; unknown outcomes flow
Enhancement
► frontend/src/tests/lib/utils/transfer-attempts.utils.spec.ts New tests for transfer attempts utilities (start/clear, timestamp reuse, expiry)
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Critical duplicate-payment risks remain because attempts are not persisted before dispatch, cross-tab state can become stale or overwrite saved attempts, and equivalent legacy ICP memos generate different retry keys.
This PR extends the unknown-outcome transfer warning with timestamp reuse, aiming to prevent duplicate payments when users retry ICP or ICRC transfers.
Changes:
Adds local storage for unresolved transfer timestamps with a 24-hour reuse window.
Reuses timestamps on retries and reports recognized duplicates as already sent.
Updates transfer modals, translations, and tests for retry handling.
…app--claude-worktrees-kind-feistel-e52350/e547e7c7-259e-45c0-bb73-6bdd37933a78/scratchpad
gh pr view 8138 --json body -q .body > $S/pr-body-dedup-live.md
python3 - "$S/pr-body-dedup-live.md" <<'P'
import sys
p=sys.argv[1]; s=open(p).read()
pairs=[
("- Added `unknownTransfersStore`, which keeps the timestamp of a transfer with an unknown outcome in local storage for 24 hours.\n",
"- Added `transfer-attempts.utils`, which saves the timestamp of each transfer in local storage before the ledger call, for all tabs and for at most 24 hours.\n"),
("- Removed the saved timestamp after a success, a duplicate, or a \"too old\" rejection, so a real second payment stays possible.\n",
"- Removed the saved timestamp after a success, a duplicate, a \"too old\" rejection, or a rejected first attempt, so a real second payment stays possible.\n"),
("- Added unit tests for the store, the repeated transfers,","- Added unit tests for the attempts, the repeated transfers,"),
]
for a,b in pairs:
assert a in s, a
s=s.replace(a,b)
open(p,'w').write(s)
P
gh pr edit 8138 --body-file $S/pr-body-dedup-live.md | tail -1
~/.claude/skills/copilot-review-loop/scripts/copilot-review.sh request 8138 | tail -2
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Critical persistence and cross-tab cleanup flaws can lose payment attempt timestamps, allowing retries after lost responses to execute duplicate payments.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Concurrent timestamp cleanup races and inconsistent destination and memo retry keys can cause retries after lost replies to use fresh timestamps, allowing duplicate payments.
No open issues 🎉
3 previously missed issues
In code that hasn't changed since last review
Normalize equivalent destination spellings before building retry keys
The retry key uses the destination's spelling rather than its encoded account, so retrying after a lost reply with equivalent text—such as uppercase versus lowercase ICP hex, or ICRC subaccount suffixes .1 versus .01—gets a fresh timestamp and can pay twice. Lowercase legacy ICP destinations and use encodeIcrcAccount(decodeIcrcAccount(to)) for ICRC destinations before building the key. Apply the same normalization in frontend/src/lib/services/icrc-accounts.services.ts:306 using encodeIcrcAccount(to) on the already-decoded account.
Normalize omitted legacy memo and zero to the same retry key
The legacy ICP SDK substitutes 0n when the memo is omitted, but this key uses undefined in that case and "0" for an explicitly entered zero (including an empty string accepted by BigInt). After a lost reply, retrying with the equivalent zero memo therefore gets a new timestamp and can pay twice. Apply the SDK's default when constructing the legacy key, while keeping ICRC text-memo semantics unchanged.
Shared reservation cleanup can clear another tab’s unresolved timestamp
If tab A reserves a timestamp and tab B reuses it, A's definite rejection can clear the shared entry even while B remains unresolved. If B executes and loses its reply, the next send gets a fresh timestamp and can pay twice. Coordinate same-key submissions through reservation, sending, and cleanup, or retain the timestamp while any submission remains unresolved; the same issue exists in frontend/src/lib/services/icp-accounts.services.ts:399.
Also handled the destination and memo keys. The two-tab race is not necessary to fix: it needs the same transfer in two tabs at once, with a rejection in one and a lost reply in the other. A lock across tabs is not worth it for that.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because cross-format retries and shared timestamp cleanup can cause duplicate payments, while ICP ICRC-1 retry lifecycle behavior lacks test coverage.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because overlapping cross-tab submissions can prematurely clear a shared reservation, allowing a retry with a fresh timestamp to cause duplicate payments.
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
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.
Motivation
The warning in #8137 does not stop a double payment. Each send gets a new
created_at_time, so the ledger treats a repeated send as a new transfer. If the repeated send reuses the timestamp, the ledger rejects it as a duplicate.Changes
transfer-attempts.utils, which saves the timestamp of each transfer in local storage before the ledger call, for all tabs and for at most 24 hours.transferICPandtransferTokensif the user repeats the same transfer.Screenshots
Tests
npm run check, the unit tests, andnpm run build.Todos
Prev. #8137