Skip to content

Commit 661e25c

Browse files
committed
fix(money): make amount-update error toast persist until dismissed
1 parent c029f4e commit 661e25c

9 files changed

Lines changed: 131 additions & 101 deletions

File tree

app/components/UI/Money/utils/moneyAccountTransactions.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ describe('moneyAccountTransactions', () => {
376376

377377
await expect(
378378
updateMoneyAccountDepositTokenAmount(mockTransactionMeta, '1.0'),
379-
).rejects.toThrow('RPC connection refused');
379+
).rejects.toThrow('Failed to preview deposit shares');
380380
});
381381
});
382382

@@ -792,7 +792,7 @@ describe('moneyAccountTransactions', () => {
792792

793793
await expect(
794794
getMoneyAccountDepositTransactionsData(MOCK_CHAIN_ID, '1.0'),
795-
).rejects.toThrow('RPC timeout');
795+
).rejects.toThrow('Failed to preview deposit shares');
796796
});
797797
});
798798

app/components/UI/Money/utils/moneyAccountTransactions.ts

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,17 @@ async function getExpectedDepositShares({
8787
provider: ethers.providers.Provider;
8888
}): Promise<bigint> {
8989
const lensContract = new ethers.Contract(lensAddress, LENS_ABI, provider);
90-
const shares = await lensContract.previewDeposit(
91-
musdAddress,
92-
amount.toString(),
93-
boringVault,
94-
accountantAddress,
95-
);
96-
return BigInt(shares.toString());
90+
try {
91+
const shares = await lensContract.previewDeposit(
92+
musdAddress,
93+
amount.toString(),
94+
boringVault,
95+
accountantAddress,
96+
);
97+
return BigInt(shares.toString());
98+
} catch {
99+
throw new Error('Failed to preview deposit shares');
100+
}
97101
}
98102

