diff --git a/app/components/Nav/Main/RootRPCMethodsUI.js b/app/components/Nav/Main/RootRPCMethodsUI.js index 3a46a71eb46..1a563dd9444 100644 --- a/app/components/Nav/Main/RootRPCMethodsUI.js +++ b/app/components/Nav/Main/RootRPCMethodsUI.js @@ -68,6 +68,7 @@ import { STX_NO_HASH_ERROR } from '../../../util/smart-transactions/smart-publis import { getSmartTransactionMetricsProperties } from '../../../util/smart-transactions'; import { cloneDeep, isEqual } from 'lodash'; import { selectSwapsTransactions } from '../../../selectors/transactionController'; +import { updateSwapsTransaction } from '../../../util/swaps/swaps-transactions'; ///: BEGIN:ONLY_INCLUDE_IF(preinstalled-snaps,external-snaps) import InstallSnapApproval from '../../Approvals/InstallSnapApproval'; @@ -86,15 +87,12 @@ export const useSwapConfirmedEvent = ({ trackSwaps }) => { const [transactionMetaIdsForListening, setTransactionMetaIdsForListening] = useState([]); - const addTransactionMetaIdForListening = useCallback( - (txMetaId) => { - setTransactionMetaIdsForListening([ - ...transactionMetaIdsForListening, - txMetaId, - ]); - }, - [transactionMetaIdsForListening], - ); + const addTransactionMetaIdForListening = useCallback((txMetaId) => { + setTransactionMetaIdsForListening((transactionMetaIdsForListening) => [ + ...transactionMetaIdsForListening, + txMetaId, + ]); + }, []); const swapsTransactions = useSwapsTransactions(); useEffect(() => { @@ -144,8 +142,8 @@ const RootRPCMethodsUI = (props) => { try { const { TransactionController, SmartTransactionsController } = Engine.context; - const newSwapsTransactions = swapsTransactions; - const swapTransaction = newSwapsTransactions[transactionMeta.id]; + const swapTransaction = swapsTransactions[transactionMeta.id]; + const { sentAt, gasEstimate, @@ -187,15 +185,6 @@ const RootRPCMethodsUI = (props) => { ethBalance, ); - newSwapsTransactions[transactionMeta.id].gasUsed = receipt.gasUsed; - if (tokensReceived) { - newSwapsTransactions[transactionMeta.id].receivedDestinationAmount = - new BigNumber(tokensReceived, 16).toString(10); - } - TransactionController.update((state) => { - state.swapsTransactions = newSwapsTransactions; - }); - const timeToMine = currentBlock.timestamp - sentAt; const estimatedVsUsedGasRatio = `${new BigNumber(receipt.gasUsed) .div(gasEstimate) @@ -218,8 +207,20 @@ const RootRPCMethodsUI = (props) => { ...swapTransaction.analytics, account_type: getAddressAccountType(transactionMeta.txParams.from), }; - delete newSwapsTransactions[transactionMeta.id].analytics; - delete newSwapsTransactions[transactionMeta.id].paramsForAnalytics; + + updateSwapsTransaction(transactionMeta.id, (swapsTransaction) => { + swapsTransaction.gasUsed = receipt.gasUsed; + + if (tokensReceived) { + swapsTransaction.receivedDestinationAmount = new BigNumber( + tokensReceived, + 16, + ).toString(10); + } + + delete swapsTransaction.analytics; + delete swapsTransaction.paramsForAnalytics; + }); const smartTransactionMetricsProperties = getSmartTransactionMetricsProperties( @@ -237,6 +238,8 @@ const RootRPCMethodsUI = (props) => { ...smartTransactionMetricsProperties, }; + Logger.log('Swaps', 'Sending metrics event', event); + trackEvent(event, { sensitiveProperties: { ...parameters } }); } catch (e) { Logger.error(e, MetaMetricsEvents.SWAP_TRACKING_FAILED); diff --git a/app/components/UI/Swaps/QuotesView.js b/app/components/UI/Swaps/QuotesView.js index 0ac70c6f606..ca0b8a17e90 100644 --- a/app/components/UI/Swaps/QuotesView.js +++ b/app/components/UI/Swaps/QuotesView.js @@ -113,7 +113,9 @@ import trackErrorAsAnalytics from '../../../util/metrics/TrackError/trackErrorAs import { selectGasFeeEstimates } from '../../../selectors/confirmTransaction'; import { selectShouldUseSmartTransaction } from '../../../selectors/smartTransactionsController'; import { selectGasFeeControllerEstimateType } from '../../../selectors/gasFeeController'; +import { addSwapsTransaction } from '../../../util/swaps/swaps-transactions'; +const LOG_PREFIX = 'Swaps'; const POLLING_INTERVAL = 30000; const SLIPPAGE_BUCKETS = { MEDIUM: AppConstants.GAS_OPTIONS.MEDIUM, @@ -792,19 +794,15 @@ function SwapsQuotesView({ ]); const updateSwapsTransactions = useCallback( - async ( - transactionMeta, - approvalTransactionMetaId, - newSwapsTransactions, - ) => { - const { TransactionController } = Engine.context; + async (transactionMeta, approvalTransactionMetaId) => { const ethQuery = Engine.getGlobalEthQuery(); const blockNumber = await query(ethQuery, 'blockNumber', []); const currentBlock = await query(ethQuery, 'getBlockByNumber', [ blockNumber, false, ]); - newSwapsTransactions[transactionMeta.id] = { + + addSwapsTransaction(transactionMeta.id, { action: 'swap', sourceToken: { address: sourceToken.address, @@ -851,9 +849,6 @@ function SwapsQuotesView({ ethAccountBalance: accounts[selectedAddress].balance, approvalTransactionMetaId, }, - }; - TransactionController.update((state) => { - state.swapsTransactions = newSwapsTransactions; }); }, [ @@ -922,7 +917,7 @@ function SwapsQuotesView({ ); const handleSwapTransaction = useCallback( - async (newSwapsTransactions, approvalTransactionMetaId) => { + async (approvalTransactionMetaId) => { if (!selectedQuote) { return; } @@ -944,19 +939,23 @@ function SwapsQuotesView({ }, ); + Logger.log(LOG_PREFIX, 'Added trade transaction', transactionMeta.id); + await result; - updateSwapsTransactions( - transactionMeta, - approvalTransactionMetaId, - newSwapsTransactions, + Logger.log( + LOG_PREFIX, + 'Submitted trade transaction', + transactionMeta.id, ); + updateSwapsTransactions(transactionMeta, approvalTransactionMetaId); + setRecipient(selectedAddress); await addTokenToAssetsController(destinationToken); await addTokenToAssetsController(sourceToken); } catch (e) { - // send analytics + Logger.log(LOG_PREFIX, 'Failed to submit trade transaction', e); } }, [ @@ -973,13 +972,8 @@ function SwapsQuotesView({ ], ); - const handleApprovaltransaction = useCallback( - async ( - TransactionController, - newSwapsTransactions, - approvalTransactionMetaId, - isHardwareAddress, - ) => { + const handleApprovalTransaction = useCallback( + async (isHardwareAddress) => { try { resetTransaction(); const { transactionMeta, result } = await addTransaction( @@ -996,11 +990,25 @@ function SwapsQuotesView({ }, ); + Logger.log( + LOG_PREFIX, + 'Added approval transaction', + transactionMeta.id, + ); + await result; + + Logger.log( + LOG_PREFIX, + 'Submitted approval transaction', + transactionMeta.id, + ); + setRecipient(selectedAddress); - approvalTransactionMetaId = transactionMeta.id; - newSwapsTransactions[transactionMeta.id] = { + const approvalTransactionMetaId = transactionMeta.id; + + addSwapsTransaction(transactionMeta.id, { action: 'approval', sourceToken: { address: sourceToken.address, @@ -1011,7 +1019,8 @@ function SwapsQuotesView({ decodeApproveData(approvalTransaction.data).encodedAmount, 16, ).toString(10), - }; + }); + if (isHardwareAddress || shouldUseSmartTransaction) { const { id: transactionId } = transactionMeta; @@ -1019,19 +1028,16 @@ function SwapsQuotesView({ 'TransactionController:transactionConfirmed', (transactionMeta) => { if (transactionMeta.status === TransactionStatus.confirmed) { - handleSwapTransaction( - TransactionController, - newSwapsTransactions, - approvalTransactionMetaId, - isHardwareAddress, - ); + handleSwapTransaction(approvalTransactionMetaId); } }, (transactionMeta) => transactionMeta.id === transactionId, ); } + + return approvalTransactionMetaId; } catch (e) { - // send analytics + Logger.log(LOG_PREFIX, 'Failed to submit approval transaction', e); } }, [ @@ -1057,16 +1063,10 @@ function SwapsQuotesView({ startSwapAnalytics(selectedQuote, selectedAddress); - const { TransactionController } = Engine.context; - - const newSwapsTransactions = - TransactionController.state.swapsTransactions || {}; let approvalTransactionMetaId; + if (approvalTransaction) { - await handleApprovaltransaction( - TransactionController, - newSwapsTransactions, - approvalTransactionMetaId, + approvalTransactionMetaId = await handleApprovalTransaction( isHardwareAddress, ); @@ -1080,12 +1080,7 @@ function SwapsQuotesView({ !shouldUseSmartTransaction || (shouldUseSmartTransaction && !approvalTransaction) ) { - await handleSwapTransaction( - TransactionController, - newSwapsTransactions, - approvalTransactionMetaId, - isHardwareAddress, - ); + await handleSwapTransaction(approvalTransactionMetaId); } navigation.dangerouslyGetParent()?.pop(); @@ -1094,7 +1089,7 @@ function SwapsQuotesView({ selectedAddress, approvalTransaction, startSwapAnalytics, - handleApprovaltransaction, + handleApprovalTransaction, handleSwapTransaction, navigation, shouldUseSmartTransaction, diff --git a/app/core/Engine.ts b/app/core/Engine.ts index 5472614ea87..a181ad0b9c1 100644 --- a/app/core/Engine.ts +++ b/app/core/Engine.ts @@ -1826,6 +1826,7 @@ class Engine { transactions: [], lastFetchedBlockNumbers: {}, submitHistory: [], + swapsTransactions: {}, })); LoggingController.clear(); diff --git a/app/selectors/smartTransactionsController.ts b/app/selectors/smartTransactionsController.ts index ae411e226b7..bddae93947c 100644 --- a/app/selectors/smartTransactionsController.ts +++ b/app/selectors/smartTransactionsController.ts @@ -78,7 +78,7 @@ export const selectPendingSmartTransactionsBySender = (state: RootState) => { // stx.uuid is one from sentinel API, not the same as tx.id which is generated client side // Doesn't matter too much because we only care about the pending stx, confirmed txs are handled like normal // However, this does make it impossible to read Swap data from TxController.swapsTransactions as that relies on client side tx.id - // To fix that we do transactionController.update({ swapsTransactions: newSwapsTransactions }) in app/util/smart-transactions/smart-tx.ts + // To fix that we create a duplicate swaps transaction for the stx.uuid in the smart publish hook. id: stx.uuid, status: stx.status?.startsWith(SmartTransactionStatuses.CANCELLED) ? SmartTransactionStatuses.CANCELLED diff --git a/app/util/smart-transactions/smart-publish-hook.test.ts b/app/util/smart-transactions/smart-publish-hook.test.ts index 75b99ac11da..c93cb77afe2 100644 --- a/app/util/smart-transactions/smart-publish-hook.test.ts +++ b/app/util/smart-transactions/smart-publish-hook.test.ts @@ -34,6 +34,14 @@ jest.mock('uuid', () => ({ v1: jest.fn(() => 'approvalId'), })); +jest.mock('../../core/Engine', () => ({ + context: { + TransactionController: { + update: jest.fn(), + }, + }, +})); + const addressFrom = '0xabce7847fd3661a9b7c86aaf1daea08d9da5750e'; const transactionHash = '0x0302b75dfb9fd9eb34056af031efcaee2a8cbd799ea054a85966165cd82a7356'; @@ -505,8 +513,6 @@ describe('submitSmartTransactionHook', () => { isSwapTransaction: true, }, }); - //@ts-expect-error - We are calling a protected method for testing purposes - expect(request.transactionController.update).toHaveBeenCalledTimes(1); }); }); }); diff --git a/app/util/smart-transactions/smart-publish-hook.ts b/app/util/smart-transactions/smart-publish-hook.ts index 15b15a922cd..21aed7ac663 100644 --- a/app/util/smart-transactions/smart-publish-hook.ts +++ b/app/util/smart-transactions/smart-publish-hook.ts @@ -21,6 +21,7 @@ import { v1 as random } from 'uuid'; import { decimalToHex } from '../conversions'; import { ApprovalTypes } from '../../core/RPCMethods/RPCMethodMiddleware'; import { RAMPS_SEND } from '../../components/UI/Ramp/constants'; +import { addSwapsTransaction } from '../swaps/swaps-transactions'; export declare type Hex = `0x${string}`; @@ -427,17 +428,16 @@ class SmartTransactionHook { this.#approvalEnded = true; }; - #updateSwapsTransactions = (id: string) => { + #updateSwapsTransactions = (uuid: string) => { // We do this so we can show the Swap data (e.g. ETH to USDC, fiat values) in the app/components/Views/TransactionsView/index.js - const newSwapsTransactions = + const swapsTransactions = // @ts-expect-error This is not defined on the type, but is a field added in app/components/UI/Swaps/QuotesView.js this.#transactionController.state.swapsTransactions || {}; - newSwapsTransactions[id] = newSwapsTransactions[this.#transactionMeta.id]; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (this.#transactionController as any).update((state: any) => { - state.swapsTransactions = newSwapsTransactions; - }); + const originalSwapsTransaction = + swapsTransactions[this.#transactionMeta.id]; + + addSwapsTransaction(uuid, originalSwapsTransaction); }; } diff --git a/app/util/swaps/swaps-transactions.test.ts b/app/util/swaps/swaps-transactions.test.ts new file mode 100644 index 00000000000..9e47dbe3f68 --- /dev/null +++ b/app/util/swaps/swaps-transactions.test.ts @@ -0,0 +1,106 @@ +import Engine from '../../core/Engine'; +import { TransactionController } from '@metamask/transaction-controller'; +import { + addSwapsTransaction, + updateSwapsTransaction, +} from './swaps-transactions'; + +jest.mock('../../core/Engine', () => ({ + context: { + TransactionController: { + update: jest.fn(), + }, + }, +})); + +const TRANSACTION_ID_MOCK = 'testTransactionId'; + +const TRANSACTION_MOCK = { + test: 'value', +}; + +describe('Swaps Transactions Utils', () => { + let transactionControllerMock: jest.Mocked; + + beforeEach(() => { + jest.resetAllMocks(); + + transactionControllerMock = Engine.context + .TransactionController as jest.Mocked; + }); + + function mockUpdate(originalState = {}) { + const state = originalState; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (transactionControllerMock as any).update.mockImplementationOnce( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (callback: any) => callback(state), + ); + + return state; + } + + describe('addSwapsTransaction', () => { + it('adds data to TransactionController state', () => { + const state = mockUpdate(); + + addSwapsTransaction(TRANSACTION_ID_MOCK, TRANSACTION_MOCK); + + expect(state).toStrictEqual({ + swapsTransactions: { + [TRANSACTION_ID_MOCK]: TRANSACTION_MOCK, + }, + }); + }); + + it('handles missing swaps transaction data', () => { + const state = mockUpdate(undefined); + + addSwapsTransaction(TRANSACTION_ID_MOCK, TRANSACTION_MOCK); + + expect(state).toStrictEqual({ + swapsTransactions: { + [TRANSACTION_ID_MOCK]: TRANSACTION_MOCK, + }, + }); + }); + }); + + describe('updateSwapsTransaction', () => { + it('updates data in TransactionController state', () => { + const state = mockUpdate({ + swapsTransactions: { + [TRANSACTION_ID_MOCK]: TRANSACTION_MOCK, + }, + }); + + updateSwapsTransaction(TRANSACTION_ID_MOCK, (transaction) => { + transaction.test2 = 'updatedValue'; + }); + + expect(state).toStrictEqual({ + swapsTransactions: { + [TRANSACTION_ID_MOCK]: { + ...TRANSACTION_MOCK, + test2: 'updatedValue', + }, + }, + }); + }); + + it('throws if no existing data found', () => { + mockUpdate({ + swapsTransactions: { + invalidId: TRANSACTION_MOCK, + }, + }); + + expect(() => + updateSwapsTransaction(TRANSACTION_ID_MOCK, (transaction) => { + transaction.test2 = 'updatedValue'; + }), + ).toThrow('Swaps transaction not found - testTransactionId'); + }); + }); +}); diff --git a/app/util/swaps/swaps-transactions.ts b/app/util/swaps/swaps-transactions.ts new file mode 100644 index 00000000000..7d64dad1c4c --- /dev/null +++ b/app/util/swaps/swaps-transactions.ts @@ -0,0 +1,45 @@ +import Engine from '../../core/Engine'; +import Logger from '../Logger'; + +const LOG_PREFIX = 'Swaps Transactions'; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type SwapsTransaction = Record; + +export function addSwapsTransaction( + transactionId: string, + data: SwapsTransaction, +) { + const { TransactionController } = Engine.context; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (TransactionController as any).update((state: SwapsTransaction) => { + if (!state.swapsTransactions) { + state.swapsTransactions = {}; + } + + state.swapsTransactions[transactionId] = data; + }); + + Logger.log(LOG_PREFIX, 'Added transaction', transactionId); +} + +export function updateSwapsTransaction( + transactionId: string, + callback: (transaction: SwapsTransaction) => void, +) { + const { TransactionController } = Engine.context; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (TransactionController as any).update((state: SwapsTransaction) => { + const existingData = state.swapsTransactions?.[transactionId]; + + if (!existingData) { + throw new Error(`Swaps transaction not found - ${transactionId}`); + } + + callback(existingData); + }); + + Logger.log(LOG_PREFIX, 'Updated transaction', transactionId); +}