From 9af1be5cc17801829d1a7c51da1fbff24aff77b5 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Thu, 13 Nov 2025 11:56:35 +0000 Subject: [PATCH 1/5] Get quote if relay and arbitrum usdc Make getStrategy synchronous. --- .../src/TransactionPayController.test.ts | 6 +- .../src/TransactionPayController.ts | 4 +- .../helpers/TransactionPayPublishHook.test.ts | 2 +- .../transaction-pay-controller/src/types.ts | 6 +- .../src/utils/quotes.test.ts | 2 +- .../src/utils/source-amounts.test.ts | 53 ++++++++++++--- .../src/utils/source-amounts.ts | 64 +++++++++++++++++-- .../src/utils/strategy.test.ts | 20 +++--- .../src/utils/strategy.ts | 6 +- 9 files changed, 124 insertions(+), 39 deletions(-) diff --git a/packages/transaction-pay-controller/src/TransactionPayController.test.ts b/packages/transaction-pay-controller/src/TransactionPayController.test.ts index 59095e59bc4..473fc919c12 100644 --- a/packages/transaction-pay-controller/src/TransactionPayController.test.ts +++ b/packages/transaction-pay-controller/src/TransactionPayController.test.ts @@ -77,7 +77,7 @@ describe('TransactionPayController', () => { createController(); expect( - await messenger.call( + messenger.call( 'TransactionPayController:getStrategy', TRANSACTION_META_MOCK, ), @@ -87,12 +87,12 @@ describe('TransactionPayController', () => { it('returns callback value if provided', async () => { new TransactionPayController({ getDelegationTransaction: jest.fn(), - getStrategy: async () => TransactionPayStrategy.Test, + getStrategy: () => TransactionPayStrategy.Test, messenger, }); expect( - await messenger.call( + messenger.call( 'TransactionPayController:getStrategy', TRANSACTION_META_MOCK, ), diff --git a/packages/transaction-pay-controller/src/TransactionPayController.ts b/packages/transaction-pay-controller/src/TransactionPayController.ts index eac0d916eb2..be80ea3f7d5 100644 --- a/packages/transaction-pay-controller/src/TransactionPayController.ts +++ b/packages/transaction-pay-controller/src/TransactionPayController.ts @@ -41,7 +41,7 @@ export class TransactionPayController extends BaseController< readonly #getStrategy?: ( transaction: TransactionMeta, - ) => Promise; + ) => TransactionPayStrategy; constructor({ getDelegationTransaction, @@ -139,7 +139,7 @@ export class TransactionPayController extends BaseController< this.messenger.registerActionHandler( 'TransactionPayController:getStrategy', - this.#getStrategy ?? (async () => TransactionPayStrategy.Relay), + this.#getStrategy ?? (() => TransactionPayStrategy.Relay), ); this.messenger.registerActionHandler( diff --git a/packages/transaction-pay-controller/src/helpers/TransactionPayPublishHook.test.ts b/packages/transaction-pay-controller/src/helpers/TransactionPayPublishHook.test.ts index b87ab8f2a22..681847534cc 100644 --- a/packages/transaction-pay-controller/src/helpers/TransactionPayPublishHook.test.ts +++ b/packages/transaction-pay-controller/src/helpers/TransactionPayPublishHook.test.ts @@ -61,7 +61,7 @@ describe('TransactionPayPublishHook', () => { }, } as TransactionPayControllerState); - getStrategyMock.mockResolvedValue(TransactionPayStrategy.Test); + getStrategyMock.mockReturnValue(TransactionPayStrategy.Test); }); it('executes strategy with quotes', async () => { diff --git a/packages/transaction-pay-controller/src/types.ts b/packages/transaction-pay-controller/src/types.ts index bb7571dac92..530abe7d4a3 100644 --- a/packages/transaction-pay-controller/src/types.ts +++ b/packages/transaction-pay-controller/src/types.ts @@ -68,7 +68,7 @@ export type TransactionPayControllerGetDelegationTransactionAction = { /** Action to get the pay strategy type used for a transaction. */ export type TransactionPayControllerGetStrategyAction = { type: `${typeof CONTROLLER_NAME}:getStrategy`; - handler: (transaction: TransactionMeta) => Promise; + handler: (transaction: TransactionMeta) => TransactionPayStrategy; }; /** Action to update the payment token for a transaction. */ @@ -104,9 +104,7 @@ export type TransactionPayControllerOptions = { getDelegationTransaction: GetDelegationTransactionCallback; /** Callback to select the PayStrategy for a transaction. */ - getStrategy?: ( - transaction: TransactionMeta, - ) => Promise; + getStrategy?: (transaction: TransactionMeta) => TransactionPayStrategy; /** Controller messenger. */ messenger: TransactionPayControllerMessenger; diff --git a/packages/transaction-pay-controller/src/utils/quotes.test.ts b/packages/transaction-pay-controller/src/utils/quotes.test.ts index 576e282c5c1..af60914f3d5 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.test.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.test.ts @@ -114,7 +114,7 @@ describe('Quotes Utils', () => { jest.resetAllMocks(); jest.clearAllTimers(); - getStrategyMock.mockResolvedValue({ + getStrategyMock.mockReturnValue({ execute: jest.fn(), getQuotes: getQuotesMock, getBatchTransactions: getBatchTransactionsMock, diff --git a/packages/transaction-pay-controller/src/utils/source-amounts.test.ts b/packages/transaction-pay-controller/src/utils/source-amounts.test.ts index 5784a49170b..6322a93d0d3 100644 --- a/packages/transaction-pay-controller/src/utils/source-amounts.test.ts +++ b/packages/transaction-pay-controller/src/utils/source-amounts.test.ts @@ -1,9 +1,16 @@ import { updateSourceAmounts } from './source-amounts'; import { getTokenFiatRate } from './token'; -import type { TransactionPaymentToken } from '..'; +import { getTransaction } from './transaction'; +import { TransactionPayStrategy, type TransactionPaymentToken } from '..'; +import { getMessengerMock } from '../tests/messenger-mock'; import type { TransactionData, TransactionPayRequiredToken } from '../types'; +import { + ARBITRUM_USDC_ADDRESS, + CHAIN_ID_ARBITRUM, +} from '../strategy/relay/constants'; jest.mock('./token'); +jest.mock('./transaction'); const PAYMENT_TOKEN_MOCK: TransactionPaymentToken = { address: '0x123', @@ -37,11 +44,15 @@ const TRANSACTION_ID_MOCK = '123-456'; describe('Source Amounts Utils', () => { const getTokenFiatRateMock = jest.mocked(getTokenFiatRate); + const getTransactionMock = jest.mocked(getTransaction); + const { messenger, getStrategyMock } = getMessengerMock(); beforeEach(() => { jest.resetAllMocks(); getTokenFiatRateMock.mockReturnValue({ fiatRate: '2.0', usdRate: '3.0' }); + getStrategyMock.mockReturnValue(TransactionPayStrategy.Test); + getTransactionMock.mockReturnValue({ id: TRANSACTION_ID_MOCK } as never); }); describe('updateSourceAmounts', () => { @@ -52,7 +63,7 @@ describe('Source Amounts Utils', () => { tokens: [TRANSACTION_TOKEN_MOCK], }; - updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, {} as never); + updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, messenger); expect(transactionData.sourceAmounts).toStrictEqual([ { @@ -76,11 +87,35 @@ describe('Source Amounts Utils', () => { ], }; - updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, {} as never); + updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, messenger); expect(transactionData.sourceAmounts).toStrictEqual([]); }); + it('does not return empty array if payment token matches but hyperliquid deposit and relay strategy', () => { + getStrategyMock.mockReturnValue(TransactionPayStrategy.Relay); + + const transactionData: TransactionData = { + isLoading: false, + paymentToken: { + ...PAYMENT_TOKEN_MOCK, + address: ARBITRUM_USDC_ADDRESS, + chainId: CHAIN_ID_ARBITRUM, + }, + tokens: [ + { + ...TRANSACTION_TOKEN_MOCK, + address: ARBITRUM_USDC_ADDRESS, + chainId: CHAIN_ID_ARBITRUM, + }, + ], + }; + + updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, messenger); + + expect(transactionData.sourceAmounts).toHaveLength(1); + }); + it('returns empty array if skipIfBalance and has balance', () => { const transactionData: TransactionData = { isLoading: false, @@ -94,7 +129,7 @@ describe('Source Amounts Utils', () => { ], }; - updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, {} as never); + updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, messenger); expect(transactionData.sourceAmounts).toStrictEqual([]); }); @@ -108,7 +143,7 @@ describe('Source Amounts Utils', () => { getTokenFiatRateMock.mockReturnValue(undefined); - updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, {} as never); + updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, messenger); expect(transactionData.sourceAmounts).toStrictEqual([]); }); @@ -125,7 +160,7 @@ describe('Source Amounts Utils', () => { ], }; - updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, {} as never); + updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, messenger); expect(transactionData.sourceAmounts).toStrictEqual([]); }); @@ -136,7 +171,7 @@ describe('Source Amounts Utils', () => { tokens: [TRANSACTION_TOKEN_MOCK], }; - updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, {} as never); + updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, messenger); expect(transactionData.sourceAmounts).toBeUndefined(); }); @@ -148,14 +183,14 @@ describe('Source Amounts Utils', () => { tokens: [], }; - updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, {} as never); + updateSourceAmounts(TRANSACTION_ID_MOCK, transactionData, messenger); expect(transactionData.sourceAmounts).toBeUndefined(); }); // eslint-disable-next-line jest/expect-expect it('does nothing if no transaction data', () => { - updateSourceAmounts(TRANSACTION_ID_MOCK, undefined, {} as never); + updateSourceAmounts(TRANSACTION_ID_MOCK, undefined, messenger); }); }); }); diff --git a/packages/transaction-pay-controller/src/utils/source-amounts.ts b/packages/transaction-pay-controller/src/utils/source-amounts.ts index fe5203b852c..9907dba2dab 100644 --- a/packages/transaction-pay-controller/src/utils/source-amounts.ts +++ b/packages/transaction-pay-controller/src/utils/source-amounts.ts @@ -2,11 +2,18 @@ import { createModuleLogger } from '@metamask/utils'; import { BigNumber } from 'bignumber.js'; import { getTokenFiatRate } from './token'; +import { getTransaction } from './transaction'; import type { TransactionPayControllerMessenger, TransactionPaymentToken, } from '..'; +import { TransactionPayStrategy } from '..'; +import type { TransactionMeta } from '../../../transaction-controller/src'; import { projectLogger } from '../logger'; +import { + ARBITRUM_USDC_ADDRESS, + CHAIN_ID_ARBITRUM, +} from '../strategy/relay/constants'; import type { TransactionPaySourceAmount, TransactionData, @@ -38,7 +45,9 @@ export function updateSourceAmounts( } const sourceAmounts = tokens - .map((t) => calculateSourceAmount(paymentToken, t, messenger)) + .map((t) => + calculateSourceAmount(paymentToken, t, messenger, transactionId), + ) .filter(Boolean) as TransactionPaySourceAmount[]; log('Updated source amounts', { transactionId, sourceAmounts }); @@ -52,12 +61,14 @@ export function updateSourceAmounts( * @param paymentToken - Selected payment token. * @param token - Target token to cover. * @param messenger - Controller messenger. + * @param transactionId - ID of the transaction. * @returns The source amount or undefined if calculation failed. */ function calculateSourceAmount( paymentToken: TransactionPaymentToken, token: TransactionPayRequiredToken, messenger: TransactionPayControllerMessenger, + transactionId: string, ): TransactionPaySourceAmount | undefined { const paymentTokenFiatRate = getTokenFiatRate( messenger, @@ -71,10 +82,6 @@ function calculateSourceAmount( const hasBalance = new BigNumber(token.balanceRaw).gte(token.amountRaw); - const isSameTokenSelected = - token.address.toLowerCase() === paymentToken.address.toLowerCase() && - token.chainId === paymentToken.chainId; - if (token.skipIfBalance && hasBalance) { log('Skipping token as sufficient balance', { tokenAddress: token.address, @@ -82,7 +89,15 @@ function calculateSourceAmount( return undefined; } - if (isSameTokenSelected) { + const strategy = getStrategyType(transactionId, messenger); + + const isSameTokenSelected = + token.address.toLowerCase() === paymentToken.address.toLowerCase() && + token.chainId === paymentToken.chainId; + + const isAlwaysRequired = isQuoteAlwaysRequired(token, strategy); + + if (isSameTokenSelected && !isAlwaysRequired) { log('Skipping token as same as payment token'); return undefined; } @@ -108,3 +123,40 @@ function calculateSourceAmount( targetTokenAddress: token.address, }; } + +/** + * Determine if a quote is always required for a token and strategy. + * + * @param token - Target token. + * @param strategy - Payment strategy. + * @returns True if a quote is always required, false otherwise. + */ +function isQuoteAlwaysRequired( + token: TransactionPayRequiredToken, + strategy: TransactionPayStrategy, +) { + const isHyperliquidDeposit = + token.chainId === CHAIN_ID_ARBITRUM && + token.address.toLowerCase() === ARBITRUM_USDC_ADDRESS.toLowerCase(); + + return strategy === TransactionPayStrategy.Relay && isHyperliquidDeposit; +} + +/** + * Get the strategy type for a transaction. + * + * @param transactionId - ID of the transaction. + * @param messenger - Controller messenger. + * @returns Payment strategy type. + */ +function getStrategyType( + transactionId: string, + messenger: TransactionPayControllerMessenger, +) { + const transaction = getTransaction( + transactionId, + messenger, + ) as TransactionMeta; + + return messenger.call('TransactionPayController:getStrategy', transaction); +} diff --git a/packages/transaction-pay-controller/src/utils/strategy.test.ts b/packages/transaction-pay-controller/src/utils/strategy.test.ts index 5af1a6694ee..7b2c65593e2 100644 --- a/packages/transaction-pay-controller/src/utils/strategy.test.ts +++ b/packages/transaction-pay-controller/src/utils/strategy.test.ts @@ -18,35 +18,35 @@ describe('Strategy Utils', () => { describe('getStrategy', () => { it('returns TestStrategy if strategy name is Test', async () => { - getStrategyMock.mockResolvedValue(TransactionPayStrategy.Test); + getStrategyMock.mockReturnValue(TransactionPayStrategy.Test); - const strategy = await getStrategy(messenger, TRANSACTION_META_MOCK); + const strategy = getStrategy(messenger, TRANSACTION_META_MOCK); expect(strategy).toBeInstanceOf(TestStrategy); }); it('returns BridgeStrategy if strategy name is Bridge', async () => { - getStrategyMock.mockResolvedValue(TransactionPayStrategy.Bridge); + getStrategyMock.mockReturnValue(TransactionPayStrategy.Bridge); - const strategy = await getStrategy(messenger, TRANSACTION_META_MOCK); + const strategy = getStrategy(messenger, TRANSACTION_META_MOCK); expect(strategy).toBeInstanceOf(BridgeStrategy); }); it('returns RelayStrategy if strategy name is Relay', async () => { - getStrategyMock.mockResolvedValue(TransactionPayStrategy.Relay); + getStrategyMock.mockReturnValue(TransactionPayStrategy.Relay); - const strategy = await getStrategy(messenger, TRANSACTION_META_MOCK); + const strategy = getStrategy(messenger, TRANSACTION_META_MOCK); expect(strategy).toBeInstanceOf(RelayStrategy); }); it('throws if strategy name is unknown', async () => { - getStrategyMock.mockResolvedValue('UnknownStrategy' as never); + getStrategyMock.mockReturnValue('UnknownStrategy' as never); - await expect( - getStrategy(messenger, TRANSACTION_META_MOCK), - ).rejects.toThrow('Unknown strategy: UnknownStrategy'); + expect(() => getStrategy(messenger, TRANSACTION_META_MOCK)).toThrow( + 'Unknown strategy: UnknownStrategy', + ); }); }); diff --git a/packages/transaction-pay-controller/src/utils/strategy.ts b/packages/transaction-pay-controller/src/utils/strategy.ts index 450e2b2cbf5..dc505431f4e 100644 --- a/packages/transaction-pay-controller/src/utils/strategy.ts +++ b/packages/transaction-pay-controller/src/utils/strategy.ts @@ -13,11 +13,11 @@ import type { PayStrategy, TransactionPayControllerMessenger } from '../types'; * @param transaction - Transaction to get the strategy for. * @returns The payment strategy instance. */ -export async function getStrategy( +export function getStrategy( messenger: TransactionPayControllerMessenger, transaction: TransactionMeta, -): Promise> { - const strategyName = await messenger.call( +): PayStrategy { + const strategyName = messenger.call( 'TransactionPayController:getStrategy', transaction, ); From e2facf9bd502c8d92b2e275dd9c948ca0542b409 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Thu, 13 Nov 2025 12:14:01 +0000 Subject: [PATCH 2/5] Update changelog --- packages/transaction-pay-controller/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index 46245d32f82..97fce185173 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Always retrieve quote if using Relay strategy and required token is Arbitrum USDC, even if payment token matches ([#7146](https://github.com/MetaMask/core/pull/7146)) + ## [5.0.0] ### Added From cddd136fff026e2d839636e60d3fa22dd294d9e2 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Thu, 13 Nov 2025 12:15:08 +0000 Subject: [PATCH 3/5] Update changelog --- packages/transaction-pay-controller/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index 97fce185173..6eb3fb4642e 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Always retrieve quote if using Relay strategy and required token is Arbitrum USDC, even if payment token matches ([#7146](https://github.com/MetaMask/core/pull/7146)) +- **BREAKING:** Always retrieve quote if using Relay strategy and required token is Arbitrum USDC, even if payment token matches ([#7146](https://github.com/MetaMask/core/pull/7146)) + - Change `getStrategy` constructor option from asynchronous to synchronous. ## [5.0.0] From 5b0636646149eec6a0ba75714d2e0a5216a4e5f8 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Thu, 13 Nov 2025 12:24:59 +0000 Subject: [PATCH 4/5] Fix linting --- .../src/utils/source-amounts.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/transaction-pay-controller/src/utils/source-amounts.test.ts b/packages/transaction-pay-controller/src/utils/source-amounts.test.ts index 6322a93d0d3..7a8bfc90921 100644 --- a/packages/transaction-pay-controller/src/utils/source-amounts.test.ts +++ b/packages/transaction-pay-controller/src/utils/source-amounts.test.ts @@ -2,12 +2,12 @@ import { updateSourceAmounts } from './source-amounts'; import { getTokenFiatRate } from './token'; import { getTransaction } from './transaction'; import { TransactionPayStrategy, type TransactionPaymentToken } from '..'; -import { getMessengerMock } from '../tests/messenger-mock'; -import type { TransactionData, TransactionPayRequiredToken } from '../types'; import { ARBITRUM_USDC_ADDRESS, CHAIN_ID_ARBITRUM, } from '../strategy/relay/constants'; +import { getMessengerMock } from '../tests/messenger-mock'; +import type { TransactionData, TransactionPayRequiredToken } from '../types'; jest.mock('./token'); jest.mock('./transaction'); From fd6a5d7bd011d90c33172da1d43a67c1160ee390 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Thu, 13 Nov 2025 12:38:51 +0000 Subject: [PATCH 5/5] Fix linting --- .../src/helpers/TransactionPayPublishHook.ts | 2 +- packages/transaction-pay-controller/src/utils/quotes.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/transaction-pay-controller/src/helpers/TransactionPayPublishHook.ts b/packages/transaction-pay-controller/src/helpers/TransactionPayPublishHook.ts index 7bbf7c7e8ac..c49cbcf252f 100644 --- a/packages/transaction-pay-controller/src/helpers/TransactionPayPublishHook.ts +++ b/packages/transaction-pay-controller/src/helpers/TransactionPayPublishHook.ts @@ -68,7 +68,7 @@ export class TransactionPayPublishHook { return EMPTY_RESULT; } - const strategy = await getStrategy(this.#messenger, transactionMeta); + const strategy = getStrategy(this.#messenger, transactionMeta); return await strategy.execute({ isSmartTransaction: this.#isSmartTransaction, diff --git a/packages/transaction-pay-controller/src/utils/quotes.ts b/packages/transaction-pay-controller/src/utils/quotes.ts index 95c498414b2..d5d94581200 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.ts @@ -272,7 +272,7 @@ async function getQuotes( messenger: TransactionPayControllerMessenger, ) { const { id: transactionId } = transaction; - const strategy = await getStrategy(messenger as never, transaction); + const strategy = getStrategy(messenger as never, transaction); let quotes: TransactionPayQuote[] | undefined = []; try {