From ecc8343e429cdba8471acb3af881c19b3959f7ad Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Thu, 12 Dec 2024 22:47:49 +0000 Subject: [PATCH] chore(runway): cherry-pick fix: mark transactions as failed for cancelled / unknown smart transactions (#12664) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Resolves an issue with stuck transactions for some users who used smart transactions, which was caused by having [this PR](https://github.com/MetaMask/metamask-mobile/pull/12274) in the 7.36.0 release even when it was marked for 7.37.0 by metamaskbot. ## **Related issues** Fixes: ## **Manual testing steps** 1. Cancelled transaction stuck thanks to super low gas settings 2. All other submitted transactions afterwards as well with the invalid_nonce error. 3. Upgrade to 7.37.1 and see all cancelled tx 4. Try a new tx ## **Screenshots/Recordings** ### **Before** https://consensys.slack.com/archives/C084S32G337/p1734034013370979 ### **After** https://consensys.slack.com/archives/C084S32G337/p1734039388196979 ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- app/store/migrations/063.test.ts | 297 +++++++++++++++++++++++++++++++ app/store/migrations/063.ts | 92 ++++++++++ app/store/migrations/index.ts | 2 + 3 files changed, 391 insertions(+) create mode 100644 app/store/migrations/063.test.ts create mode 100644 app/store/migrations/063.ts diff --git a/app/store/migrations/063.test.ts b/app/store/migrations/063.test.ts new file mode 100644 index 00000000000..2cee3b36f8c --- /dev/null +++ b/app/store/migrations/063.test.ts @@ -0,0 +1,297 @@ +import migrate from './063'; +import { merge } from 'lodash'; +import { captureException } from '@sentry/react-native'; +import initialRootState from '../../util/test/initial-root-state'; +import { SmartTransactionStatuses } from '@metamask/smart-transactions-controller/dist/types'; +import { TransactionStatus, CHAIN_IDS } from '@metamask/transaction-controller'; + +const expectedState = { + engine: { + backgroundState: { + TransactionController: { + transactions: [ + { + chainId: CHAIN_IDS.MAINNET, + id: '1', + origin: 'test.com', + status: TransactionStatus.confirmed, + time: 1631714312, + txParams: { + from: '0x1', + }, + hash: '0x2', + rawTx: '0x3', + }, + { + chainId: CHAIN_IDS.LINEA_MAINNET, + id: '2', + origin: 'test.com', + status: TransactionStatus.confirmed, + time: 1631714312, + txParams: { + from: '0x1', + }, + hash: '0x3', + }, + { + chainId: CHAIN_IDS.MAINNET, + id: '3', + origin: 'test2.com', + status: TransactionStatus.failed, + time: 1631714313, + txParams: { + from: '0x6', + }, + hash: '0x4', + rawTx: '0x5', + error: { + name: 'SmartTransactionCancelled', + message: + 'Smart transaction cancelled. Previous status: submitted', + }, + }, + { + chainId: CHAIN_IDS.MAINNET, + id: '4', + origin: 'test2.com', + status: TransactionStatus.failed, + time: 1631714313, + txParams: { + from: '0x6', + }, + hash: '0x5', + rawTx: '0x6', + error: { + name: 'SmartTransactionCancelled', + message: 'Smart transaction cancelled. Previous status: signed', + }, + }, + { + chainId: CHAIN_IDS.MAINNET, + id: '5', + origin: 'test2.com', + status: TransactionStatus.failed, + time: 1631714313, + txParams: { + from: '0x6', + }, + hash: '0x6', + rawTx: '0x7', + error: { + name: 'SmartTransactionCancelled', + message: 'Smart transaction cancelled. Previous status: signed', + }, + }, + ], + }, + SmartTransactionsController: { + smartTransactionsState: { + smartTransactions: { + [CHAIN_IDS.MAINNET]: [ + { + txHash: '0x2', + status: SmartTransactionStatuses.SUCCESS, + }, + { + txHash: '0x4', + status: SmartTransactionStatuses.CANCELLED, + }, + { + txHash: '0x5', + status: SmartTransactionStatuses.CANCELLED, + }, + { + txHash: '0x6', + status: SmartTransactionStatuses.UNKNOWN, + }, + ], + }, + }, + }, + }, + }, +}; + +jest.mock('@sentry/react-native', () => ({ + captureException: jest.fn(), +})); +const mockedCaptureException = jest.mocked(captureException); + +describe('Migration #63', () => { + beforeEach(() => { + jest.restoreAllMocks(); + jest.resetAllMocks(); + }); + + const invalidStates = [ + { + state: null, + errorMessage: "FATAL ERROR: Migration 63: Invalid state error: 'object'", + scenario: 'state is invalid', + }, + { + state: merge({}, initialRootState, { + engine: null, + }), + errorMessage: + "FATAL ERROR: Migration 63: Invalid engine state error: 'object'", + scenario: 'engine state is invalid', + }, + { + state: merge({}, initialRootState, { + engine: { + backgroundState: null, + }, + }), + errorMessage: + "FATAL ERROR: Migration 63: Invalid engine backgroundState error: 'object'", + scenario: 'backgroundState is invalid', + }, + { + state: merge({}, initialRootState, { + engine: { + backgroundState: { TransactionController: null }, + }, + }), + errorMessage: "Migration 63: Invalid TransactionController state: 'null'", + scenario: 'transactionController is invalid', + }, + { + state: merge({}, initialRootState, { + engine: { + backgroundState: { SmartTransactionsController: null }, + }, + }), + errorMessage: + "Migration 63: Invalid SmartTransactionsController state: 'null'", + scenario: 'smartTransactionsController is invalid', + }, + { + state: merge({}, initialRootState, { + engine: { + backgroundState: { + SmartTransactionsController: { + smartTransactionsState: { smartTransactions: null }, + }, + }, + }, + }), + errorMessage: + "Migration 63: Missing smart transactions property from SmartTransactionsController: 'object'", + scenario: + 'smartTransactionsController.smartTransactionsState.smartTransactions is invalid', + }, + ]; + it.each(invalidStates)( + 'captures exception if $scenario', + ({ errorMessage, state }) => { + const newState = migrate(state); + + expect(newState).toStrictEqual(state); + expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error)); + expect(mockedCaptureException.mock.calls[0][0].message).toBe( + errorMessage, + ); + }, + ); + + it('applies migration, changes transaction status to failed if a smart transaction was cancelled or unknown', () => { + const oldState = { + engine: { + backgroundState: { + TransactionController: { + transactions: [ + { + chainId: CHAIN_IDS.MAINNET, + id: '1', + origin: 'test.com', + status: TransactionStatus.confirmed, + time: 1631714312, + txParams: { + from: '0x1', + }, + hash: '0x2', + rawTx: '0x3', + }, + { + chainId: CHAIN_IDS.LINEA_MAINNET, + id: '2', + origin: 'test.com', + status: TransactionStatus.confirmed, + time: 1631714312, + txParams: { + from: '0x1', + }, + hash: '0x3', + }, + { + chainId: CHAIN_IDS.MAINNET, + id: '3', + origin: 'test2.com', + status: TransactionStatus.submitted, + time: 1631714313, + txParams: { + from: '0x6', + }, + hash: '0x4', + rawTx: '0x5', + }, + { + chainId: CHAIN_IDS.MAINNET, + id: '4', + origin: 'test2.com', + status: TransactionStatus.signed, + time: 1631714313, + txParams: { + from: '0x6', + }, + hash: '0x5', + rawTx: '0x6', + }, + { + chainId: CHAIN_IDS.MAINNET, + id: '5', + origin: 'test2.com', + status: TransactionStatus.signed, + time: 1631714313, + txParams: { + from: '0x6', + }, + hash: '0x6', + rawTx: '0x7', + }, + ], + }, + SmartTransactionsController: { + smartTransactionsState: { + smartTransactions: { + [CHAIN_IDS.MAINNET]: [ + { + txHash: '0x2', + status: SmartTransactionStatuses.SUCCESS, + }, + { + txHash: '0x4', + status: SmartTransactionStatuses.CANCELLED, + }, + { + txHash: '0x5', + status: SmartTransactionStatuses.CANCELLED, + }, + { + txHash: '0x6', + status: SmartTransactionStatuses.UNKNOWN, + }, + ], + }, + }, + }, + }, + }, + }; + + const newState = migrate(oldState); + + expect(newState).toStrictEqual(expectedState); + }); +}); diff --git a/app/store/migrations/063.ts b/app/store/migrations/063.ts new file mode 100644 index 00000000000..e2f6166ac0d --- /dev/null +++ b/app/store/migrations/063.ts @@ -0,0 +1,92 @@ +import { isObject } from '@metamask/utils'; +import { captureException } from '@sentry/react-native'; +import { ensureValidState } from './util'; +import { SmartTransactionStatuses } from '@metamask/smart-transactions-controller/dist/types'; +import { TransactionStatus, CHAIN_IDS } from '@metamask/transaction-controller'; + +export default function migrate(state: unknown) { + if (!ensureValidState(state, 63)) { + return state; + } + + if (!isObject(state.engine.backgroundState.TransactionController)) { + captureException( + new Error( + `Migration 63: Invalid TransactionController state: '${state.engine.backgroundState.TransactionController}'`, + ), + ); + return state; + } + + if (!isObject(state.engine.backgroundState.SmartTransactionsController)) { + captureException( + new Error( + `Migration 63: Invalid SmartTransactionsController state: '${state.engine.backgroundState.SmartTransactionsController}'`, + ), + ); + return state; + } + + const transactionControllerState = + state.engine.backgroundState.TransactionController; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const smartTransactionsControllerState = state.engine.backgroundState.SmartTransactionsController as any; + + if (!Array.isArray(transactionControllerState.transactions)) { + captureException( + new Error( + `Migration 63: Missing transactions property from TransactionController: '${typeof state + .engine.backgroundState.TransactionController}'`, + ), + ); + return state; + } + const smartTransactions = + smartTransactionsControllerState?.smartTransactionsState?.smartTransactions; + if (!isObject(smartTransactions)) { + captureException( + new Error( + `Migration 63: Missing smart transactions property from SmartTransactionsController: '${typeof state + .engine.backgroundState.SmartTransactionsController?.smartTransactionsState}'`, + ), + ); + return state; + } + + const ethereumMainnetSmartTransactions = smartTransactions[CHAIN_IDS.MAINNET]; + if ( + !Array.isArray(ethereumMainnetSmartTransactions) || + ethereumMainnetSmartTransactions.length === 0 + ) { + // If there are no smart transactions, we can skip this migration. + return state; + } + + const smartTransactionTxHashesForUpdate: Record = {}; + ethereumMainnetSmartTransactions.forEach((smartTransaction) => { + if ( + smartTransaction.txHash && + (smartTransaction.status === SmartTransactionStatuses.CANCELLED || + smartTransaction.status === SmartTransactionStatuses.UNKNOWN) + ) { + smartTransactionTxHashesForUpdate[smartTransaction.txHash.toLowerCase()] = true; + } + }); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + transactionControllerState.transactions.forEach((transaction: any) => { + if (!transaction.hash || transaction.status === TransactionStatus.failed) { + return; + } + const previousStatus = transaction.status; + if (smartTransactionTxHashesForUpdate[transaction.hash.toLowerCase()]) { + transaction.status = TransactionStatus.failed; + transaction.error = { + name: 'SmartTransactionCancelled', + message: `Smart transaction cancelled. Previous status: ${previousStatus}`, + }; + } + }); + + return state; +} diff --git a/app/store/migrations/index.ts b/app/store/migrations/index.ts index 4a8d11ad47c..06f50319276 100644 --- a/app/store/migrations/index.ts +++ b/app/store/migrations/index.ts @@ -63,6 +63,7 @@ import migration59 from './059'; import migration60 from './060'; import migration61 from './061'; import migration62 from './062'; +import migration63 from './063'; type MigrationFunction = (state: unknown) => unknown; type AsyncMigrationFunction = (state: unknown) => Promise; @@ -138,6 +139,7 @@ export const migrationList: MigrationsList = { 60: migration60, 61: migration61, 62: migration62, + 63: migration63, }; // Enable both synchronous and asynchronous migrations