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
[$250] [Exploratory] Email - Can transfer the same balance twice to two accounts and receive 2 emails #100290
Preconditions:
Both accounts with wallet enabled are set up, and in USD by default
User B has set up 2 personal bank accounts: xxxx0000 and xxxx1111
Steps
As User A - Pay User B so they have a positive amount in their wallet
As User B - Navigate to Wallet and Click “Transfer”
Select bank accounts: xxxx0000 -> Transfer
Repeat and select bank accounts: xxxx1111 -> Transfer
Expected Result:
User should only transfer to one bank account
Actual Result:
User can transfer to 2 bank accounts and receive 2 emails about the same balance
Nothing in the App clears or re-reads the wallet balance after a successful transfer, so the Transfer button stays enabled on a balance that has already been sent.
transferWalletBalance() writes only to ONYXKEYS.WALLET_TRANSFER (loading / shouldShowSuccess). Its OnyxUpdate type is narrowed to that key, so it structurally cannot touch userWallet.
The Wallet page's only fetch effect is keyed on [network.isOffline] (WalletPage/index.tsx:407-412). Transfer Balance is an RHP screen layered over a still-mounted Wallet page, so returning from it does not re-run getPaymentMethods().
So on the second visit the page re-mounts, resetWalletTransferData() clears loading/shouldShowSuccess, the stale balance still reads > 0, and a second TransferWalletBalance fires with the other bankAccountID.
This is not a double-tap race — the button is genuinely disabled while walletTransfer.loading is true. It is two deliberate, sequential submits against a stale balance.
Supporting evidence and what I could not verify
The client is not swallowing a server update.SaveResponseInOnyx.ts:29 applies any onyxData the server returns, with no per-command allowlist, and userWallet is read reactively via useOnyx. If a zeroed currentBalance had reached the client at any point before transfer #2, the button would have disabled itself with no code change. It didn't — so no fresh balance arrived.
availableBalance is declared but never used.UserWallet.ts:17-19 has both availableBalance and currentBalance, but every transfer decision in the App reads currentBalance. If the backend reserves funds by moving availableBalance while currentBalance only settles later, that alone produces this bug.
Secondary defect in the same flow.resetWalletTransferData() unconditionally sets loading: false on every mount. Navigating away mid-request and back re-enables the button before the first response lands, allowing a true concurrent double-submit.
Alternatives I ruled out. A double-tap race (button is disabled via isLoading — Button.tsx:146); a missing in-flight guard (it exists and works); a duplicate-notification-only bug (the reporter's video shows both transfers completing).
Not verified. I could not confirm from production logs whether both requests returned success, nor whether the second transfer actually moved money a second time. Log tooling was unavailable in this run, and the backend command is outside this repo. The App-side conclusion above rests on code reading plus the reported repro.
Adjacent bug worth fixing in the same PR.TransferBalancePage.tsx:149 has an operator-precedence bug: ?? binds looser than -, so userWallet?.currentBalance ?? 0 - calculatedFee parses as userWallet?.currentBalance ?? (0 - calculatedFee). The fee is never subtracted when currentBalance is defined. Harmless today only because the ACH rate and minimum fee are both 0 and the instant option is commented out.
What changes do you think we should make in order to solve the problem?
Refresh the wallet after a successful transfer. In dismissSuccessfulTransferBalancePage(), call getPaymentMethods() (the OpenPaymentsPage read) before navigating back, so the Wallet page and any later visit to Transfer Balance see the real balance instead of the pre-transfer one.
Optimistically zero the balance. Add a USER_WALLET entry to transferWalletBalance()'s successData setting currentBalance (and availableBalance) to 0, with failureData restoring the prior value. This closes the window before the refetch returns and makes the button disable itself immediately.
Stop clobbering the in-flight flag. Have resetWalletTransferData() preserve loading when a request is outstanding, rather than forcing it to false on every mount.
Fix the precedence bug on TransferBalancePage.tsx:149 by parenthesising: (userWallet?.currentBalance ?? 0) - calculatedFee.
Add regression tests. There are currently none for this flow — the only transfer-related assertion in the whole suite is a fee-rounding test in tests/unit/PaymentUtilsTest.ts:36. A test asserting the button is disabled after a successful transfer would have caught this.
⚠️This fix alone is not sufficient. Steps 1–3 stop the App from offering a second transfer, but the client sends no amount and no idempotency key — the request body is just {bankAccountID}. Preventing the same balance from actually moving twice requires a server-side balance re-check or lock on the transfer command, which lives outside this repo. Someone with backend access should confirm whether that guard exists and whether the transfer response returns an updated wallet balance. Please escalate that half to an internal engineer.
What alternative solutions did you explore? (Optional)
Gate on availableBalance instead of currentBalance. Cleaner if the backend already reserves funds there, but the App never reads that field today and I could not confirm the backend populates it on transfer. Worth checking as part of the backend escalation — if it does, this becomes the simplest fix.
Hide the Transfer Balance menu item when the balance is zero.WalletPage/index.tsx:832-848 gates it only on wallet tier. Adding a balance check is a reasonable defence-in-depth addition, but it fixes nothing on its own since it reads the same stale value.
Send an idempotency key with the transfer. The right long-term shape, but it needs a matching backend change, so it can't ship from this repo alone.
Next Steps for Contributor+ team: To accept:@MelvinBot implement [this](https://github.com/Expensify/App/issues/100290) to create a draft PR. To refine:@MelvinBot <your feedback> To reject: Explain why you are rejecting Melvin's proposal.
Blocked on dev NewDot web: Account opened Profile, but no Wallet navigation was exposed. No transfers were submitted or fixtures invented.
Step
Status
Positive wallet balance and two linked personal bank accountsPrerequisites could not be verified. Snapshots showed Profile; the Wallet selector did not resolve.
❌
Bank A transfer succeeds and returns to Wallet with zero balanceNot tested: Wallet was inaccessible and no safe funded fixture was established.
❌
Reopening Transfer balance and selecting Bank B prevents a second transferNot tested: prerequisite transfer state unavailable.
❌
Leaving and reopening during a pending transfer keeps submission disabledNot tested: no pending-transfer fixture available.
Six action tests, typecheck, changed-file lint, explicit-file compiler check, and Storybook smoke test passed. Broader checks remain incomplete; details are in the PR. Backend duplicate-transfer protection still needs internal confirmation.
Please mark it as "Ready for review" or ask me to do that for you when it is ready for review.
Activity
applause-bot commentedon Sep 3, 2026
While Applause doesn't embed attachments for this bug, here are the links for quick view:
https://platform.applause.com/services/links/v1/external/00b70febfc5845d810b6d09881b5917821a063b43d9d07db6c76a43240a9e740
MelvinBot commentedon Sep 24, 2026
Proposal
What is the root cause of that problem?
Nothing in the App clears or re-reads the wallet balance after a successful transfer, so the Transfer button stays enabled on a balance that has already been sent.
The only thing gating a transfer is
TransferBalancePage.tsx:148-151, which reads the Onyx-cacheduserWallet.currentBalance:That cached value is never invalidated:
transferWalletBalance()writes only toONYXKEYS.WALLET_TRANSFER(loading/shouldShowSuccess). ItsOnyxUpdatetype is narrowed to that key, so it structurally cannot touchuserWallet.dismissSuccessfulTransferBalancePage()just flipsshouldShowSuccessand callsNavigation.goBack()— no refetch.[network.isOffline](WalletPage/index.tsx:407-412). Transfer Balance is an RHP screen layered over a still-mounted Wallet page, so returning from it does not re-rungetPaymentMethods().So on the second visit the page re-mounts,
resetWalletTransferData()clearsloading/shouldShowSuccess, the stale balance still reads> 0, and a secondTransferWalletBalancefires with the otherbankAccountID.This is not a double-tap race — the button is genuinely disabled while
walletTransfer.loadingis true. It is two deliberate, sequential submits against a stale balance.Supporting evidence and what I could not verify
The client is not swallowing a server update.
SaveResponseInOnyx.ts:29applies anyonyxDatathe server returns, with no per-command allowlist, anduserWalletis read reactively viauseOnyx. If a zeroedcurrentBalancehad reached the client at any point before transfer #2, the button would have disabled itself with no code change. It didn't — so no fresh balance arrived.availableBalanceis declared but never used.UserWallet.ts:17-19has bothavailableBalanceandcurrentBalance, but every transfer decision in the App readscurrentBalance. If the backend reserves funds by movingavailableBalancewhilecurrentBalanceonly settles later, that alone produces this bug.Secondary defect in the same flow.
resetWalletTransferData()unconditionally setsloading: falseon every mount. Navigating away mid-request and back re-enables the button before the first response lands, allowing a true concurrent double-submit.Alternatives I ruled out. A double-tap race (button is disabled via
isLoading—Button.tsx:146); a missing in-flight guard (it exists and works); a duplicate-notification-only bug (the reporter's video shows both transfers completing).Not verified. I could not confirm from production logs whether both requests returned success, nor whether the second transfer actually moved money a second time. Log tooling was unavailable in this run, and the backend command is outside this repo. The App-side conclusion above rests on code reading plus the reported repro.
Adjacent bug worth fixing in the same PR.
TransferBalancePage.tsx:149has an operator-precedence bug:??binds looser than-, souserWallet?.currentBalance ?? 0 - calculatedFeeparses asuserWallet?.currentBalance ?? (0 - calculatedFee). The fee is never subtracted whencurrentBalanceis defined. Harmless today only because the ACH rate and minimum fee are both0and the instant option is commented out.What changes do you think we should make in order to solve the problem?
dismissSuccessfulTransferBalancePage(), callgetPaymentMethods()(theOpenPaymentsPageread) before navigating back, so the Wallet page and any later visit to Transfer Balance see the real balance instead of the pre-transfer one.USER_WALLETentry totransferWalletBalance()'ssuccessDatasettingcurrentBalance(andavailableBalance) to0, withfailureDatarestoring the prior value. This closes the window before the refetch returns and makes the button disable itself immediately.resetWalletTransferData()preserveloadingwhen a request is outstanding, rather than forcing it tofalseon every mount.TransferBalancePage.tsx:149by parenthesising:(userWallet?.currentBalance ?? 0) - calculatedFee.tests/unit/PaymentUtilsTest.ts:36. A test asserting the button is disabled after a successful transfer would have caught this.What alternative solutions did you explore? (Optional)
availableBalanceinstead ofcurrentBalance. Cleaner if the backend already reserves funds there, but the App never reads that field today and I could not confirm the backend populates it on transfer. Worth checking as part of the backend escalation — if it does, this becomes the simplest fix.WalletPage/index.tsx:832-848gates it only on wallet tier. Adding a balance check is a reasonable defence-in-depth addition, but it fixes nothing on its own since it reads the same stale value.Next Steps for Contributor+ team:
To accept:
@MelvinBot implement [this](https://github.com/Expensify/App/issues/100290)to create a draft PR.To refine:
@MelvinBot <your feedback>To reject: Explain why you are rejecting Melvin's proposal.
view run
melvin-bot commentedon Sep 24, 2026
Triggered auto assignment to Contributor-plus team member for initial proposal review - @eh2077 (
External)melvin-bot commentedon Sep 24, 2026
Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.
melvin-bot commentedon Sep 29, 2026
@eh2077 Eep! 4 days overdue now. Issues have feelings too...
eh2077 commentedon Oct 1, 2026
checking
64 remaining items
melvin-bot commentedon Nov 2, 2026
@situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!
melvin-bot commentedon Nov 3, 2026
@situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
melvin-bot commentedon Nov 3, 2026
@situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!
melvin-bot commentedon Nov 3, 2026
@situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
situchan commentedon Nov 4, 2026
@MelvinBot draft implementation for your proposal
MelvinBot commentedon Nov 4, 2026
🤖 Web test steps verification results: ❌ fail
Blocked on dev NewDot web: Account opened Profile, but no Wallet navigation was exposed. No transfers were submitted or fixtures invented.
Positive wallet balance and two linked personal bank accounts
Prerequisites could not be verified. Snapshots showed Profile; the Wallet selector did not resolve.Bank A transfer succeeds and returns to Wallet with zero balance
Not tested: Wallet was inaccessible and no safe funded fixture was established.Reopening Transfer balance and selecting Bank B prevents a second transfer
Not tested: prerequisite transfer state unavailable.Leaving and reopening during a pending transfer keeps submission disabled
Not tested: no pending-transfer fixture available.Draft PR created: Prevent duplicate wallet balance transfers, assigned to
situchan.Six action tests, typecheck, changed-file lint, explicit-file compiler check, and Storybook smoke test passed. Broader checks remain incomplete; details are in the PR. Backend duplicate-transfer protection still needs internal confirmation.
Please mark it as "Ready for review" or ask me to do that for you when it is ready for review.
view run · no recording available
melvin-bot commentedon Nov 5, 2026
@situchan Eep! 4 days overdue now. Issues have feelings too...
jeffrey-graham commentedon Nov 7, 2026
melvin-bot commentedon Nov 7, 2026
melvin-bot commentedon Nov 10, 2026
@situchan Huh... This is 4 days overdue. Who can take care of this?