From c60b605af77fd34c4544e6cf3f88067eb8ed6200 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 18 Oct 2023 13:55:34 +0100 Subject: [PATCH] fix: transactions stuck in submitted status [Release Branch] (#7532) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** _Identical to #7519 but targeting the release branch due to additional patch changes not yet being released._ Resolve a bug causing some new transactions to be stuck in the `submitted` status as they are rejected by the network due to `nonce` values that are too high. This is caused by incorrect callbacks in the `NonceTracker` in the `TransactionController`, resulting in correct `nonce` values being skipped because a local confirmed transaction (with the same address but a different chain ID) has the same `nonce`. The solution is to update the callbacks used by the `NonceTracker` to filter transactions to the current chain only. ## **Manual testing steps** 1. Create a new account. 2. Send some Goerli to the account. 3. Send some Sepolia to the account. 4. Send a transaction on Goerli using the new account. 5. Send a transaction on Sepolia using the new account. 6. Verify both are successful and both using the same nonce. ## **Related issues** Fixes [#1304](https://github.com/MetaMask/mobile-planning/issues/1304) ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained: - [x] What problem this PR is solving. - [x] How this problem was solved. - [x] How reviewers can test my changes. - [x] I’ve indicated what issue this PR is linked to: Fixes #??? - [x] I’ve included tests if applicable. - [x] I’ve documented any added code. - [x] 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)). - [x] I’ve properly set the pull request status: - [x] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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. --- ...etamask+transaction-controller+5.0.0.patch | 54 ++++++++++++++++--- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/patches/@metamask+transaction-controller+5.0.0.patch b/patches/@metamask+transaction-controller+5.0.0.patch index dd0bed45d64..f6c14e48973 100644 --- a/patches/@metamask+transaction-controller+5.0.0.patch +++ b/patches/@metamask+transaction-controller+5.0.0.patch @@ -366,7 +366,7 @@ index 0000000..8da9369 +//# sourceMappingURL=IncomingTransactionHelper.js.map \ No newline at end of file diff --git a/node_modules/@metamask/transaction-controller/dist/TransactionController.d.ts b/node_modules/@metamask/transaction-controller/dist/TransactionController.d.ts -index e242aed..d315c6c 100644 +index e242aed..7efd89a 100644 --- a/node_modules/@metamask/transaction-controller/dist/TransactionController.d.ts +++ b/node_modules/@metamask/transaction-controller/dist/TransactionController.d.ts @@ -2,8 +2,10 @@ @@ -688,7 +688,7 @@ index e242aed..d315c6c 100644 /** * Trim the amount of transactions that are set on the state. Checks * if the length of the tx history is longer then desired persistence -@@ -413,59 +272,36 @@ export declare class TransactionController extends BaseController (0, utils_1.getAndFormatTransactionsForNonceTracker)(address, TransactionStatus.submitted, this.state.transactions), - getConfirmedTransactions: (address) => (0, utils_1.getAndFormatTransactionsForNonceTracker)(address, TransactionStatus.confirmed, this.state.transactions), -+ getPendingTransactions: (address) => (0, utils_1.getAndFormatTransactionsForNonceTracker)(address, types_1.TransactionStatus.submitted, this.state.transactions), -+ getConfirmedTransactions: (address) => (0, utils_1.getAndFormatTransactionsForNonceTracker)(address, types_1.TransactionStatus.confirmed, this.state.transactions), ++ getPendingTransactions: this.getNonceTrackerTransactions.bind(this, types_1.TransactionStatus.submitted), ++ getConfirmedTransactions: this.getNonceTrackerTransactions.bind(this, types_1.TransactionStatus.confirmed), + }); + this.incomingTransactionHelper = new IncomingTransactionHelper_1.IncomingTransactionHelper({ + blockTracker, @@ -1303,7 +1304,7 @@ index 814a899..411b310 100644 this.hub.emit(`${meta.id}:confirmed`, meta); return [meta, true]; } -@@ -856,88 +644,205 @@ class TransactionController extends base_controller_1.BaseController { +@@ -856,88 +644,209 @@ class TransactionController extends base_controller_1.BaseController { return Number(txReceipt.status) === 0; }); } @@ -1571,6 +1572,10 @@ index 814a899..411b310 100644 + onUpdatedLastFetchedBlockNumbers({ lastFetchedBlockNumbers, blockNumber, }) { + this.update({ lastFetchedBlockNumbers }); + this.hub.emit('incomingTransactionBlock', blockNumber); ++ } ++ getNonceTrackerTransactions(status, address) { ++ const { chainId: currentChainId } = this.getNetworkState().providerConfig; ++ return (0, utils_1.getAndFormatTransactionsForNonceTracker)(currentChainId, address, status, this.state.transactions); } } exports.TransactionController = TransactionController; @@ -2864,7 +2869,7 @@ index 0000000..1cddb49 +//# sourceMappingURL=types.js.map \ No newline at end of file diff --git a/node_modules/@metamask/transaction-controller/dist/utils.d.ts b/node_modules/@metamask/transaction-controller/dist/utils.d.ts -index ccf3362..895cdf1 100644 +index ccf3362..301b1b5 100644 --- a/node_modules/@metamask/transaction-controller/dist/utils.d.ts +++ b/node_modules/@metamask/transaction-controller/dist/utils.d.ts @@ -1,6 +1,6 @@ @@ -2897,6 +2902,20 @@ index ccf3362..895cdf1 100644 export declare const validateGasValues: (gasValues: GasPriceValue | FeeMarketEIP1559Values) => void; export declare const isFeeMarketEIP1559Values: (gasValues?: GasPriceValue | FeeMarketEIP1559Values | undefined) => gasValues is FeeMarketEIP1559Values; export declare const isGasPriceValue: (gasValues?: GasPriceValue | FeeMarketEIP1559Values | undefined) => gasValues is GasPriceValue; +@@ -63,10 +49,11 @@ export declare function validateMinimumIncrease(proposed: string, min: string): + /** + * Helper function to filter and format transactions for the nonce tracker. + * ++ * @param currentChainId - Chain ID of the current network. + * @param fromAddress - Address of the account from which the transactions to filter from are sent. + * @param transactionStatus - Status of the transactions for which to filter. + * @param transactions - Array of transactionMeta objects that have been prefiltered. + * @returns Array of transactions formatted for the nonce tracker. + */ +-export declare function getAndFormatTransactionsForNonceTracker(fromAddress: string, transactionStatus: TransactionStatus, transactions: TransactionMeta[]): NonceTrackerTransaction[]; ++export declare function getAndFormatTransactionsForNonceTracker(currentChainId: string, fromAddress: string, transactionStatus: TransactionStatus, transactions: TransactionMeta[]): NonceTrackerTransaction[]; + //# sourceMappingURL=utils.d.ts.map +\ No newline at end of file diff --git a/node_modules/@metamask/transaction-controller/dist/utils.d.ts.map b/node_modules/@metamask/transaction-controller/dist/utils.d.ts.map deleted file mode 100644 index 9d4045c..0000000 @@ -2906,7 +2925,7 @@ index 9d4045c..0000000 -{"version":3,"file":"utils.d.ts","sourceRoot":"","sources":["../src/utils.ts"],"names":[],"mappings":"AAOA,OAAO,KAAK,EAAE,WAAW,IAAI,uBAAuB,EAAE,MAAM,iCAAiC,CAAC;AAC9F,OAAO,EACL,WAAW,EACX,eAAe,EACf,aAAa,EACb,sBAAsB,EACtB,iBAAiB,EAClB,MAAM,yBAAyB,CAAC;AACjC,OAAO,KAAK,EAAE,eAAe,EAAE,MAAM,yBAAyB,CAAC;AAE/D,eAAO,MAAM,kBAAkB,qCAAqC,CAAC;AAiBrE;;;;;;GAMG;AACH,wBAAgB,kBAAkB,CAChC,WAAW,EAAE,MAAM,EACnB,SAAS,EAAE,GAAG,GACb,MAAM,CAeR;AAED;;;;;GAKG;AACH,wBAAgB,oBAAoB,CAAC,WAAW,EAAE,WAAW,eAS5D;AAED;;;;;GAKG;AACH,wBAAgB,mBAAmB,CAAC,WAAW,EAAE,WAAW,QAmD3D;AAED;;;;;;GAMG;AACH,eAAO,MAAM,oBAAoB,gBAAiB,WAAW,KAAG,OAO/D,CAAC;AAEF;;;;;;;;GAQG;AACH,wBAAsB,sBAAsB,CAC1C,WAAW,EAAE,MAAM,EACnB,OAAO,EAAE,MAAM,EACf,cAAc,EAAE,MAAM,EACtB,GAAG,CAAC,EAAE,eAAe,GACpB,OAAO,CAAC,CAAC;IAAE,CAAC,MAAM,EAAE,MAAM,GAAG,EAAE,CAAA;CAAE,EAAE;IAAE,CAAC,MAAM,EAAE,MAAM,GAAG,EAAE,CAAA;CAAE,CAAC,CAAC,CA8C/D;AAED,eAAO,MAAM,iBAAiB,cACjB,aAAa,GAAG,sBAAsB,SAUlD,CAAC;AAEF,eAAO,MAAM,wBAAwB,yGAIsC,CAAC;AAE5E,eAAO,MAAM,eAAe,gGAG0B,CAAC;AAEvD,eAAO,MAAM,oBAAoB,UAAW,MAAM,QAAQ,MAAM,KAAG,MACF,CAAC;AAElE,eAAO,MAAM,6BAA6B,UACjC,MAAM,GAAG,SAAS,QACnB,MAAM,KACX,MAEF,CAAC;AAEF;;;;;;;GAOG;AACH,wBAAgB,uBAAuB,CAAC,QAAQ,EAAE,MAAM,EAAE,GAAG,EAAE,MAAM,UAQpE;AAED;;;;;;;GAOG;AACH,wBAAgB,uCAAuC,CACrD,WAAW,EAAE,MAAM,EACnB,iBAAiB,EAAE,iBAAiB,EACpC,YAAY,EAAE,eAAe,EAAE,GAC9B,uBAAuB,EAAE,CAsB3B"} \ No newline at end of file diff --git a/node_modules/@metamask/transaction-controller/dist/utils.js b/node_modules/@metamask/transaction-controller/dist/utils.js -index 179a564..ce955ba 100644 +index 179a564..37403ab 100644 --- a/node_modules/@metamask/transaction-controller/dist/utils.js +++ b/node_modules/@metamask/transaction-controller/dist/utils.js @@ -1,15 +1,6 @@ @@ -2985,6 +3004,25 @@ index 179a564..ce955ba 100644 const validateGasValues = (gasValues) => { Object.keys(gasValues).forEach((key) => { const value = gasValues[key]; +@@ -206,14 +154,16 @@ exports.validateMinimumIncrease = validateMinimumIncrease; + /** + * Helper function to filter and format transactions for the nonce tracker. + * ++ * @param currentChainId - Chain ID of the current network. + * @param fromAddress - Address of the account from which the transactions to filter from are sent. + * @param transactionStatus - Status of the transactions for which to filter. + * @param transactions - Array of transactionMeta objects that have been prefiltered. + * @returns Array of transactions formatted for the nonce tracker. + */ +-function getAndFormatTransactionsForNonceTracker(fromAddress, transactionStatus, transactions) { ++function getAndFormatTransactionsForNonceTracker(currentChainId, fromAddress, transactionStatus, transactions) { + return transactions +- .filter(({ status, transaction: { from } }) => status === transactionStatus && ++ .filter(({ chainId, status, transaction: { from } }) => chainId === currentChainId && ++ status === transactionStatus && + from.toLowerCase() === fromAddress.toLowerCase()) + .map(({ status, transaction: { from, gas, value, nonce } }) => { + // the only value we care about is the nonce diff --git a/node_modules/@metamask/transaction-controller/dist/utils.js.map b/node_modules/@metamask/transaction-controller/dist/utils.js.map deleted file mode 100644 index 8f2f6e3..0000000