Skip to content

Commit e7a2296

Browse files
authored
feat(transaction-pay-controller): store no-op quotes for direct routes (MetaMask#9484)
## Explanation `TransactionPayController` stores an empty quotes array both when no conversion is needed and when a quote could not be fetched, so at publish time the two cases cannot be told apart. This PR makes the distinction explicit: - Store a no-op quote (`TransactionPayStrategy.None`) when a payment token is selected but the route needs no conversion. The no-op quote is built inside `getQuotes` when there are no quote requests, and is skipped when `isQuoteRequired` is set, since an empty request list then means source amounts could not be calculated. - No-op quotes are not routable: the quote refresher skips them, they are excluded from totals and the external-sign flag, and `TransactionPayPublishHook` filters them out before selecting a strategy. - Clients can rely on `quotes.length > 0` whenever MetaMask Pay is engaged, and enforce quote presence at publish time on their side. ## References - Mobile adoption: MetaMask/metamask-mobile#33194 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [x] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes quote shape and publish-time behavior (breaking for quote consumers) in the transaction pay path, but logic is guarded with filters and broad tests rather than touching auth or funds directly. > > **Overview** > Direct MetaMask Pay routes (payment token selected, no conversion) now persist a **no-op quote** with strategy `none` instead of an empty `quotes` array, so **“no conversion needed”** is distinguishable from **“quote still missing.”** > > `getQuotes` builds that marker via `buildNoOpQuote` when there are no quote requests and `isQuoteRequired` is false; fiat-only empty cases and quote-required flows still clear or omit it. **`TransactionPayStrategy.None` is not routable**—`isTransactionPayStrategy` excludes it, the publish hook skips execution, quote refresh/polling ignores only-no-op state, and fee totals / `hasQuotes` sync use **executable** quotes only. > > **Breaking:** downstream clients that treat any entry in `quotes` as a real pay quote must ignore or handle `strategy: 'none'`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 30646eb. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent a9b5174 commit e7a2296

10 files changed

Lines changed: 359 additions & 13 deletions

File tree

packages/transaction-pay-controller/CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
12+
- Store a no-op quote (`TransactionPayStrategy.None`) when a payment token is selected but the route needs no conversion, so clients can tell "no conversion needed" apart from "quote missing" ([#9484](https://github.com/MetaMask/core/pull/9484))
13+
- Add `None` value to `TransactionPayStrategy` enum ([#9484](https://github.com/MetaMask/core/pull/9484))
14+
15+
### Changed
16+
17+
- **BREAKING:** Quotes stored in `transactionData` can now contain a single no-op quote with the `none` strategy; clients that read quotes for display, metrics, or publish validation must ignore or handle no-op quotes ([#9484](https://github.com/MetaMask/core/pull/9484))
18+
1019
## [24.1.0]
1120

1221
### Added

packages/transaction-pay-controller/src/constants.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,24 @@ export enum PaymentOverride {
7272
export enum TransactionPayStrategy {
7373
Across = 'across',
7474
Fiat = 'fiat',
75+
/**
76+
* Internal marker for no-op quotes generated when the controller determines
77+
* that no conversion is needed. Not a routable strategy.
78+
*/
79+
None = 'none',
7580
Relay = 'relay',
7681
Server = 'server',
7782
}
7883

79-
const VALID_STRATEGIES = new Set(Object.values(TransactionPayStrategy));
84+
const VALID_STRATEGIES = new Set(
85+
Object.values(TransactionPayStrategy).filter(
86+
(strategy) => strategy !== TransactionPayStrategy.None,
87+
),
88+
);
8089

8190
/**
82-
* Checks if a value is a valid transaction pay strategy.
91+
* Checks if a value is a valid, routable transaction pay strategy.
92+
* The internal no-op marker is not routable.
8393
*
8494
* @param strategy - Candidate strategy value.
8595
* @returns True if the value is a valid strategy.

packages/transaction-pay-controller/src/helpers/QuoteRefresher.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,31 @@ describe('QuoteRefresher', () => {
8080
expect(refreshQuotesMock).not.toHaveBeenCalled();
8181
});
8282

83+
it('does not poll if only no-op quotes in state', async () => {
84+
new QuoteRefresher({
85+
getStrategies: jest.fn().mockReturnValue([TransactionPayStrategy.Relay]),
86+
messenger,
87+
updateTransactionData: jest.fn(),
88+
});
89+
90+
publish(
91+
'TransactionPayController:stateChange',
92+
{
93+
transactionData: {
94+
'123': {
95+
quotes: [{ strategy: TransactionPayStrategy.None }],
96+
} as TransactionData,
97+
},
98+
},
99+
[],
100+
);
101+
102+
jest.runAllTimers();
103+
await flushPromises();
104+
105+
expect(refreshQuotesMock).not.toHaveBeenCalled();
106+
});
107+
83108
it('polls again after interval', async () => {
84109
new QuoteRefresher({
85110
getStrategies: jest.fn().mockReturnValue([TransactionPayStrategy.Relay]),

packages/transaction-pay-controller/src/helpers/QuoteRefresher.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,11 @@ export class QuoteRefresher {
106106
}
107107

108108
#onStateChange(state: TransactionPayControllerState): void {
109+
// No-op quotes never refresh, so they don't need the refresh loop.
109110
const hasQuotes = Object.values(state.transactionData).some((transaction) =>
110-
Boolean(transaction.quotes?.length),
111+
transaction.quotes?.some(
112+
(quote) => quote.strategy !== TransactionPayStrategy.None,
113+
),
111114
);
112115

113116
if (hasQuotes && !this.#isRunning) {

packages/transaction-pay-controller/src/helpers/TransactionPayPublishHook.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,43 @@ describe('TransactionPayPublishHook', () => {
115115
expect(executeMock).not.toHaveBeenCalled();
116116
});
117117

118+
it('does nothing if transaction data has no quotes', async () => {
119+
getControllerStateMock.mockReturnValue({
120+
transactionData: {
121+
[TRANSACTION_META_MOCK.id]: {
122+
isLoading: false,
123+
paymentToken: { address: '0x123' },
124+
tokens: [],
125+
},
126+
},
127+
} as unknown as TransactionPayControllerState);
128+
129+
const result = await runHook();
130+
131+
expect(result).toStrictEqual({ transactionHash: undefined });
132+
expect(executeMock).not.toHaveBeenCalled();
133+
expect(updateTransactionMock).not.toHaveBeenCalled();
134+
});
135+
136+
it('does nothing if only a no-op quote is present', async () => {
137+
getControllerStateMock.mockReturnValue({
138+
transactionData: {
139+
[TRANSACTION_META_MOCK.id]: {
140+
isLoading: false,
141+
paymentToken: { address: '0x123' },
142+
quotes: [{ strategy: TransactionPayStrategy.None }],
143+
tokens: [],
144+
},
145+
},
146+
} as unknown as TransactionPayControllerState);
147+
148+
const result = await runHook();
149+
150+
expect(result).toStrictEqual({ transactionHash: undefined });
151+
expect(executeMock).not.toHaveBeenCalled();
152+
expect(updateTransactionMock).not.toHaveBeenCalled();
153+
});
154+
118155
it('throws if fiat payment is selected but no quotes are in state', async () => {
119156
getControllerStateMock.mockReturnValue({
120157
transactionData: {

packages/transaction-pay-controller/src/helpers/TransactionPayPublishHook.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { PublishHookResult } from '@metamask/transaction-controller';
44
import type { Hex } from '@metamask/utils';
55
import { createModuleLogger } from '@metamask/utils';
66

7+
import { TransactionPayStrategy } from '../constants';
78
import { projectLogger } from '../logger';
89
import type {
910
TransactionPayControllerMessenger,
@@ -64,18 +65,23 @@ export class TransactionPayPublishHook {
6465
);
6566

6667
const transactionData = controllerState.transactionData?.[transactionId];
67-
const quotes =
68-
(transactionData?.quotes as TransactionPayQuote<unknown>[]) ?? [];
68+
69+
// No-op quotes mark direct routes and cannot be executed by any strategy.
70+
const quotes = (
71+
(transactionData?.quotes as TransactionPayQuote<unknown>[]) ?? []
72+
).filter((quote) => quote.strategy !== TransactionPayStrategy.None);
73+
6974
const isFiatSelected = Boolean(
7075
transactionData?.fiatPayment?.selectedPaymentMethodId,
7176
);
7277

73-
if (!quotes?.length) {
78+
if (!quotes.length) {
7479
if (isFiatSelected) {
7580
throw new Error('Fiat: Missing quote');
7681
}
7782

78-
log('Skipping as no quotes found');
83+
log('Skipping as no executable quotes found', { transactionId });
84+
7985
return EMPTY_RESULT;
8086
}
8187

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import type { Hex } from '@metamask/utils';
2+
3+
import { TransactionPayStrategy } from '../constants';
4+
import type { TransactionPaymentToken } from '../types';
5+
import { buildNoOpQuote } from './no-op-quote';
6+
7+
const FROM_MOCK = '0xabc' as Hex;
8+
9+
const PAYMENT_TOKEN_MOCK = {
10+
address: '0x123' as Hex,
11+
balanceRaw: '2000000',
12+
chainId: '0x89' as Hex,
13+
decimals: 6,
14+
symbol: 'pUSD',
15+
} as TransactionPaymentToken;
16+
17+
describe('No-Op Quote Utils', () => {
18+
describe('buildNoOpQuote', () => {
19+
it('builds quote with none strategy', () => {
20+
const quote = buildNoOpQuote(FROM_MOCK, PAYMENT_TOKEN_MOCK);
21+
22+
expect(quote.strategy).toBe(TransactionPayStrategy.None);
23+
});
24+
25+
it('builds quote with zero fees and amounts', () => {
26+
const quote = buildNoOpQuote(FROM_MOCK, PAYMENT_TOKEN_MOCK);
27+
28+
expect(quote.estimatedDuration).toBe(0);
29+
expect(quote.dust).toStrictEqual({ fiat: '0', usd: '0' });
30+
expect(quote.targetAmount).toStrictEqual({ fiat: '0', usd: '0' });
31+
expect(quote.sourceAmount).toStrictEqual({
32+
fiat: '0',
33+
usd: '0',
34+
human: '0',
35+
raw: '0',
36+
});
37+
expect(quote.fees).toStrictEqual({
38+
metaMask: { fiat: '0', usd: '0' },
39+
provider: { fiat: '0', usd: '0' },
40+
sourceNetwork: {
41+
estimate: { fiat: '0', usd: '0', human: '0', raw: '0' },
42+
max: { fiat: '0', usd: '0', human: '0', raw: '0' },
43+
},
44+
targetNetwork: { fiat: '0', usd: '0' },
45+
});
46+
});
47+
48+
it('builds request targeting the payment token', () => {
49+
const quote = buildNoOpQuote(FROM_MOCK, PAYMENT_TOKEN_MOCK);
50+
51+
expect(quote.request).toStrictEqual({
52+
from: FROM_MOCK,
53+
sourceBalanceRaw: PAYMENT_TOKEN_MOCK.balanceRaw,
54+
sourceChainId: PAYMENT_TOKEN_MOCK.chainId,
55+
sourceTokenAddress: PAYMENT_TOKEN_MOCK.address,
56+
sourceTokenAmount: '0',
57+
targetAmountMinimum: '0',
58+
targetChainId: PAYMENT_TOKEN_MOCK.chainId,
59+
targetTokenAddress: PAYMENT_TOKEN_MOCK.address,
60+
});
61+
});
62+
});
63+
});
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import type { Hex, Json } from '@metamask/utils';
2+
3+
import { TransactionPayStrategy } from '../constants';
4+
import type {
5+
Amount,
6+
FiatValue,
7+
TransactionPaymentToken,
8+
TransactionPayQuote,
9+
} from '../types';
10+
11+
function zeroFiat(): FiatValue {
12+
return { fiat: '0', usd: '0' };
13+
}
14+
15+
function zeroAmount(): Amount {
16+
return { ...zeroFiat(), human: '0', raw: '0' };
17+
}
18+
19+
/**
20+
* Build a no-op quote for a transaction that needs no conversion.
21+
*
22+
* Stored instead of an empty quotes array when a payment token is selected
23+
* but the route is direct (e.g. same token and chain, or sufficient balance).
24+
* This gives clients and the publish hook an explicit marker to distinguish
25+
* "no quote needed" from "quote needed but missing". No-op quotes use the
26+
* `TransactionPayStrategy.None` strategy, cannot be executed, and must be
27+
* ignored by fee totals and quote refreshes.
28+
*
29+
* @param from - Resolved wallet address for the transaction.
30+
* @param paymentToken - Selected payment token.
31+
* @returns The no-op quote.
32+
*/
33+
export function buildNoOpQuote(
34+
from: Hex,
35+
paymentToken: TransactionPaymentToken,
36+
): TransactionPayQuote<Json> {
37+
return {
38+
dust: zeroFiat(),
39+
estimatedDuration: 0,
40+
fees: {
41+
metaMask: zeroFiat(),
42+
provider: zeroFiat(),
43+
sourceNetwork: {
44+
estimate: zeroAmount(),
45+
max: zeroAmount(),
46+
},
47+
targetNetwork: zeroFiat(),
48+
},
49+
original: null,
50+
request: {
51+
from,
52+
sourceBalanceRaw: paymentToken.balanceRaw,
53+
sourceChainId: paymentToken.chainId,
54+
sourceTokenAddress: paymentToken.address,
55+
sourceTokenAmount: '0',
56+
targetAmountMinimum: '0',
57+
targetChainId: paymentToken.chainId,
58+
targetTokenAddress: paymentToken.address,
59+
},
60+
sourceAmount: zeroAmount(),
61+
strategy: TransactionPayStrategy.None,
62+
targetAmount: zeroFiat(),
63+
};
64+
}

0 commit comments

Comments
 (0)