Skip to content

fix(money): correct transfer toasts (MUSD-845)#30770

Merged
Kureev merged 10 commits into
mainfrom
kureev/MUSD-845
Jun 3, 2026
Merged

fix(money): correct transfer toasts (MUSD-845)#30770
Kureev merged 10 commits into
mainfrom
kureev/MUSD-845

Conversation

@Kureev

@Kureev Kureev commented May 28, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to PR #30741 (MUSD-849, MUSD-845) which corrected the deposit toasts. The same lifecycle for transfer (Money account → another account) was still showing the generic "Transaction in progress / Transaction complete / {{amount}} moved to Between accounts." copy.

This PR updates the three withdraw lifecycle toasts to match Figma. The new copy is used for every transfer out of the Money account (today: "Transfer → Between accounts"; the upcoming Perps / Predictions / external-send paths will reuse the same lifecycle):

State Title Body
In progress Transfer in progress This may take a few minutes.
Success Transfer complete {{amount}} added to {{destination}}.
Failed Transfer failed Unable to transfer funds. Try again.

Figma references:

Because all transfers share this copy today, no intent layer is introduced on the withdraw side — the toast subscriber just maps TransactionType.moneyAccountWithdraw to the new strings unconditionally. (The deposit-intent infrastructure from #30741 stays in place — Convert crypto vs. Add mUSD still have distinct copy.)

Changelog

CHANGELOG entry: Updated the in-progress, success, and failed toasts for Money account transfers to "Transfer in progress / Transfer complete / Transfer failed" with the Figma body copy.

Related issues

Fixes: MUSD-845

Manual testing steps

Feature: Transfer toast copy

  Scenario: Transfer between accounts lifecycle
    Given the user is on the Money account home screen
    And the user has a non-zero mUSD balance

    When the user taps Transfer
    And taps "Between accounts"
    And confirms the transaction on the confirmation screen
    Then after a brief deferral the in-progress toast shows "Transfer in progress" / "This may take a few minutes."
    When the transaction is confirmed
    Then the success toast shows "Transfer complete" / "${{amount}} added to Money account."

  Scenario: Transfer failure
    Given the user submitted a transfer
    When the transaction fails (dropped / cancelled / failed)
    Then the failed toast shows "Transfer failed" / "Unable to transfer funds. Try again."

Screenshots/Recordings

Before

In-progress / success toasts used the generic "Transaction in progress" / "Transaction complete" / "{{amount}} moved to Between accounts." copy.

After

Toasts match the Figma per state (see table above).

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
  • I've tested with a power user scenario
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Low Risk
User-facing toast strings and destination labeling for Money withdraws only; no auth, payments, or persistence changes beyond reading account metadata at confirm time.

Overview
Money account transfer (withdraw) toasts now use Figma-specific copy instead of the generic deposit-style “Transaction in progress / complete” strings and the hardcoded “Between accounts” success destination.

Copy & i18n: Withdraw in-progress, success, and failed titles/bodies are wired to new money.toasts.withdraw_* keys (e.g. “Transfer in progress”, “Transfer complete”, “{{amount}} added to {{destination}}.”, “Transfer failed”). Success wording shifts from “moved to” to “added to” the destination.

Success destination: On confirmed withdraw, useMoneyTransactionStatus derives the label from the nested ERC20 transfer recipient: internal accounts use account group name (preferred), then account name, then shortened address for unknown recipients, with “your account” when metadata or nested transfer is missing.

Tests were tightened for exact toast strings and for the destination-resolution fallbacks.

Reviewed by Cursor Bugbot for commit 0af17bb. Bugbot is set up for automated code reviews on this repo. Configure here.

Mirrors the deposit-intent pattern (PR #30741) for the withdraw flow so
transfer "between accounts" emits its own toast copy per Figma:

  - In progress: "Transfer in progress" / "This may take a few minutes."
  - Success:     "Transfer complete"     / "{{amount}} added to Money account."
  - Failed:      "Transfer failed"       / "Unable to transfer funds. Try again."

initiateWithdrawal() now pre-generates a client-side batchId and registers
a MoneyAccountWithdrawIntent ("betweenAccounts" by default) before the
addTransactionBatch await, so useMoneyTransactionStatus can resolve the
intent for both deferred in-progress toasts and the synchronous immediate
failure path. The intent is cleared on every terminal status (confirmed,
failed, dropped, cancelled, rejected) and when initiateWithdrawal throws.
@Kureev Kureev added team-earn pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. labels May 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@Kureev Kureev removed the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label May 28, 2026
@Kureev Kureev self-assigned this May 28, 2026
@Kureev Kureev marked this pull request as ready for review May 28, 2026 21:12
@Kureev Kureev requested a review from a team as a code owner May 28, 2026 21:12
Comment thread app/components/UI/Money/hooks/useMoneyAccount.ts Outdated
expect(toast.labelOptions).toHaveLength(3);
});

