From 57972e1c215b4844d5a4ea2b618464d93690b8ae Mon Sep 17 00:00:00 2001 From: "runway-github[bot]" <73448015+runway-github[bot]@users.noreply.github.com> Date: Fri, 13 Dec 2024 10:08:30 -0700 Subject: [PATCH] chore(runway): cherry-pick fix: small refactoring of the latest migration script + add a new migration case (#12698) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - fix: small refactoring of the latest migration script + add a new migration case (#12694) ## **Description** Little follow up on this one: https://github.com/MetaMask/metamask-mobile/pull/12664 Main thing is to also mark a regular transaction as `failed` if the STX status was `resolved`. It's very rare (like the `unknown` state), but we want to handle such case as well. ## **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** ### **After** ## **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. [82c500f](https://github.com/MetaMask/metamask-mobile/commit/82c500f02c5daa8dc0dd12fb454421f275ed6239) Co-authored-by: Daniel <80175477+dan437@users.noreply.github.com> --- app/store/migrations/063.test.ts | 47 ++++++++++++++++++---- app/store/migrations/063.ts | 69 +++++++++++++++++++------------- 2 files changed, 80 insertions(+), 36 deletions(-) diff --git a/app/store/migrations/063.test.ts b/app/store/migrations/063.test.ts index 2cee3b36f8c..c57e1163b1d 100644 --- a/app/store/migrations/063.test.ts +++ b/app/store/migrations/063.test.ts @@ -44,11 +44,6 @@ const expectedState = { }, hash: '0x4', rawTx: '0x5', - error: { - name: 'SmartTransactionCancelled', - message: - 'Smart transaction cancelled. Previous status: submitted', - }, }, { chainId: CHAIN_IDS.MAINNET, @@ -63,7 +58,7 @@ const expectedState = { rawTx: '0x6', error: { name: 'SmartTransactionCancelled', - message: 'Smart transaction cancelled. Previous status: signed', + message: 'Smart transaction cancelled. Previous status: submitted', }, }, { @@ -82,6 +77,22 @@ const expectedState = { message: 'Smart transaction cancelled. Previous status: signed', }, }, + { + chainId: CHAIN_IDS.MAINNET, + id: '6', + origin: 'test2.com', + status: TransactionStatus.failed, + time: 1631714313, + txParams: { + from: '0x6', + }, + hash: '0x7', + rawTx: '0x8', + error: { + name: 'SmartTransactionCancelled', + message: 'Smart transaction cancelled. Previous status: signed', + }, + }, ], }, SmartTransactionsController: { @@ -104,6 +115,10 @@ const expectedState = { txHash: '0x6', status: SmartTransactionStatuses.UNKNOWN, }, + { + txHash: '0x7', + status: SmartTransactionStatuses.RESOLVED, + }, ], }, }, @@ -228,7 +243,7 @@ describe('Migration #63', () => { chainId: CHAIN_IDS.MAINNET, id: '3', origin: 'test2.com', - status: TransactionStatus.submitted, + status: TransactionStatus.failed, time: 1631714313, txParams: { from: '0x6', @@ -240,7 +255,7 @@ describe('Migration #63', () => { chainId: CHAIN_IDS.MAINNET, id: '4', origin: 'test2.com', - status: TransactionStatus.signed, + status: TransactionStatus.submitted, time: 1631714313, txParams: { from: '0x6', @@ -260,6 +275,18 @@ describe('Migration #63', () => { hash: '0x6', rawTx: '0x7', }, + { + chainId: CHAIN_IDS.MAINNET, + id: '6', + origin: 'test2.com', + status: TransactionStatus.signed, + time: 1631714313, + txParams: { + from: '0x6', + }, + hash: '0x7', + rawTx: '0x8', + }, ], }, SmartTransactionsController: { @@ -282,6 +309,10 @@ describe('Migration #63', () => { txHash: '0x6', status: SmartTransactionStatuses.UNKNOWN, }, + { + txHash: '0x7', + status: SmartTransactionStatuses.RESOLVED, + }, ], }, }, diff --git a/app/store/migrations/063.ts b/app/store/migrations/063.ts index e2f6166ac0d..5d453db3cc2 100644 --- a/app/store/migrations/063.ts +++ b/app/store/migrations/063.ts @@ -1,85 +1,98 @@ 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 { SmartTransactionStatuses, type SmartTransaction } from '@metamask/smart-transactions-controller/dist/types'; import { TransactionStatus, CHAIN_IDS } from '@metamask/transaction-controller'; +const migrationVersion = 63; + +interface SmartTransactionsState { + smartTransactions: { + [chainId: string]: SmartTransaction[]; + }; +} + export default function migrate(state: unknown) { - if (!ensureValidState(state, 63)) { + if (!ensureValidState(state, migrationVersion)) { return state; } - if (!isObject(state.engine.backgroundState.TransactionController)) { + const transactionControllerState = state.engine.backgroundState.TransactionController; + const smartTransactionsControllerState = state.engine.backgroundState.SmartTransactionsController; + + if (!isObject(transactionControllerState)) { captureException( new Error( - `Migration 63: Invalid TransactionController state: '${state.engine.backgroundState.TransactionController}'`, + `Migration ${migrationVersion}: Invalid TransactionController state: '${transactionControllerState}'`, ), ); return state; } - if (!isObject(state.engine.backgroundState.SmartTransactionsController)) { + if (!isObject(smartTransactionsControllerState)) { captureException( new Error( - `Migration 63: Invalid SmartTransactionsController state: '${state.engine.backgroundState.SmartTransactionsController}'`, + `Migration ${migrationVersion}: Invalid SmartTransactionsController state: '${smartTransactionsControllerState}'`, ), ); 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 + `Migration ${migrationVersion}: Missing transactions property from TransactionController: '${typeof state .engine.backgroundState.TransactionController}'`, ), ); return state; } - const smartTransactions = - smartTransactionsControllerState?.smartTransactionsState?.smartTransactions; + const smartTransactions = (smartTransactionsControllerState?.smartTransactionsState as SmartTransactionsState)?.smartTransactions; if (!isObject(smartTransactions)) { captureException( new Error( - `Migration 63: Missing smart transactions property from SmartTransactionsController: '${typeof state - .engine.backgroundState.SmartTransactionsController?.smartTransactionsState}'`, + `Migration ${migrationVersion}: Missing smart transactions property from SmartTransactionsController: '${typeof smartTransactionsControllerState?.smartTransactionsState}'`, ), ); return state; } const ethereumMainnetSmartTransactions = smartTransactions[CHAIN_IDS.MAINNET]; + + // If there are no smart transactions, we can skip this migration. 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; - } - }); + const smartTransactionStatusesForUpdate: SmartTransactionStatuses[] = [ + SmartTransactionStatuses.CANCELLED, + SmartTransactionStatuses.UNKNOWN, + SmartTransactionStatuses.RESOLVED, + ]; + + // Create a Set of transaction hashes for quick lookup. + const smartTransactionTxHashesForUpdate = new Set( + ethereumMainnetSmartTransactions + .filter( + (smartTransaction) => + smartTransaction.txHash && + smartTransaction.status && + smartTransactionStatusesForUpdate.includes(smartTransaction.status as SmartTransactionStatuses), + ) + .map((smartTransaction) => smartTransaction.txHash?.toLowerCase()), + ); + // Update transactions based on the Set. // 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()]) { + if (smartTransactionTxHashesForUpdate.has(transaction.hash.toLowerCase())) { transaction.status = TransactionStatus.failed; transaction.error = { name: 'SmartTransactionCancelled',