Skip to content

Commit 4aedbb4

Browse files
authored
fix: show transaction fee, est. time, and total rows for same-chain payments (#28652)
## **Description** PR #28588 added a `hasQuoteResults` guard to hide fee/time/total rows when bridge quotes fail (preventing incorrect data from displaying). However, this also hid the rows for same-chain payments (USDC.E on Polygon → Predict deposit on Polygon) where no bridge quote is needed but valid fee data still exists. This PR refines the visibility logic so rows are shown for same-chain payments while still being hidden when quotes fail with a blocking alert. It also removes the per-row `!hasQuotes` guards in `BridgeFeeRow` and `ReceiveRow` since the parent component now handles the "no quotes" error case. **Changes:** - `custom-amount-info.tsx`: `hasQuoteResults` now considers `!hasSourceAmount && !hasBlockingAlerts` — shows rows for same-chain, hides on quote failure - `bridge-fee-row.tsx`: Removed `!hasQuotes` guard from fee calculation — totals are trusted when the parent decides to render - `bridge-time-row.tsx`: Added `isSameChain` to `showEstimate` — displays "< 10 sec" for same-chain payments - `receive-row.tsx`: Removed `!hasQuotes` guard and unused `useTransactionPayQuotes` import ## **Changelog** CHANGELOG entry: Fixed transaction fee, estimated time, and total rows not appearing for same-chain Predict deposits/withdrawals ## **Related issues** Fixes: ## **Manual testing steps** 1. Open Add Prediction funds with USDC.E on Polygon 2. Enter an amount and press Done 3. Verify Transaction fee, Est. time, and Total rows appear with values 4. Open Predict Withdraw, select USDC.E on Polygon as receive token 5. Enter a withdrawal amount and press Done 6. Verify Transaction fee, Est. time, and You'll receive rows appear with values 7. Trigger a "no quotes" scenario (e.g. select a token/amount with no available route) 8. Verify fee/time/total rows are hidden and the button shows "No quotes" disabled ## **Screenshots/Recordings** ### **Before** N/A ### **After** N/A ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Adjusts confirmation UI gating for fee/time/total rows based on quotes, source amount, and alerts; mistakes could hide or show incorrect cost details during transaction confirmation. > > **Overview** > Fixes confirmation amount review so **fee/estimated time/total (or receive) rows render for same-chain payments even when no bridge quotes exist**, while still hiding those rows when a blocking `AlertKeys.NoPayTokenQuotes` alert is present. > > Moves the “no quotes” error handling to the parent (`CustomAmountInfo`) via `showPaymentDetails`, and simplifies `BridgeFeeRow` and `ReceiveRow` to compute from `totals` even without quotes (with `BridgeFeeRow` only showing its tooltip when quotes exist). `BridgeTimeRow` now also shows an estimate for same-chain payments without quotes. Tests are updated/added to cover these scenarios. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 39e0f79. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Signed-off-by: dan437 <80175477+dan437@users.noreply.github.com>
1 parent 9f195d0 commit 4aedbb4

File tree

8 files changed

+110
-31
lines changed

8 files changed

+110
-31
lines changed

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

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ import { otherControllersMock } from '../../../__mocks__/controllers/other-contr
1212
import { useTransactionPayToken } from '../../../hooks/pay/useTransactionPayToken';
1313
import { useTransactionCustomAmount } from '../../../hooks/transactions/useTransactionCustomAmount';
1414
import { useConfirmationContext } from '../../../context/confirmation-context';
15-
import { Alert } from '../../../types/alerts';
15+
import { Alert, Severity } from '../../../types/alerts';
16+
import { AlertKeys } from '../../../constants/alerts';
1617
import {
1718
AlertsContextParams,
1819
useAlerts,
@@ -24,7 +25,9 @@ import { AssetType } from '../../../types/token';
2425
import {
2526
useTransactionPayRequiredTokens,
2627
useIsTransactionPayLoading,
28+
useTransactionPayQuotes,
2729
} from '../../../hooks/pay/useTransactionPayData';
30+
import { useTransactionPayHasSourceAmount } from '../../../hooks/pay/useTransactionPayHasSourceAmount';
2831
import { strings } from '../../../../../../../locales/i18n';
2932
import { Hex } from '@metamask/utils';
3033
import { TransactionPayRequiredToken } from '@metamask/transaction-pay-controller';
@@ -47,6 +50,8 @@ jest.mock('../../../hooks/pay/useTransactionPayMetrics');
4750
jest.mock('../../../hooks/send/useAccountTokens');
4851
jest.mock('../../../hooks/pay/useTransactionPayAvailableTokens');
4952
jest.mock('../../../hooks/pay/useTransactionPayData');
53+
jest.mock('../../../hooks/pay/useTransactionPayHasSourceAmount');
54+
jest.mock('../../../hooks/pay/useTransactionPaySelectedFiatPaymentMethod');
5055
jest.mock('../../../hooks/transactions/useTransactionConfirm');
5156
jest.mock('../../../hooks/transactions/useTransactionMetadataRequest');
5257
jest.mock('../../../hooks/pay/useTransactionPayWithdraw', () => ({
@@ -184,6 +189,12 @@ describe('CustomAmountInfo', () => {
184189
useIsTransactionPayLoading,
185190
);
186191

192+
const useTransactionPayQuotesMock = jest.mocked(useTransactionPayQuotes);
193+
194+
const useTransactionPayHasSourceAmountMock = jest.mocked(
195+
useTransactionPayHasSourceAmount,
196+
);
197+
187198
const useTransactionCustomAmountAlertsMock = jest.mocked(
188199
useTransactionCustomAmountAlerts,
189200
);
@@ -255,6 +266,8 @@ describe('CustomAmountInfo', () => {
255266
useTransactionPayRequiredTokensMock.mockReturnValue([]);
256267
useTransactionConfirmMock.mockReturnValue({} as never);
257268
useIsTransactionPayLoadingMock.mockReturnValue(false);
269+
useTransactionPayQuotesMock.mockReturnValue([]);
270+
useTransactionPayHasSourceAmountMock.mockReturnValue(false);
258271
useTokenFiatRatesMock.mockReturnValue([1, 1]);
259272
useTransactionMetadataRequestMock.mockReturnValue({
260273
type: TransactionType.contractInteraction,
@@ -537,6 +550,57 @@ describe('CustomAmountInfo', () => {
537550
from: '0xTestRecipient',
538551
});
539552
});
553+
554+
describe('showPaymentDetails', () => {
555+
async function pressDone(
556+
getByText: ReturnType<typeof render>['getByText'],
557+
) {
558+
await act(async () => {
559+
fireEvent.press(getByText(strings('confirm.edit_amount_done')));
560+
});
561+
}
562+
563+
it('shows fee rows for same-chain payment without quotes', async () => {
564+
useTransactionPayHasSourceAmountMock.mockReturnValue(false);
565+
useTransactionPayQuotesMock.mockReturnValue([]);
566+
567+
const { getByText, getByTestId } = render();
568+
await pressDone(getByText);
569+
570+
expect(getByTestId('bridge-fee-row')).toBeOnTheScreen();
571+
});
572+
573+
it('hides fee rows when no-quotes alert is present', async () => {
574+
useTransactionPayHasSourceAmountMock.mockReturnValue(false);
575+
useTransactionPayQuotesMock.mockReturnValue([]);
576+
useAlertsMock.mockReturnValue({
577+
alerts: [
578+
{
579+
key: AlertKeys.NoPayTokenQuotes,
580+
severity: Severity.Danger,
581+
isBlocking: true,
582+
},
583+
] as Alert[],
584+
generalAlerts: [] as Alert[],
585+
fieldAlerts: [] as Alert[],
586+
} as AlertsContextParams);
587+
588+
const { getByText, queryByTestId } = render();
589+
await pressDone(getByText);
590+
591+
expect(queryByTestId('bridge-fee-row')).toBeNull();
592+
});
593+
594+
it('shows fee rows when quotes exist regardless of source amount', async () => {
595+
useTransactionPayHasSourceAmountMock.mockReturnValue(true);
596+
useTransactionPayQuotesMock.mockReturnValue([{} as never]);
597+
598+
const { getByText, getByTestId } = render();
599+
await pressDone(getByText);
600+
601+
expect(getByTestId('bridge-fee-row')).toBeOnTheScreen();
602+
});
603+
});
540604
});
541605

542606
describe('CustomAmountInfoSkeleton', () => {

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ import {
5858
TextVariant as TextVariantComponent,
5959
} from '@metamask/design-system-react-native';
6060
import { useAlerts } from '../../../context/alert-system-context';
61+
import { AlertKeys } from '../../../constants/alerts';
6162
import { useTransactionConfirm } from '../../../hooks/transactions/useTransactionConfirm';
6263
import EngineService from '../../../../../../core/EngineService';
6364
import Engine from '../../../../../../core/Engine';
@@ -169,7 +170,15 @@ export const CustomAmountInfo: React.FC<CustomAmountInfoProps> = memo(
169170
const isResultReady = useIsResultReady({ isKeyboardVisible });
170171
const quotes = useTransactionPayQuotes();
171172
const isQuotesLoading = useIsTransactionPayLoading();
172-
const hasQuoteResults = isQuotesLoading || Boolean(quotes?.length);
173+
const hasSourceAmount = useTransactionPayHasSourceAmount();
174+
const { alerts } = useAlerts();
175+
const hasNoQuotesAlert = alerts.some(
176+
(a) => a.key === AlertKeys.NoPayTokenQuotes,
177+
);
178+
const showPaymentDetails =
179+
isQuotesLoading ||
180+
Boolean(quotes?.length) ||
181+
(!hasSourceAmount && !hasNoQuotesAlert);
173182

174183
const {
175184
amountFiat,
@@ -272,7 +281,7 @@ export const CustomAmountInfo: React.FC<CustomAmountInfoProps> = memo(
272281
{!overrideContent && disablePay !== true && hasTokens && (
273282
<PayWithRow />
274283
)}
275-
{hasQuoteResults && (
284+
{showPaymentDetails && (
276285
<>
277286
<BridgeFeeRow />
278287
<BridgeTimeRow />

app/components/Views/confirmations/components/rows/bridge-fee-row/bridge-fee-row.test.tsx

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,16 @@ describe('BridgeFeeRow', () => {
106106
expect(queryByTestId('metamask-fee-row-skeleton')).toBeNull();
107107
});
108108

109-
it('renders empty fee when there are no quotes', () => {
109+
it('renders fee from totals when there are no quotes', () => {
110110
useTransactionPayQuotesMock.mockReturnValue([]);
111-
const { getByTestId } = render();
112-
expect(getByTestId('bridge-fee-row')).toBeDefined();
111+
const { getByText } = render();
112+
expect(getByText('$1.23')).toBeOnTheScreen();
113+
});
114+
115+
it('does not render tooltip when there are no quotes', () => {
116+
useTransactionPayQuotesMock.mockReturnValue([]);
117+
const { queryByTestId } = render();
118+
expect(queryByTestId('info-row-tooltip-open-btn')).toBeNull();
113119
});
114120

115121
it('includes metamask fee in transaction fee total', () => {

app/components/Views/confirmations/components/rows/bridge-fee-row/bridge-fee-row.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ function TransactionFeeRow({
6767
const hasQuotes = Boolean(quotes?.length);
6868

6969
const feeTotalUsd = useMemo(() => {
70-
if (!totals?.fees || !hasQuotes) return '';
70+
if (!totals?.fees) return '';
7171

7272
const metaMask = totals.fees.metaMask.usd ?? 0;
7373
const provider = totals.fees.provider.usd;
@@ -80,7 +80,7 @@ function TransactionFeeRow({
8080
.plus(sourceNetwork)
8181
.plus(targetNetwork),
8282
);
83-
}, [totals, formatFiat, hasQuotes]);
83+
}, [totals, formatFiat]);
8484

8585
if (isLoading) return <InfoRowSkeleton testId="bridge-fee-row-skeleton" />;
8686

app/components/Views/confirmations/components/rows/bridge-time-row/bridge-time-row.test.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,20 @@ describe('BridgeTimeRow', () => {
102102
expect(getByText('< 10 sec')).toBeDefined();
103103
});
104104

105+
it('renders estimated time for same-chain payment without quotes', () => {
106+
useTransactionPayQuotesMock.mockReturnValue([]);
107+
useTransactionPayTotalsMock.mockReturnValue({
108+
estimatedDuration: 0,
109+
} as TransactionPayTotals);
110+
useTransactionPayTokenMock.mockReturnValue({
111+
payToken: { chainId: '0x1' as Hex },
112+
} as ReturnType<typeof useTransactionPayToken>);
113+
114+
const { getByText } = render();
115+
116+
expect(getByText('< 10 sec')).toBeOnTheScreen();
117+
});
118+
105119
it('renders skeleton if quotes loading', async () => {
106120
useIsTransactionPayLoadingMock.mockReturnValue(true);
107121

app/components/Views/confirmations/components/rows/bridge-time-row/bridge-time-row.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,11 @@ export function BridgeTimeRow() {
3232
const selectedFiatPaymentMethod =
3333
useTransactionPaySelectedFiatPaymentMethod();
3434

35+
const isSameChain = payToken?.chainId != null && payToken.chainId === chainId;
36+
3537
const showEstimate =
3638
!hasTransactionType(transactionMetadata, HIDE_TYPES) &&
37-
(isLoading || Boolean(quotes?.length)) &&
39+
(isLoading || Boolean(quotes?.length) || isSameChain) &&
3840
!selectedFiatPaymentMethod;
3941

4042
if (!showEstimate) {
@@ -44,8 +46,6 @@ export function BridgeTimeRow() {
4446
if (isLoading) {
4547
return <InfoRowSkeleton testId="bridge-time-row-skeleton" />;
4648
}
47-
48-
const isSameChain = payToken?.chainId === chainId;
4949
const formattedSeconds = formatSeconds(estimatedDuration ?? 0, isSameChain);
5050

5151
return (

app/components/Views/confirmations/components/rows/receive-row/receive-row.test.tsx

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,9 @@ import { simpleSendTransactionControllerMock } from '../../../__mocks__/controll
66
import { transactionApprovalControllerMock } from '../../../__mocks__/controllers/approval-controller-mock';
77
import {
88
useIsTransactionPayLoading,
9-
useTransactionPayQuotes,
109
useTransactionPayTotals,
1110
} from '../../../hooks/pay/useTransactionPayData';
12-
import {
13-
TransactionPayQuote,
14-
TransactionPayTotals,
15-
} from '@metamask/transaction-pay-controller';
16-
import { Json } from '@metamask/utils';
11+
import { TransactionPayTotals } from '@metamask/transaction-pay-controller';
1712
import { otherControllersMock } from '../../../__mocks__/controllers/other-controllers-mock';
1813

1914
jest.mock('../../../hooks/pay/useTransactionPayData');
@@ -40,7 +35,6 @@ describe('ReceiveRow', () => {
4035
const useIsTransactionPayLoadingMock = jest.mocked(
4136
useIsTransactionPayLoading,
4237
);
43-
const useTransactionPayQuotesMock = jest.mocked(useTransactionPayQuotes);
4438

4539
beforeEach(() => {
4640
jest.clearAllMocks();
@@ -54,9 +48,6 @@ describe('ReceiveRow', () => {
5448
} as unknown as TransactionPayTotals);
5549

5650
useIsTransactionPayLoadingMock.mockReturnValue(false);
57-
useTransactionPayQuotesMock.mockReturnValue([
58-
{} as TransactionPayQuote<Json>,
59-
]);
6051
});
6152

6253
it('renders the receive amount (input minus all fees)', () => {
@@ -97,10 +88,8 @@ describe('ReceiveRow', () => {
9788
expect(queryByText(EXPECTED_RECEIVE_MOCK)).toBeNull();
9889
});
9990

100-
it('renders empty receive amount when there are no quotes', () => {
101-
useTransactionPayQuotesMock.mockReturnValue([]);
102-
103-
const { getByTestId } = render();
104-
expect(getByTestId('receive-row')).toBeDefined();
91+
it('renders receive amount from totals without quotes', () => {
92+
const { getByText } = render();
93+
expect(getByText(EXPECTED_RECEIVE_MOCK)).toBeOnTheScreen();
10594
});
10695
});

app/components/Views/confirmations/components/rows/receive-row/receive-row.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { View } from 'react-native';
99
import { BigNumber } from 'bignumber.js';
1010
import {
1111
useIsTransactionPayLoading,
12-
useTransactionPayQuotes,
1312
useTransactionPayTotals,
1413
} from '../../../hooks/pay/useTransactionPayData';
1514
import { InfoRowSkeleton, InfoRowVariant } from '../../UI/info-row/info-row';
@@ -31,11 +30,9 @@ export function ReceiveRow({ inputAmountUsd }: ReceiveRowProps) {
3130
const formatFiat = useFiatFormatter({ currency: 'usd' });
3231
const isLoading = useIsTransactionPayLoading();
3332
const totals = useTransactionPayTotals();
34-
const quotes = useTransactionPayQuotes();
35-
const hasQuotes = Boolean(quotes?.length);
3633

3734
const receiveUsd = useMemo(() => {
38-
if (!totals || inputAmountUsd == null || !hasQuotes) return '';
35+
if (!totals || inputAmountUsd == null) return '';
3936

4037
const inputUsd = new BigNumber(inputAmountUsd);
4138
const providerFee = new BigNumber(totals.fees?.provider?.usd ?? 0);
@@ -53,7 +50,7 @@ export function ReceiveRow({ inputAmountUsd }: ReceiveRowProps) {
5350
.plus(metaMaskFee);
5451
const youReceive = inputUsd.minus(totalFees);
5552
return formatFiat(youReceive.isPositive() ? youReceive : new BigNumber(0));
56-
}, [totals, formatFiat, inputAmountUsd, hasQuotes]);
53+
}, [totals, formatFiat, inputAmountUsd]);
5754

5855
if (isLoading) {
5956
return <InfoRowSkeleton testId="receive-row-skeleton" />;

0 commit comments

Comments
 (0)