it('success title/body is "Transfer complete" / "$50.00 added to Money account." for betweenAccounts', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is asserting against the result.current.MoneyToastOptions.withdraw.success toast but "$50.00 added to Money account." doesn't make sense if we're withdrawing from the Money account to another EVM account in the user's wallet.

We'll want to make sure the copy makes sense for the intent.

Please push back if I'm misunderstanding!

Comment thread locales/languages/en.json Outdated
Comment on lines +6917 to +6918
"withdraw_success_body_between_accounts": "{{amount}} added to Money account.",
"withdraw_success_body_no_amount_between_accounts": "Added to Money account.",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same point here. We'll want to double-check copy. Right now, I don't think this makes sense.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, I took it from https://www.figma.com/design/XKZ8hRqSn2iTiuzmlQLuYQ/Money-account?node-id=6412-27526&m=dev, but didn't pay too much attention to it

…conditionally (MUSD-845)

All transfers out of the Money account share the same Figma toast copy
("Transfer in progress / complete / failed"), so the intent-aware
plumbing added in the previous commit was over-engineered:

  - Drop MoneyAccountWithdrawIntent + the batchId-keyed registry, the
    pre-generated batchId in initiateWithdrawal, and the matching
    get/clear/InitiateWithdrawalOptions exports.
  - Drop WithdrawIntent / WithdrawInProgressParams / WithdrawFailedParams
    and the `destination` param on WithdrawSuccessParams.
  - Rename locale keys to drop the now-redundant `_between_accounts`
    suffix, and remove the unused "moved to {{destination}}" copy.
  - useMoneyTransactionStatus.showFor* maps moneyAccountWithdraw
    straight to the new copy with no intent lookup.

Deposit-intent infrastructure (Convert crypto vs Add mUSD) is unchanged.
@Kureev Kureev changed the title fix(money): correct transfer between-accounts toasts (MUSD-845) fix(money): correct transfer toasts (MUSD-845) May 29, 2026
@Kureev Kureev marked this pull request as draft May 29, 2026 11:06
@Kureev Kureev added the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label May 29, 2026
@Kureev Kureev requested a review from Matt561 May 29, 2026 11:07
@github-actions github-actions Bot added size-S and removed size-M labels May 29, 2026
Comment thread locales/languages/en.json Outdated
…toast (MUSD-845)

The transfer success body now reads "{{amount}} added to {{destination}}."
where destination is the user's currently selected internal account's
display name, resolved at toast time from selectSelectedInternalAccount.
Falls back to "Money account" only when no account name is available.
@github-actions github-actions Bot added size-M and removed size-S labels May 29, 2026
@Kureev Kureev removed the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label May 29, 2026
@codecov-commenter

codecov-commenter commented May 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.70%. Comparing base (f0b6941) to head (05e4e15).
⚠️ Report is 32 commits behind head on main.

Files with missing lines Patch % Lines
...onents/UI/Money/hooks/useMoneyTransactionStatus.ts 86.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #30770      +/-   ##
==========================================
- Coverage   82.70%   82.70%   -0.01%     
==========================================
  Files        5545     5550       +5     
  Lines      142217   142337     +120     
  Branches    32834    32860      +26     
==========================================
+ Hits       117624   117722      +98     
- Misses      16746    16754       +8     
- Partials     7847     7861      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Kureev Kureev marked this pull request as ready for review May 29, 2026 11:35
@Kureev Kureev enabled auto-merge May 29, 2026 11:36
@github-actions github-actions Bot added the risk:medium AI analysis: medium risk label May 29, 2026
Comment thread app/components/UI/Money/hooks/useMoneyTransactionStatus.ts Outdated
…nt (MUSD-845)

Previously, the transfer success toast read the destination name from
selectSelectedInternalAccount at toast time. That breaks two ways:

  1. If the user switches accounts between submit and confirmation, the
     toast names the wrong account.
  2. The fallback "Money account" was misleading on a withdraw — funds
     don't return to the Money account, they leave it.

The destination is now derived from the actual on-chain transfer: decode
the recipient from the nested tokenMethodTransfer calldata, look it up
via getMemoizedInternalAccountByAddress, and use the account's display
name. Falls back to a shortened recipient address when the recipient is
not one of the user's internal accounts (e.g. external send), and to
"your account" only when the nested transfer tx itself is absent.
@Kureev Kureev marked this pull request as draft May 29, 2026 12:33
auto-merge was automatically disabled May 29, 2026 12:33

Pull request was converted to draft

@Kureev Kureev added the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label May 29, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 11ee11b. Configure here.

Comment thread app/components/UI/Money/hooks/useMoneyTransactionStatus.ts Outdated
@Kureev Kureev removed the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label May 29, 2026
@Kureev Kureev marked this pull request as ready for review May 29, 2026 12:57
Replace the hardcoded English fallback "your account" with a
strings('money.toasts.withdraw_fallback_destination') lookup so it gets
translated for non-English users.
@Kureev Kureev marked this pull request as draft May 29, 2026 13:06
@Kureev Kureev added pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. and removed pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. labels May 29, 2026
@Kureev Kureev marked this pull request as ready for review May 29, 2026 13:10
Kureev added 2 commits May 29, 2026 15:15
…D-845)