99103
function buildApproveData(boringVault: string, amount: bigint): Hex {
@@ -391,8 +395,12 @@ async function getVaultRate({
391395
ACCOUNTANT_ABI,
392396
provider,
393397
);
394-
const rate = await accountant.getRate();
395-
return BigInt(rate.toString());
398+
try {
399+
const rate = await accountant.getRate();
400+
return BigInt(rate.toString());
401+
} catch {
402+
throw new Error('Failed to fetch vault rate');
403+
}
396404
}
397405

398406
const SHARE_DECIMALS_SCALAR = BigInt(1_000_000);

app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.test.tsx

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ import { useTokenFiatRates } from '../../../hooks/tokens/useTokenFiatRates';
4444
import { useTransactionPayWithdraw } from '../../../hooks/pay/useTransactionPayWithdraw';
4545
import { useTransactionAccountOverride } from '../../../hooks/transactions/useTransactionAccountOverride';
4646
import { useMoneyNoFeeTokens } from '../../../hooks/pay/useMoneyNoFeeTokens';
47-
import Logger from '../../../../../../util/Logger';
47+
4848
import useClearConfirmationOnBackSwipe from '../../../hooks/ui/useClearConfirmationOnBackSwipe';
4949

5050
jest.mock('../../../hooks/ui/useClearConfirmationOnBackSwipe');
@@ -78,7 +78,7 @@ jest.mock('../../../hooks/pay/useTransactionPayWithdraw', () => ({
7878
jest.mock('../../../hooks/transactions/useTransactionAccountOverride');
7979
jest.mock('../../../hooks/pay/useMoneyNoFeeTokens');
8080
jest.mock('../../../../../../util/transaction-controller', () => ({}));
81-
jest.mock('../../../../../../util/Logger');
81+
8282
jest.mock('../../../../../../core/Engine', () => ({
8383
context: {
8484
TransactionPayController: {
@@ -593,9 +593,6 @@ describe('CustomAmountInfo', () => {
593593
const error = new Error('update failed');
594594
const updateTokenAmountMock = jest.fn().mockRejectedValue(error);
595595
const mockOnAmountSubmit = jest.fn();
596-
const loggerErrorMock = jest.mocked(Logger.error);
597-
loggerErrorMock.mockClear();
598-
599596
useTransactionCustomAmountMock.mockReturnValue({
600597
amountFiat: '123.45',
601598
amountHuman: '0',
@@ -626,10 +623,6 @@ describe('CustomAmountInfo', () => {
626623
);
627624
// Keyboard stays open: Done button still present
628625
expect(queryByText(strings('confirm.edit_amount_done'))).toBeOnTheScreen();
629-
expect(loggerErrorMock).toHaveBeenCalledWith(
630-
error,
631-
expect.stringContaining('Failed to apply custom amount on Done press'),
632-
);
633626
});
634627

635628
it('renders PayAccountSelector when supportAccountSelection is true', () => {
@@ -957,7 +950,6 @@ describe('CustomAmountInfo', () => {
957950
it('does not fire RAMPS_ORDER_PROPOSED and shows toast when applying the amount throws on Done', async () => {
958951
setMoneyFlow();
959952
const error = new Error('update failed');
960-
const loggerErrorMock = jest.mocked(Logger.error);
961953
useTransactionCustomAmountMock.mockReturnValue({
962954
...useTransactionCustomAmountMock(),
963955
updateTokenAmount: jest.fn().mockRejectedValue(error),
@@ -974,10 +966,6 @@ describe('CustomAmountInfo', () => {
974966
// The Done handler's catch shows a toast and returns early without committing.
975967
expect(emittedPayloadFor('RAMPS_ORDER_PROPOSED')).toBeUndefined();
976968
expect(mockShowToast).toHaveBeenCalledTimes(1);
977-
expect(loggerErrorMock).toHaveBeenCalledWith(
978-
error,
979-
expect.stringContaining('Failed to apply custom amount on Done press'),
980-
);
981969
});
982970

983971
// Regression guard for FIX 2 (no double emission). Renders the REAL money

app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.tsx

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,9 @@ import { useAccountNoFundsAlert } from '../../../hooks/alerts/useAccountNoFundsA
7171
import { useConfirmActions } from '../../../hooks/useConfirmActions';
7272
import EngineService from '../../../../../../core/EngineService';
7373
import Engine from '../../../../../../core/Engine';
74-
import Logger from '../../../../../../util/Logger';
75-
import { getTransactionUpdateErrorToastOptions } from '../../../../../../util/confirmation/transactions';
74+
import { getAmountUpdateErrorToastOptions } from '../../../../../../util/confirmation/transactions';
7675
import { ToastContext } from '../../../../../../component-library/components/Toast';
76+
import { prefixError } from '../../../../../../util/transactions/error-prefix';
7777
import { ConfirmationFooterSelectorIDs } from '../../../ConfirmationView.testIds';
7878
import { useTransactionPayToken } from '../../../hooks/pay/useTransactionPayToken';
7979
import { useMoneyNoFeeTokens } from '../../../hooks/pay/useMoneyNoFeeTokens';
@@ -86,6 +86,8 @@ import { CustomAmountInfoTestIds } from './custom-amount-info.testIds';
8686
import { useConfirmationContext } from '../../../context/confirmation-context';
8787
import { useFiatFunnelMetricsAdapter } from '../../../../../UI/Ramp/hooks/useFiatFunnelMetricsAdapter';
8888

89+
const AMOUNT_UPDATE_ERROR_PREFIX = 'MetaMask Pay: Amount Update: ';
90+
8991
export interface CustomAmountInfoProps {
9092
autoSelectFiatPayment?: boolean;
9193
children?: ReactNode;
@@ -208,39 +210,24 @@ export const CustomAmountInfo: React.FC<CustomAmountInfoProps> = memo(
208210

209211
const handleDone = useCallback(async () => {
210212
try {
213+
await updateTokenAmount();
211214
if (selectedFiatPaymentMethodId && transactionId) {
212-
if (isMoneyAccountDeposit) {
213-
// Update calldata first; only commit the fiat payment amount after
214-
// success so Pay config does not reflect an amount whose calldata
215-
// update failed.
216-
await updateTokenAmount();
217-
Engine.context.TransactionPayController.updateFiatPayment({
218-
transactionId,
219-
callback: (fp) => {
220-
fp.amountFiat = amountFiat;
221-
},
222-
});
223-
} else {
224-
Engine.context.TransactionPayController.updateFiatPayment({
225-
transactionId,
226-
callback: (fp) => {
227-
fp.amountFiat = amountFiat;
228-
},
229-
});
230-
}
231-
} else {
232-
await updateTokenAmount();
215+
Engine.context.TransactionPayController.updateFiatPayment({
216+
transactionId,
217+
callback: (fp) => {
218+
fp.amountFiat = amountFiat;
219+
},
220+
});
233221
}
234222
// Amount committed (pre-quote) funnel event; only fires once the amount
235223
// has been successfully applied above (no-op for non-money flows).
236224
trackAmountCommitted();
237225
} catch (error) {
238-
Logger.error(
239-
error as Error,
240-
'Failed to apply custom amount on Done press',
241-
);
226+
const prefixed = prefixError(error, AMOUNT_UPDATE_ERROR_PREFIX);
242227
toastRef?.current?.showToast(
243-
getTransactionUpdateErrorToastOptions(error),
228+
getAmountUpdateErrorToastOptions(prefixed, () =>
229+
toastRef?.current?.closeToast(),
230+
),
244231
);
245232
// Keep keyboard visible so the user can retry; do not advance the flow.
246233
return;
@@ -250,7 +237,6 @@ export const CustomAmountInfo: React.FC<CustomAmountInfoProps> = memo(
250237
onAmountSubmit?.();
251238
}, [
252239
amountFiat,
253-
isMoneyAccountDeposit,
254240
onAmountSubmit,
255241
selectedFiatPaymentMethodId,
256242
toastRef,

app/components/Views/confirmations/hooks/pay/useUpdateTransactionPayAmount.test.ts

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
} from '@metamask/transaction-controller';
1919
import { useUpdateTokenAmount } from '../transactions/useUpdateTokenAmount';
2020
import { useTransactionAccountOverride } from '../transactions/useTransactionAccountOverride';
21-
import Logger from '../../../../../util/Logger';
21+
2222
import { useTransactionPayRequiredTokens } from './useTransactionPayData';
2323
import { TransactionPayRequiredToken } from '@metamask/transaction-pay-controller';
2424
import { Hex } from '@metamask/utils';
@@ -27,7 +27,7 @@ jest.mock('../../../../../util/transaction-controller');
2727
jest.mock('../../../../UI/Money/utils/moneyAccountTransactions');
2828
jest.mock('../transactions/useUpdateTokenAmount');
2929
jest.mock('../transactions/useTransactionAccountOverride');
30-
jest.mock('../../../../../util/Logger');
30+
3131
jest.mock('./useTransactionPayData');
3232

3333
const moneyAccountDepositMeta: Partial<TransactionMeta> = {
@@ -81,8 +81,6 @@ describe('useUpdateTransactionPayAmount', () => {
8181
useTransactionPayRequiredTokens,
8282
);
8383
const updateTokenAmountMock = jest.fn();
84-
const loggerErrorMock = jest.mocked(Logger.error);
85-
8684
beforeEach(() => {
8785
jest.resetAllMocks();
8886
updateAtomicBatchDataMock.mockResolvedValue('0x0');
@@ -204,10 +202,6 @@ describe('useUpdateTransactionPayAmount', () => {
204202
).rejects.toThrow('rpc failure');
205203

206204
expect(updateAtomicBatchDataMock).not.toHaveBeenCalled();
207-
expect(loggerErrorMock).toHaveBeenCalledWith(
208-
error,
209-
expect.stringContaining('Failed to prepare Money Account deposit'),
210-
);
211205
});
212206

213207
it('calls updateAtomicBatchData for each update returned from updateMoneyAccountWithdrawTokenAmount', async () => {
@@ -285,10 +279,6 @@ describe('useUpdateTransactionPayAmount', () => {
285279
).rejects.toThrow('withdraw rpc failure');
286280

287281
expect(updateAtomicBatchDataMock).not.toHaveBeenCalled();
288-
expect(loggerErrorMock).toHaveBeenCalledWith(
289-
error,
290-
expect.stringContaining('Failed to prepare Money Account withdraw'),
291-
);
292282
});
293283

294284
describe('syncMoneyAccountDepositRequiredAssets', () => {
@@ -414,11 +404,6 @@ describe('useUpdateTransactionPayAmount', () => {
414404
await expect(
415405
result.current.updateTransactionPayAmount('1'),
416406
).rejects.toThrow('updateTransaction failed');
417-
418-
expect(loggerErrorMock).toHaveBeenCalledWith(
419-
error,
420-
'Failed to sync Money Account deposit requiredAssets amount',
421-
);
422407
});
423408

424409
it('still applies money account deposit updates after syncing requiredAssets', async () => {

app/components/Views/confirmations/hooks/pay/useUpdateTransactionPayAmount.ts

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@ import {
1919
} from '../../../../UI/Money/utils/moneyAccountTransactions';
2020
import { UpdateTransactionPayAmountCall } from '../../types/transactions';
2121
import { hasTransactionType } from '../../utils/transaction';
22-
import Logger from '../../../../../util/Logger';
22+
import { prefixError } from '../../../../../util/transactions/error-prefix';
2323
import { useTransactionPayRequiredTokens } from './useTransactionPayData';
2424

25+
const DEPOSIT_ERROR_PREFIX = 'Money Account Deposit: ';
26+
const WITHDRAW_ERROR_PREFIX = 'Money Account Withdrawal: ';
27+
2528
type MoneyAccountAmountUpdater = (
2629
transactionMeta: TransactionMeta,
2730
amountHuman: string,
@@ -38,7 +41,7 @@ export function useUpdateTransactionPayAmount() {
3841
async (
3942
amountHuman: string,
4043
updater: MoneyAccountAmountUpdater,
41-
preparationErrorMessage: string,
44+
errorPrefix: string,
4245
recipientOverride?: Hex,
4346
) => {
4447
if (!transactionMeta) {
@@ -53,19 +56,22 @@ export function useUpdateTransactionPayAmount() {
5356
recipientOverride,
5457
);
5558
} catch (error) {
56-
Logger.error(error as Error, preparationErrorMessage);
57-
throw error;
59+
throw prefixError(error, errorPrefix);
5860
}
5961

60-
await Promise.all(
61-
updates.map(({ nestedTransactionIndex, transactionData }) =>
62-
updateAtomicBatchData({
63-
transactionId: transactionMeta.id,
64-
transactionIndex: nestedTransactionIndex,
65-
transactionData,
66-
}),
67-
),
68-
);
62+
try {
63+
await Promise.all(
64+
updates.map(({ nestedTransactionIndex, transactionData }) =>
65+
updateAtomicBatchData({
66+
transactionId: transactionMeta.id,
67+
transactionIndex: nestedTransactionIndex,
68+
transactionData,
69+
}),
70+
),
71+
);
72+
} catch (error) {
73+
throw prefixError(error, errorPrefix);
74+
}
6975
},
7076
[transactionMeta],
7177
);
@@ -89,7 +95,7 @@ export function useUpdateTransactionPayAmount() {
8995
await applyMoneyAccountAmountUpdates(
9096
amountHuman,
9197
updateMoneyAccountDepositTokenAmount,
92-
'Failed to prepare Money Account deposit amount update',
98+
DEPOSIT_ERROR_PREFIX,
9399
);
94100
return;
95101
}
@@ -102,7 +108,7 @@ export function useUpdateTransactionPayAmount() {
102108
await applyMoneyAccountAmountUpdates(
103109
amountHuman,
104110
updateMoneyAccountWithdrawTokenAmount,
105-
'Failed to prepare Money Account withdraw amount update',
111+
WITHDRAW_ERROR_PREFIX,
106112
accountOverride,
107113
);
108114
return;
@@ -147,10 +153,6 @@ function syncMoneyAccountDepositRequiredAssets(
147153
'Money Account deposit: sync requiredAssets amount',
148154
);
149155
} catch (error) {
150-
Logger.error(
151-
error as Error,
152-
'Failed to sync Money Account deposit requiredAssets amount',
153-
);
154-
throw error;
156+
throw prefixError(error, DEPOSIT_ERROR_PREFIX);
155157
}
156158
}

app/util/confirmation/transactions.test.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
import { strings } from '../../../locales/i18n';
1111
import {
1212
buildTransactionParams,
13+
getAmountUpdateErrorToastOptions,
1314
getTransactionUpdateErrorToastOptions,
1415
resolveTransactionUpdateErrorMessage,
1516
} from './transactions';
@@ -180,7 +181,6 @@ describe('Confirmation Transactions Utils', () => {
180181
},
181182
],
182183
descriptionOptions: { description: 'boom' },
183-
hasNoTimeout: false,
184184
}),
185185
);
186186
});
@@ -204,4 +204,27 @@ describe('Confirmation Transactions Utils', () => {
204204
).toBeUndefined();
205205
});
206206
});
207+
208+
describe('getAmountUpdateErrorToastOptions', () => {
209+
const onClose = jest.fn();
210+
211+
it('returns a persistent toast with a close button', () => {
212+
expect(
213+
getAmountUpdateErrorToastOptions(new Error('boom'), onClose),
214+
).toEqual(
215+
expect.objectContaining({
216+
variant: ToastVariants.Icon,
217+
descriptionOptions: { description: 'boom' },
218+
hasNoTimeout: true,
219+
closeButtonOptions: expect.objectContaining({ onPress: onClose }),
220+
}),
221+
);
222+
});
223+
224+
it('omits description when no message is available', () => {
225+
expect(
226+
getAmountUpdateErrorToastOptions(undefined, onClose).descriptionOptions,
227+
).toBeUndefined();
228+
});
229+
});
207230
});

0 commit comments

Comments
 (0)