diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index 46245d32f82..6eb3fb4642e 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **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] ### Added 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/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/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/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 { 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..7a8bfc90921 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 { + 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'); 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, );