Skip to content

Commit

Permalink
chore(runway): cherry-pick fix: small refactoring of the latest migra…
Browse files Browse the repository at this point in the history
…tion script + add a new migration case (#12698)

- fix: small refactoring of the latest migration script + add a new
migration case (#12694)

## **Description**
Little follow up on this one:
#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**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **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](82c500f)

Co-authored-by: Daniel <[email protected]>
  • Loading branch information
runway-github[bot] and dan437 authored Dec 13, 2024
1 parent 0b4ddfd commit 57972e1
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 36 deletions.
47 changes: 39 additions & 8 deletions app/store/migrations/063.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,6 @@ const expectedState = {
},
hash: '0x4',
rawTx: '0x5',
error: {
name: 'SmartTransactionCancelled',
message:
'Smart transaction cancelled. Previous status: submitted',
},
},
{
chainId: CHAIN_IDS.MAINNET,
Expand All @@ -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',
},
},
{
Expand All @@ -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: {
Expand All @@ -104,6 +115,10 @@ const expectedState = {
txHash: '0x6',
status: SmartTransactionStatuses.UNKNOWN,
},
{
txHash: '0x7',
status: SmartTransactionStatuses.RESOLVED,
},
],
},
},
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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: {
Expand All @@ -282,6 +309,10 @@ describe('Migration #63', () => {
txHash: '0x6',
status: SmartTransactionStatuses.UNKNOWN,
},
{
txHash: '0x7',
status: SmartTransactionStatuses.RESOLVED,
},
],
},
},
Expand Down
69 changes: 41 additions & 28 deletions app/store/migrations/063.ts
Original file line number Diff line number Diff line change
@@ -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<string, boolean> = {};
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',
Expand Down

0 comments on commit 57972e1

Please sign in to comment.