diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index c4e82c68b24..1039de52ed2 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/assets-controller` from `^2.4.0` to `^3.0.0` ([#8232](https://github.com/MetaMask/core/pull/8232)) - Bump `@metamask/assets-controllers` from `^101.0.0` to `^101.0.1` ([#8232](https://github.com/MetaMask/core/pull/8232)) - Remove duplication in gas estimation for Relay and Across strategies ([#8145](https://github.com/MetaMask/core/pull/8145)) +- Improve Across quote handling to decode supported destination calls into post-swap actions while sending transfer-only destinations directly to the destination recipient ([#8208](https://github.com/MetaMask/core/pull/8208)) ## [17.1.0] diff --git a/packages/transaction-pay-controller/src/strategy/across/across-actions.test.ts b/packages/transaction-pay-controller/src/strategy/across/across-actions.test.ts new file mode 100644 index 00000000000..358a86f19da --- /dev/null +++ b/packages/transaction-pay-controller/src/strategy/across/across-actions.test.ts @@ -0,0 +1,231 @@ +import { Interface } from '@ethersproject/abi'; +import type { TransactionMeta } from '@metamask/transaction-controller'; +import type { Hex } from '@metamask/utils'; + +import { + buildAcrossActionFromCall, + CREATE_PROXY_SIGNATURE, + getAcrossDestination, + getTransferRecipient, + isExtractableOutputTokenTransferCall, + SAFE_EXEC_TRANSACTION_SIGNATURE, + TOKEN_TRANSFER_SIGNATURE, +} from './across-actions'; +import type { QuoteRequest } from '../../types'; + +const TOKEN_TRANSFER_INTERFACE = new Interface([TOKEN_TRANSFER_SIGNATURE]); +const CREATE_PROXY_INTERFACE = new Interface([CREATE_PROXY_SIGNATURE]); +const SAFE_EXEC_TRANSACTION_INTERFACE = new Interface([ + SAFE_EXEC_TRANSACTION_SIGNATURE, +]); + +const REQUEST_MOCK: QuoteRequest = { + from: '0x1234567890123456789012345678901234567891' as Hex, + sourceBalanceRaw: '10000000000000000000', + sourceChainId: '0x1', + sourceTokenAddress: '0xabc' as Hex, + sourceTokenAmount: '1000000000000000000', + targetAmountMinimum: '123', + targetChainId: '0x2', + targetTokenAddress: '0xdef' as Hex, +}; + +const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; +const TRANSFER_RECIPIENT = '0xabcdefabcdefabcdefabcdefabcdefabcdefabcd' as Hex; +const TRANSFER_TARGET = REQUEST_MOCK.targetTokenAddress; +const CREATE_PROXY_TARGET = '0xfac7fac7fac7fac7fac7fac7fac7fac7fac7fac7' as Hex; +const EXEC_TRANSACTION_TARGET = + '0x5afe5afe5afe5afe5afe5afe5afe5afe5afe5afe' as Hex; + +function buildTransferData( + recipient: Hex = TRANSFER_RECIPIENT, + amount = 1, +): Hex { + return TOKEN_TRANSFER_INTERFACE.encodeFunctionData('transfer', [ + recipient, + amount, + ]) as Hex; +} + +function buildCreateProxyData(): Hex { + return CREATE_PROXY_INTERFACE.encodeFunctionData('createProxy', [ + ZERO_ADDRESS, + '0', + ZERO_ADDRESS, + { + r: `0x${'11'.repeat(32)}`, + s: `0x${'22'.repeat(32)}`, + v: 27, + }, + ]) as Hex; +} + +function buildExecTransactionData(): Hex { + return SAFE_EXEC_TRANSACTION_INTERFACE.encodeFunctionData('execTransaction', [ + '0xc0ffee254729296a45a3885639AC7E10F9d54979', + '0', + '0x12345678', + 0, + 0, + 0, + 0, + ZERO_ADDRESS, + ZERO_ADDRESS, + '0xabcdef', + ]) as Hex; +} + +describe('across-actions', () => { + it('builds transfer actions with a dynamic output-token amount', () => { + expect( + buildAcrossActionFromCall({ data: buildTransferData() }, REQUEST_MOCK), + ).toStrictEqual({ + args: [ + { + populateDynamically: false, + value: TRANSFER_RECIPIENT.toLowerCase(), + }, + { + balanceSourceToken: REQUEST_MOCK.targetTokenAddress, + populateDynamically: true, + value: '0', + }, + ], + functionSignature: TOKEN_TRANSFER_SIGNATURE, + isNativeTransfer: false, + target: REQUEST_MOCK.targetTokenAddress, + value: '0', + }); + + expect( + buildAcrossActionFromCall( + { + data: buildTransferData(), + target: TRANSFER_TARGET, + }, + REQUEST_MOCK, + ).target, + ).toBe(TRANSFER_TARGET); + }); + + it('builds non-transfer actions by decoding the supported signature registry', () => { + expect( + buildAcrossActionFromCall( + { + data: buildExecTransactionData(), + target: EXEC_TRANSACTION_TARGET, + }, + REQUEST_MOCK, + ), + ).toMatchObject({ + functionSignature: SAFE_EXEC_TRANSACTION_SIGNATURE, + target: EXEC_TRANSACTION_TARGET, + value: '0', + }); + }); + + it('builds an Across destination directly from a transfer transaction', () => { + expect( + getAcrossDestination( + { + txParams: { + data: buildTransferData(), + from: REQUEST_MOCK.from, + }, + } as TransactionMeta, + REQUEST_MOCK, + ), + ).toStrictEqual({ + actions: [], + recipient: TRANSFER_RECIPIENT.toLowerCase(), + }); + }); + + it('prefers nested destination calls over top-level calldata', () => { + expect( + getAcrossDestination( + { + nestedTransactions: [ + { + data: buildTransferData(), + to: TRANSFER_TARGET, + }, + ], + txParams: { + data: buildCreateProxyData(), + from: REQUEST_MOCK.from, + to: CREATE_PROXY_TARGET, + }, + } as TransactionMeta, + REQUEST_MOCK, + ), + ).toStrictEqual({ + actions: [], + recipient: TRANSFER_RECIPIENT.toLowerCase(), + }); + }); + + it('throws when decoding a supported action that requires a target without one', () => { + expect(() => + buildAcrossActionFromCall({ data: buildCreateProxyData() }, REQUEST_MOCK), + ).toThrow(/Across only supports direct token transfers/u); + }); + + it('throws when the calldata does not match a supported signature', () => { + expect(() => + buildAcrossActionFromCall( + { + data: '0xdeadbeef' as Hex, + target: CREATE_PROXY_TARGET, + }, + REQUEST_MOCK, + ), + ).toThrow(/Destination selector: 0xdeadbeef/u); + }); + + it('extracts and normalizes transfer recipients from calldata', () => { + expect(getTransferRecipient(buildTransferData())).toBe( + TRANSFER_RECIPIENT.toLowerCase(), + ); + }); + + it('throws when asking for a transfer recipient from non-transfer calldata', () => { + expect(() => getTransferRecipient(buildCreateProxyData())).toThrow( + /Across only supports direct token transfers/u, + ); + }); + + it('recognizes extractable output-token transfers', () => { + expect( + isExtractableOutputTokenTransferCall( + { + data: buildTransferData(), + target: TRANSFER_TARGET, + }, + REQUEST_MOCK, + ), + ).toBe(true); + }); + + it('rejects unsupported or non-output-token transfer calls as extractable recipients', () => { + expect( + isExtractableOutputTokenTransferCall( + { + data: buildTransferData(), + target: '0x9999999999999999999999999999999999999999' as Hex, + }, + REQUEST_MOCK, + ), + ).toBe(false); + + expect( + isExtractableOutputTokenTransferCall( + { + data: '0xdeadbeef' as Hex, + target: TRANSFER_TARGET, + }, + REQUEST_MOCK, + ), + ).toBe(false); + }); +}); diff --git a/packages/transaction-pay-controller/src/strategy/across/across-actions.ts b/packages/transaction-pay-controller/src/strategy/across/across-actions.ts new file mode 100644 index 00000000000..2fa7613e7f2 --- /dev/null +++ b/packages/transaction-pay-controller/src/strategy/across/across-actions.ts @@ -0,0 +1,283 @@ +import { Interface } from '@ethersproject/abi'; +import type { TransactionDescription } from '@ethersproject/abi'; +import type { TransactionMeta } from '@metamask/transaction-controller'; +import type { Hex } from '@metamask/utils'; + +import type { AcrossAction, AcrossActionArg } from './types'; +import type { QuoteRequest } from '../../types'; + +export const TOKEN_TRANSFER_SIGNATURE = + 'function transfer(address to, uint256 value)'; +export const CREATE_PROXY_SIGNATURE = + 'function createProxy(address paymentToken, uint256 payment, address payable paymentReceiver, (uint8 v, bytes32 r, bytes32 s) createSig)'; +export const SAFE_EXEC_TRANSACTION_SIGNATURE = + 'function execTransaction(address to, uint256 value, bytes data, uint8 operation, uint256 safeTxGas, uint256 baseGas, uint256 gasPrice, address gasToken, address refundReceiver, bytes signatures)'; + +export const UNSUPPORTED_DESTINATION_ERROR = + 'Across only supports direct token transfers and a limited set of post-swap destination actions at the moment'; + +export type AcrossDestinationCall = { + data: Hex; + target?: Hex; +}; + +export type AcrossDestination = { + actions: AcrossAction[]; + recipient: Hex; +}; + +type ParsedAcrossActionCall = { + functionSignature: string; + transaction: TransactionDescription; +}; + +type BigNumberLike = { + _isBigNumber: true; + toString: () => string; +}; + +const ACROSS_ACTION_SIGNATURES = [ + CREATE_PROXY_SIGNATURE, + SAFE_EXEC_TRANSACTION_SIGNATURE, +]; + +export function buildAcrossActionFromCall( + call: AcrossDestinationCall, + request: QuoteRequest, +): AcrossAction { + if (isTransferCall(call.data)) { + return buildAcrossTransferAction(call, request); + } + + const parsedCall = parseAcrossActionCall(call.data); + + return { + args: Array.from(parsedCall.transaction.args).map((arg) => ({ + populateDynamically: false, + value: serializeAcrossActionValue(arg), + })), + functionSignature: parsedCall.functionSignature, + isNativeTransfer: false, + target: getRequiredTarget(call), + value: '0', + }; +} + +export function getTransferRecipient(data: Hex): Hex { + const parsedCall = tryParseTransferCall(data); + + if (!parsedCall) { + throw new Error(getUnsupportedDestinationErrorMessage(data)); + } + + return normalizeHexString(String(parsedCall.args[0])) as Hex; +} + +export function getAcrossDestination( + transaction: TransactionMeta, + request: QuoteRequest, +): AcrossDestination { + const { from } = request; + const destinationCalls = getDestinationCalls(transaction); + const swapRecipientTransferCallIndex = destinationCalls.findIndex((call) => + isExtractableOutputTokenTransferCall(call, request), + ); + const callsForActions = [...destinationCalls]; + let recipient = from; + + if (swapRecipientTransferCallIndex !== -1) { + const [swapRecipientTransferCall] = callsForActions.splice( + swapRecipientTransferCallIndex, + 1, + ); + + recipient = getTransferRecipient(swapRecipientTransferCall.data); + } + + return { + actions: callsForActions.map((call) => + buildAcrossActionFromCall(call, request), + ), + recipient, + }; +} + +export function isExtractableOutputTokenTransferCall( + call: AcrossDestinationCall, + request: QuoteRequest, +): boolean { + return ( + isTransferCall(call.data) && + (call.target === undefined || + normalizeHexString(call.target) === + normalizeHexString(request.targetTokenAddress)) + ); +} + +function getRequiredTarget(call: AcrossDestinationCall): Hex { + if (!call.target) { + throw new Error(UNSUPPORTED_DESTINATION_ERROR); + } + + return call.target; +} + +function normalizeHexString(value: string): string { + /* istanbul ignore next: current supported Across action signatures only emit hex strings here. */ + if (!value.startsWith('0x')) { + return value; + } + + return value.toLowerCase(); +} + +function parseAcrossActionCall(data: Hex): ParsedAcrossActionCall { + const parsedCall = tryParseAcrossActionCall(data); + + if (!parsedCall) { + throw new Error(getUnsupportedDestinationErrorMessage(data)); + } + + return parsedCall; +} + +function serializeAcrossActionValue(value: unknown): AcrossActionArg['value'] { + if (Array.isArray(value)) { + return value.map((entry) => + serializeAcrossActionScalar(entry), + ) as AcrossActionArg['value']; + } + + return serializeAcrossActionScalar(value); +} + +function buildAcrossTransferAction( + call: AcrossDestinationCall, + request: QuoteRequest, +): AcrossAction { + return { + args: [ + { + populateDynamically: false, + value: getTransferRecipient(call.data), + }, + { + balanceSourceToken: request.targetTokenAddress, + populateDynamically: true, + value: '0', + }, + ], + functionSignature: TOKEN_TRANSFER_SIGNATURE, + isNativeTransfer: false, + target: call.target ?? request.targetTokenAddress, + value: '0', + }; +} + +function getDestinationCalls( + transaction: TransactionMeta, +): AcrossDestinationCall[] { + const nestedCalls = ( + transaction.nestedTransactions ?? [] + ).flatMap((nestedTx: { data?: Hex; to?: Hex }) => + nestedTx.data !== undefined && nestedTx.data !== '0x' + ? [{ data: nestedTx.data, target: nestedTx.to }] + : [], + ); + + if (nestedCalls.length > 0) { + return nestedCalls; + } + + const data = transaction.txParams?.data as Hex | undefined; + + if (data === undefined || data === '0x') { + return []; + } + + return [ + { + data, + target: transaction.txParams?.to as Hex | undefined, + }, + ]; +} + +function getUnsupportedDestinationErrorMessage(data?: Hex): string { + const selector = getDestinationSelector(data); + + return selector + ? `${UNSUPPORTED_DESTINATION_ERROR}. Destination selector: ${selector}` + : UNSUPPORTED_DESTINATION_ERROR; +} + +function getDestinationSelector(data?: Hex): Hex | undefined { + if (!data || data.length < 10) { + return undefined; + } + + return data.slice(0, 10).toLowerCase() as Hex; +} + +function serializeAcrossActionScalar(value: unknown): string { + if (typeof value === 'string') { + return normalizeHexString(value); + } + + if ( + typeof value === 'number' || + typeof value === 'bigint' || + typeof value === 'boolean' + ) { + return String(value); + } + + if (isBigNumberLike(value)) { + return value.toString(); + } + + /* istanbul ignore next: supported Across action ABIs only decode scalars and tuples of scalars. */ + throw new Error(UNSUPPORTED_DESTINATION_ERROR); +} + +function isBigNumberLike(value: unknown): value is BigNumberLike { + return ( + typeof value === 'object' && + value !== null && + '_isBigNumber' in value && + value._isBigNumber === true && + 'toString' in value && + typeof value.toString === 'function' + ); +} + +function isTransferCall(data: Hex): boolean { + return tryParseTransferCall(data) !== undefined; +} + +function tryParseAcrossActionCall( + data: Hex, +): ParsedAcrossActionCall | undefined { + for (const functionSignature of ACROSS_ACTION_SIGNATURES) { + try { + const actionInterface = new Interface([functionSignature]); + + return { + functionSignature, + transaction: actionInterface.parseTransaction({ data }), + }; + } catch { + // Intentionally empty. + } + } + + return undefined; +} + +function tryParseTransferCall(data: Hex): TransactionDescription | undefined { + try { + return new Interface([TOKEN_TRANSFER_SIGNATURE]).parseTransaction({ data }); + } catch { + return undefined; + } +} diff --git a/packages/transaction-pay-controller/src/strategy/across/across-quotes.test.ts b/packages/transaction-pay-controller/src/strategy/across/across-quotes.test.ts index 11da6c8cf36..29a80dd984d 100644 --- a/packages/transaction-pay-controller/src/strategy/across/across-quotes.test.ts +++ b/packages/transaction-pay-controller/src/strategy/across/across-quotes.test.ts @@ -87,8 +87,22 @@ const QUOTE_MOCK: AcrossSwapApprovalResponse = { const TOKEN_TRANSFER_INTERFACE = new Interface([ 'function transfer(address to, uint256 amount)', ]); +const SAFE_FACTORY_INTERFACE = new Interface([ + 'function createProxy(address paymentToken, uint256 payment, address payable paymentReceiver, (uint8 v, bytes32 r, bytes32 s) createSig)', +]); +const SAFE_INTERFACE = new Interface([ + 'function execTransaction(address to, uint256 value, bytes data, uint8 operation, uint256 safeTxGas, uint256 baseGas, uint256 gasPrice, address gasToken, address refundReceiver, bytes signatures)', +]); const TRANSFER_RECIPIENT = '0xabcdefabcdefabcdefabcdefabcdefabcdefabcd'; +const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; +const FACTORY_ADDRESS = '0xfac7fac7fac7fac7fac7fac7fac7fac7fac7fac7' as Hex; +const SAFE_ADDRESS = '0x5afe5afe5afe5afe5afe5afe5afe5afe5afe5afe' as Hex; +const SAFE_TX_TARGET = '0xc0ffee254729296a45a3885639AC7E10F9d54979'; +const SAFE_TX_DATA = '0x12345678'; +const SAFE_SIGNATURE_BYTES = '0xabcdef'; +const CREATE_PROXY_R = `0x${'11'.repeat(32)}`; +const CREATE_PROXY_S = `0x${'22'.repeat(32)}`; function buildTransferData( recipient: string = TRANSFER_RECIPIENT, @@ -100,6 +114,34 @@ function buildTransferData( ]) as Hex; } +function buildCreateProxyData(): Hex { + return SAFE_FACTORY_INTERFACE.encodeFunctionData('createProxy', [ + ZERO_ADDRESS, + '0', + ZERO_ADDRESS, + { + r: CREATE_PROXY_R, + s: CREATE_PROXY_S, + v: 27, + }, + ]) as Hex; +} + +function buildExecTransactionData(): Hex { + return SAFE_INTERFACE.encodeFunctionData('execTransaction', [ + SAFE_TX_TARGET, + '0', + SAFE_TX_DATA, + 0, + 0, + 0, + 0, + ZERO_ADDRESS, + ZERO_ADDRESS, + SAFE_SIGNATURE_BYTES, + ]) as Hex; +} + function getRequestBody(): { actions: unknown[] } { const [, options] = jest.mocked(successfulFetch).mock.calls[0]; @@ -390,7 +432,7 @@ describe('Across Quotes', () => { expect(getRequestBody().actions).toStrictEqual([]); }); - it('uses predict deposit post-swap action for token transfer transactions', async () => { + it('uses direct recipient for predict transfer-only transactions', async () => { const transferData = buildTransferData(TRANSFER_RECIPIENT); successfulFetchMock.mockResolvedValue({ @@ -413,30 +455,141 @@ describe('Across Quotes', () => { const [url] = successfulFetchMock.mock.calls[0]; const params = new URL(url as string).searchParams; - expect(params.get('recipient')).toBe(FROM_MOCK); + expect(params.get('recipient')).toBe(TRANSFER_RECIPIENT.toLowerCase()); + expect(getRequestBody().actions).toStrictEqual([]); + }); + + it('uses direct recipient for nested predict transfer-only transactions', async () => { + const transferData = buildTransferData(TRANSFER_RECIPIENT); + + successfulFetchMock.mockResolvedValue({ + json: async () => QUOTE_MOCK, + } as Response); + + await getAcrossQuotes({ + messenger, + requests: [QUOTE_REQUEST_MOCK], + transaction: { + ...TRANSACTION_META_MOCK, + type: TransactionType.predictDeposit, + nestedTransactions: [{ data: transferData }], + } as TransactionMeta, + }); + + const [url] = successfulFetchMock.mock.calls[0]; + const params = new URL(url as string).searchParams; + + expect(params.get('recipient')).toBe(TRANSFER_RECIPIENT.toLowerCase()); + expect(getRequestBody().actions).toStrictEqual([]); + }); + + it('uses decoded actions for supported setup transactions', async () => { + const createProxyData = buildCreateProxyData(); + const execTransactionData = buildExecTransactionData(); + const transferData = buildTransferData(TRANSFER_RECIPIENT, 0); + + successfulFetchMock.mockResolvedValue({ + json: async () => QUOTE_MOCK, + } as Response); + + await getAcrossQuotes({ + messenger, + requests: [QUOTE_REQUEST_MOCK], + transaction: { + ...TRANSACTION_META_MOCK, + nestedTransactions: [ + { to: FACTORY_ADDRESS, data: createProxyData }, + { to: SAFE_ADDRESS, data: execTransactionData }, + { to: QUOTE_REQUEST_MOCK.targetTokenAddress, data: transferData }, + ], + } as TransactionMeta, + }); + + const [url] = successfulFetchMock.mock.calls[0]; + const params = new URL(url as string).searchParams; + + expect(params.get('recipient')).toBe(TRANSFER_RECIPIENT.toLowerCase()); expect(getRequestBody().actions).toStrictEqual([ { args: [ { populateDynamically: false, - value: TRANSFER_RECIPIENT.toLowerCase(), + value: ZERO_ADDRESS, }, { - balanceSourceToken: QUOTE_REQUEST_MOCK.targetTokenAddress, - populateDynamically: true, + populateDynamically: false, value: '0', }, + { + populateDynamically: false, + value: ZERO_ADDRESS, + }, + { + populateDynamically: false, + value: ['27', CREATE_PROXY_R, CREATE_PROXY_S], + }, ], - functionSignature: 'function transfer(address to, uint256 value)', + functionSignature: + 'function createProxy(address paymentToken, uint256 payment, address payable paymentReceiver, (uint8 v, bytes32 r, bytes32 s) createSig)', isNativeTransfer: false, - target: QUOTE_REQUEST_MOCK.targetTokenAddress, + target: FACTORY_ADDRESS, + value: '0', + }, + { + args: [ + { + populateDynamically: false, + value: SAFE_TX_TARGET.toLowerCase(), + }, + { + populateDynamically: false, + value: '0', + }, + { + populateDynamically: false, + value: SAFE_TX_DATA, + }, + { + populateDynamically: false, + value: '0', + }, + { + populateDynamically: false, + value: '0', + }, + { + populateDynamically: false, + value: '0', + }, + { + populateDynamically: false, + value: '0', + }, + { + populateDynamically: false, + value: ZERO_ADDRESS, + }, + { + populateDynamically: false, + value: ZERO_ADDRESS, + }, + { + populateDynamically: false, + value: SAFE_SIGNATURE_BYTES, + }, + ], + functionSignature: + 'function execTransaction(address to, uint256 value, bytes data, uint8 operation, uint256 safeTxGas, uint256 baseGas, uint256 gasPrice, address gasToken, address refundReceiver, bytes signatures)', + isNativeTransfer: false, + target: SAFE_ADDRESS, value: '0', }, ]); }); - it('uses predict deposit post-swap action from nested transfer transactions', async () => { - const transferData = buildTransferData(TRANSFER_RECIPIENT); + it('falls back to target token address for setup transfer actions without an explicit target', async () => { + const createProxyData = buildCreateProxyData(); + const transferData = buildTransferData(TRANSFER_RECIPIENT, 0); successfulFetchMock.mockResolvedValue({ json: async () => QUOTE_MOCK, @@ -447,21 +600,68 @@ describe('Across Quotes', () => { requests: [QUOTE_REQUEST_MOCK], transaction: { ...TRANSACTION_META_MOCK, - type: TransactionType.predictDeposit, - nestedTransactions: [{ data: transferData }], + nestedTransactions: [ + { to: FACTORY_ADDRESS, data: createProxyData }, + { data: transferData }, + ], } as TransactionMeta, }); + const body = getRequestBody(); const [url] = successfulFetchMock.mock.calls[0]; const params = new URL(url as string).searchParams; - expect(params.get('recipient')).toBe(FROM_MOCK); + expect(params.get('recipient')).toBe(TRANSFER_RECIPIENT.toLowerCase()); + expect(body.actions).toHaveLength(1); + expect(body.actions[0]).toMatchObject({ + functionSignature: + 'function createProxy(address paymentToken, uint256 payment, address payable paymentReceiver, (uint8 v, bytes32 r, bytes32 s) createSig)', + target: FACTORY_ADDRESS, + }); + }); + + it('uses the first transfer in a batch as recipient and keeps later transfers as post-swap actions', async () => { + const firstTransferRecipient = + '0x1111111111111111111111111111111111111111' as Hex; + const secondTransferRecipient = + '0x2222222222222222222222222222222222222222' as Hex; + const firstTransferData = buildTransferData(firstTransferRecipient, 0); + const secondTransferData = buildTransferData(secondTransferRecipient, 0); + + successfulFetchMock.mockResolvedValue({ + json: async () => QUOTE_MOCK, + } as Response); + + await getAcrossQuotes({ + messenger, + requests: [QUOTE_REQUEST_MOCK], + transaction: { + ...TRANSACTION_META_MOCK, + nestedTransactions: [ + { + to: QUOTE_REQUEST_MOCK.targetTokenAddress, + data: firstTransferData, + }, + { + to: QUOTE_REQUEST_MOCK.targetTokenAddress, + data: secondTransferData, + }, + ], + } as TransactionMeta, + }); + + const [url] = successfulFetchMock.mock.calls[0]; + const params = new URL(url as string).searchParams; + + expect(params.get('recipient')).toBe( + firstTransferRecipient.toLowerCase(), + ); expect(getRequestBody().actions).toStrictEqual([ { args: [ { populateDynamically: false, - value: TRANSFER_RECIPIENT.toLowerCase(), + value: secondTransferRecipient.toLowerCase(), }, { balanceSourceToken: QUOTE_REQUEST_MOCK.targetTokenAddress, @@ -477,6 +677,137 @@ describe('Across Quotes', () => { ]); }); + it('uses from address when no destination calldata exists', async () => { + successfulFetchMock.mockResolvedValue({ + json: async () => QUOTE_MOCK, + } as Response); + + await getAcrossQuotes({ + messenger, + requests: [QUOTE_REQUEST_MOCK], + transaction: TRANSACTION_META_MOCK, + }); + + const [url] = successfulFetchMock.mock.calls[0]; + const params = new URL(url as string).searchParams; + + expect(params.get('recipient')).toBe(FROM_MOCK); + expect(getRequestBody().actions).toStrictEqual([]); + }); + + it('falls back to top-level setup calldata when no nested calldata exists', async () => { + const createProxyData = buildCreateProxyData(); + + successfulFetchMock.mockResolvedValue({ + json: async () => QUOTE_MOCK, + } as Response); + + await getAcrossQuotes({ + messenger, + requests: [QUOTE_REQUEST_MOCK], + transaction: { + ...TRANSACTION_META_MOCK, + txParams: { + from: FROM_MOCK, + to: FACTORY_ADDRESS, + data: createProxyData, + }, + }, + }); + + const [url] = successfulFetchMock.mock.calls[0]; + const params = new URL(url as string).searchParams; + + expect(params.get('recipient')).toBe(FROM_MOCK); + expect(getRequestBody().actions).toStrictEqual([ + { + args: [ + { + populateDynamically: false, + value: ZERO_ADDRESS, + }, + { + populateDynamically: false, + value: '0', + }, + { + populateDynamically: false, + value: ZERO_ADDRESS, + }, + { + populateDynamically: false, + value: ['27', CREATE_PROXY_R, CREATE_PROXY_S], + }, + ], + functionSignature: + 'function createProxy(address paymentToken, uint256 payment, address payable paymentReceiver, (uint8 v, bytes32 r, bytes32 s) createSig)', + isNativeTransfer: false, + target: FACTORY_ADDRESS, + value: '0', + }, + ]); + }); + + it('throws for unsupported setup calldata', async () => { + successfulFetchMock.mockResolvedValue({ + json: async () => QUOTE_MOCK, + } as Response); + + await expect( + getAcrossQuotes({ + messenger, + requests: [QUOTE_REQUEST_MOCK], + transaction: { + ...TRANSACTION_META_MOCK, + nestedTransactions: [ + { + to: FACTORY_ADDRESS, + data: '0xdeadbeef' as Hex, + }, + ], + } as TransactionMeta, + }), + ).rejects.toThrow(/Destination selector: 0xdeadbeef/u); + }); + + it('throws when createProxy calldata is missing target', async () => { + const createProxyData = buildCreateProxyData(); + + successfulFetchMock.mockResolvedValue({ + json: async () => QUOTE_MOCK, + } as Response); + + await expect( + getAcrossQuotes({ + messenger, + requests: [QUOTE_REQUEST_MOCK], + transaction: { + ...TRANSACTION_META_MOCK, + nestedTransactions: [{ data: createProxyData }], + } as TransactionMeta, + }), + ).rejects.toThrow(/Across only supports direct token transfers/u); + }); + + it('throws when execTransaction calldata is missing target', async () => { + const execTransactionData = buildExecTransactionData(); + + successfulFetchMock.mockResolvedValue({ + json: async () => QUOTE_MOCK, + } as Response); + + await expect( + getAcrossQuotes({ + messenger, + requests: [QUOTE_REQUEST_MOCK], + transaction: { + ...TRANSACTION_META_MOCK, + nestedTransactions: [{ data: execTransactionData }], + } as TransactionMeta, + }), + ).rejects.toThrow(/Across only supports direct token transfers/u); + }); + it('throws when destination flow is not transfer-style', async () => { await expect( getAcrossQuotes({ @@ -490,7 +821,7 @@ describe('Across Quotes', () => { }, }, }), - ).rejects.toThrow(/Across only supports transfer-style/u); + ).rejects.toThrow(/Across only supports direct token transfers/u); }); it('throws when txParams include authorization list', async () => { @@ -1252,33 +1583,30 @@ describe('Across Quotes', () => { expect(params.get('recipient')).toBe(FROM_MOCK); }); - it('uses nested transaction transfer recipient when available', async () => { + it('throws when nested transactions mix a transfer with unsupported calldata', async () => { const transferData = buildTransferData(TRANSFER_RECIPIENT); successfulFetchMock.mockResolvedValue({ json: async () => QUOTE_MOCK, } as Response); - await getAcrossQuotes({ - messenger, - requests: [QUOTE_REQUEST_MOCK], - transaction: { - ...TRANSACTION_META_MOCK, - nestedTransactions: [ - { data: transferData }, - { data: '0xbeef' as Hex }, - ], - txParams: { - from: FROM_MOCK, - data: '0xabc' as Hex, - }, - } as TransactionMeta, - }); - - const [url] = successfulFetchMock.mock.calls[0]; - const params = new URL(url as string).searchParams; - - expect(params.get('recipient')).toBe(TRANSFER_RECIPIENT.toLowerCase()); + await expect( + getAcrossQuotes({ + messenger, + requests: [QUOTE_REQUEST_MOCK], + transaction: { + ...TRANSACTION_META_MOCK, + nestedTransactions: [ + { data: transferData }, + { data: '0xbeef' as Hex }, + ], + txParams: { + from: FROM_MOCK, + data: '0xabc' as Hex, + }, + } as TransactionMeta, + }), + ).rejects.toThrow(/Across only supports direct token transfers/u); }); it('uses txParams data when single nested transaction has no data', async () => { @@ -1299,7 +1627,7 @@ describe('Across Quotes', () => { }, } as TransactionMeta, }), - ).rejects.toThrow(/Across only supports transfer-style/u); + ).rejects.toThrow(/Destination selector: 0xdeadbeef/u); }); it('omits slippage param when slippage is undefined', async () => { @@ -1358,33 +1686,30 @@ describe('Across Quotes', () => { expect(result).toHaveLength(1); }); - it('extracts recipient from token transfer in nested transactions array', async () => { + it('throws when nested transactions include unsupported calldata before a transfer', async () => { const transferData = buildTransferData(TRANSFER_RECIPIENT); successfulFetchMock.mockResolvedValue({ json: async () => QUOTE_MOCK, } as Response); - await getAcrossQuotes({ - messenger, - requests: [QUOTE_REQUEST_MOCK], - transaction: { - ...TRANSACTION_META_MOCK, - nestedTransactions: [ - { data: '0xother' as Hex }, - { data: transferData }, - ], - txParams: { - from: FROM_MOCK, - data: '0xnonTransferData' as Hex, - }, - } as TransactionMeta, - }); - - const [url] = successfulFetchMock.mock.calls[0]; - const params = new URL(url as string).searchParams; - - expect(params.get('recipient')).toBe(TRANSFER_RECIPIENT.toLowerCase()); + await expect( + getAcrossQuotes({ + messenger, + requests: [QUOTE_REQUEST_MOCK], + transaction: { + ...TRANSACTION_META_MOCK, + nestedTransactions: [ + { data: '0xother' as Hex }, + { data: transferData }, + ], + txParams: { + from: FROM_MOCK, + data: '0xnonTransferData' as Hex, + }, + } as TransactionMeta, + }), + ).rejects.toThrow(/Across only supports direct token transfers/u); }); it('handles nested transactions with undefined data', async () => { @@ -1434,7 +1759,7 @@ describe('Across Quotes', () => { }, } as TransactionMeta, }), - ).rejects.toThrow(/Across only supports transfer-style/u); + ).rejects.toThrow(/Destination selector: 0xdeadbeef/u); }); }); }); diff --git a/packages/transaction-pay-controller/src/strategy/across/across-quotes.ts b/packages/transaction-pay-controller/src/strategy/across/across-quotes.ts index 8de9e76ef73..713d0a316f8 100644 --- a/packages/transaction-pay-controller/src/strategy/across/across-quotes.ts +++ b/packages/transaction-pay-controller/src/strategy/across/across-quotes.ts @@ -1,11 +1,9 @@ -import { Interface } from '@ethersproject/abi'; import { successfulFetch, toHex } from '@metamask/controller-utils'; -import { TransactionType } from '@metamask/transaction-controller'; -import type { TransactionMeta } from '@metamask/transaction-controller'; import type { Hex } from '@metamask/utils'; import { createModuleLogger } from '@metamask/utils'; import { BigNumber } from 'bignumber.js'; +import { getAcrossDestination } from './across-actions'; import { getAcrossOrderedTransactions } from './transactions'; import type { AcrossAction, @@ -29,26 +27,14 @@ import { getPayStrategiesConfig, getSlippage } from '../../utils/feature-flags'; import { calculateGasCost } from '../../utils/gas'; import { estimateQuoteGasLimits } from '../../utils/quote-gas'; import { getTokenFiatRate } from '../../utils/token'; -import { TOKEN_TRANSFER_FOUR_BYTE } from '../relay/constants'; const log = createModuleLogger(projectLogger, 'across-strategy'); -const TOKEN_TRANSFER_INTERFACE = new Interface([ - 'function transfer(address to, uint256 value)', -]); - const UNSUPPORTED_AUTHORIZATION_LIST_ERROR = 'Across does not support type-4/EIP-7702 authorization lists yet'; -const UNSUPPORTED_DESTINATION_ERROR = - 'Across only supports transfer-style destination flows at the moment'; type AcrossQuoteWithoutMetaMask = Omit; -type AcrossDestination = { - actions: AcrossAction[]; - recipient: Hex; -}; - /** * Fetch Across quotes. * @@ -200,99 +186,6 @@ async function requestAcrossApproval( return (await response.json()) as AcrossSwapApprovalResponse; } -function getAcrossDestination( - transaction: TransactionMeta, - request: QuoteRequest, -): AcrossDestination { - const { txParams } = transaction; - const { from } = request; - const transferData = getTransferData(transaction); - - if (transferData) { - const transferRecipient = getTransferRecipient(transferData); - - if (transaction.type === TransactionType.predictDeposit) { - return { - actions: [buildAcrossTransferAction(transferRecipient, request)], - recipient: from, - }; - } - - return { - actions: [], - recipient: transferRecipient, - }; - } - - const data = txParams?.data as Hex | undefined; - const hasNoData = data === undefined || data === '0x'; - const nestedCalldata = getNestedCalldata(transaction); - - if (hasNoData && nestedCalldata.length === 0) { - return { - actions: [], - recipient: from, - }; - } - - throw new Error(UNSUPPORTED_DESTINATION_ERROR); -} - -function buildAcrossTransferAction( - transferRecipient: Hex, - request: QuoteRequest, -): AcrossAction { - return { - args: [ - { - populateDynamically: false, - value: transferRecipient, - }, - { - balanceSourceToken: request.targetTokenAddress, - populateDynamically: true, - value: '0', - }, - ], - functionSignature: 'function transfer(address to, uint256 value)', - isNativeTransfer: false, - target: request.targetTokenAddress, - value: '0', - }; -} - -function getTransferData(transaction: TransactionMeta): Hex | undefined { - const { nestedTransactions, txParams } = transaction; - - const nestedTransferData = nestedTransactions?.find( - (nestedTx: { data?: Hex }) => - nestedTx.data?.startsWith(TOKEN_TRANSFER_FOUR_BYTE), - )?.data; - - const data = txParams?.data as Hex | undefined; - const tokenTransferData = data?.startsWith(TOKEN_TRANSFER_FOUR_BYTE) - ? data - : undefined; - - return nestedTransferData ?? tokenTransferData; -} - -function getNestedCalldata(transaction: TransactionMeta): Hex[] { - return (transaction.nestedTransactions ?? []) - .map((nestedTx: { data?: Hex }) => nestedTx.data) - .filter( - (data: Hex | undefined): data is Hex => - data !== undefined && data !== '0x', - ); -} - -function getTransferRecipient(data: Hex): Hex { - return TOKEN_TRANSFER_INTERFACE.decodeFunctionData( - 'transfer', - data, - ).to.toLowerCase() as Hex; -} - async function normalizeQuote( original: AcrossQuoteWithoutMetaMask, request: QuoteRequest,