The success toast rendered "added to ." when the recipient account had a blank name: the name fell through the ?? guard because empty strings are not nullish. Trim the name and branch on whether the recipient is an internal account — name when present, "your account" for a nameless own account, shortened address only for true external recipients.
recipient,
);
if (!account) return renderShortAddress(recipient);
const accountName = account.metadata?.name?.trim();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've named my account, and this value is empty for some reason.

The account name does appear in account picker - which is weird.

Image

This might be related to the getMemoizedInternalAccountByAddress memoization I guess, since I named the account to test this. Does it work for you?

@Matt561 Matt561 Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jwhiles I was able to test successfully after renaming my account.

Withdrawal 1
image

Withdrawal 2
image

…D-845)

The withdraw success toast read the recipient name from internalAccounts[].metadata.name, which stays empty under the multichain-accounts model where the display name lives on the AccountTreeController account group. Renamed accounts therefore fell back to "your account" even though the picker showed the name.

Prefer the account-group name via selectAccountToGroupMap (keyed by account.id), falling back to the internal-account name, then the localized default — matching the Bridge useRecipientDisplayData precedent.
@Kureev Kureev requested a review from Jwhiles June 2, 2026 14:00

@Matt561 Matt561 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kureev can you try renaming your account and withdrawing again? Just want to make sure the account rename case @Jwhiles mentioned is working correctly.

@Kureev Kureev enabled auto-merge June 3, 2026 10:05
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeMoney
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: low
  • AI Confidence: 92%
click to see 🤖 AI reasoning details

E2E Test Selection:
The changes are tightly scoped to the Money feature's withdraw toast notification system:

  1. useMoneyToasts.tsx: Updated string keys for withdraw toasts - in_progress_titlewithdraw_in_progress_title and success_titlewithdraw_success_title. This is a rename to avoid key collisions with deposit toasts.

  2. useMoneyTransactionStatus.ts: Added resolveWithdrawDestination() logic that decodes the ERC-20 transfer recipient from nested transaction data, then resolves it to an account name (group name > account name > shortened address > fallback "your account"). This replaces the hardcoded "Between accounts" string.

  3. locales/languages/en.json: Added new locale strings (withdraw_in_progress_title, withdraw_success_title, withdraw_fallback_destination) and updated body strings to use "added to" instead of "moved to".

  4. Test files: Unit test updates matching the new behavior.

The useMoneyTransactionStatus hook is only imported by MoneyTransactionMonitor - no shared components are affected. No navigation, confirmations, swap flows, or other feature areas are touched. SmokeMoney is the appropriate tag to validate the withdraw flow toast notifications work correctly end-to-end.

Performance Test Selection:
These changes are limited to toast notification logic and locale strings within the Money feature. No UI rendering performance, list components, data loading, or app initialization paths are affected. No performance tests are warranted.

View GitHub Actions results

@Kureev Kureev added this pull request to the merge queue Jun 3, 2026
Merged via the queue into main with commit c24bb5f Jun 3, 2026
187 of 189 checks passed
@Kureev Kureev deleted the kureev/MUSD-845 branch June 3, 2026 11:09
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 3, 2026
@metamaskbotv2 metamaskbotv2 Bot added the release-7.81.0 Issue or pull request that will be included in release 7.81.0 label Jun 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.81.0 Issue or pull request that will be included in release 7.81.0 risk:medium AI analysis: medium risk size-M team-earn

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants