From 2820a1e90cd1ff82131c8ef5fc372e736630f1ad Mon Sep 17 00:00:00 2001 From: lwin Date: Tue, 18 Nov 2025 18:08:00 +0800 Subject: [PATCH 1/5] feat: include 'rawTxHex' in logTransaction requests --- packages/shield-controller/src/ShieldController.ts | 6 ++++++ packages/shield-controller/src/backend.ts | 2 ++ packages/shield-controller/src/types.ts | 1 + 3 files changed, 9 insertions(+) diff --git a/packages/shield-controller/src/ShieldController.ts b/packages/shield-controller/src/ShieldController.ts index 8a422b1b548..83d33f1cd2d 100644 --- a/packages/shield-controller/src/ShieldController.ts +++ b/packages/shield-controller/src/ShieldController.ts @@ -446,11 +446,17 @@ export class ShieldController extends BaseController< throw new Error('Transaction hash not found'); } + const rawTransactionHex = txMeta.rawTx; + if (!rawTransactionHex) { + throw new Error('Raw transaction hex not found'); + } + const { status } = this.#getCoverageStatus(txMeta.id); await this.#backend.logTransaction({ txMeta, transactionHash, + rawTransactionHex, status, }); } diff --git a/packages/shield-controller/src/backend.ts b/packages/shield-controller/src/backend.ts index 0db6e677ef2..cd94bc910a7 100644 --- a/packages/shield-controller/src/backend.ts +++ b/packages/shield-controller/src/backend.ts @@ -177,8 +177,10 @@ export class ShieldRemoteBackend implements ShieldBackend { async logTransaction(req: LogTransactionRequest): Promise { const initBody = makeInitCoverageCheckBody(req.txMeta); + const body = { transactionHash: req.transactionHash, + rawTransactionHex: req.rawTransactionHex, status: req.status, ...initBody, }; diff --git a/packages/shield-controller/src/types.ts b/packages/shield-controller/src/types.ts index 1c9a911c691..7f2d32fb183 100644 --- a/packages/shield-controller/src/types.ts +++ b/packages/shield-controller/src/types.ts @@ -23,6 +23,7 @@ export type LogSignatureRequest = { export type LogTransactionRequest = { txMeta: TransactionMeta; transactionHash: string; + rawTransactionHex: string; status: string; }; From e5182fad3e0a6632b38a839b1535cafc1e756453 Mon Sep 17 00:00:00 2001 From: lwin Date: Tue, 18 Nov 2025 18:08:09 +0800 Subject: [PATCH 2/5] test: updated tests --- .../shield-controller/src/ShieldController.test.ts | 13 +++++++++++++ packages/shield-controller/src/backend.test.ts | 2 ++ packages/shield-controller/tests/utils.ts | 1 + 3 files changed, 16 insertions(+) diff --git a/packages/shield-controller/src/ShieldController.test.ts b/packages/shield-controller/src/ShieldController.test.ts index d25e15cb221..91e1904c60f 100644 --- a/packages/shield-controller/src/ShieldController.test.ts +++ b/packages/shield-controller/src/ShieldController.test.ts @@ -461,6 +461,7 @@ describe('ShieldController', () => { txMeta: updatedTxMeta, status: 'shown', transactionHash: '0x00', + rawTransactionHex: '0xdeadbeef', }); }); @@ -478,6 +479,7 @@ describe('ShieldController', () => { expect(components.backend.logTransaction).toHaveBeenCalledWith({ status: 'not_shown', transactionHash: '0x00', + rawTransactionHex: '0xdeadbeef', txMeta: updatedTxMeta, }); }); @@ -492,6 +494,17 @@ describe('ShieldController', () => { // Check that backend was not called expect(components.backend.logTransaction).not.toHaveBeenCalled(); }); + + it('does not log when raw transaction hex is missing', async () => { + const components = setup(); + + await runTest(components, { + updateTransaction: (txMeta) => delete txMeta.rawTx, + }); + + // Check that backend was not called + expect(components.backend.logTransaction).not.toHaveBeenCalled(); + }); }); describe('metadata', () => { diff --git a/packages/shield-controller/src/backend.test.ts b/packages/shield-controller/src/backend.test.ts index b5777f6bbc7..820986bc948 100644 --- a/packages/shield-controller/src/backend.test.ts +++ b/packages/shield-controller/src/backend.test.ts @@ -362,6 +362,7 @@ describe('ShieldRemoteBackend', () => { await backend.logTransaction({ txMeta: generateMockTxMeta(), transactionHash: '0x00', + rawTransactionHex: '0xdeadbeef', status: 'shown', }); expect(fetchMock).toHaveBeenCalledTimes(1); @@ -377,6 +378,7 @@ describe('ShieldRemoteBackend', () => { backend.logTransaction({ txMeta: generateMockTxMeta(), transactionHash: '0x00', + rawTransactionHex: '0xdeadbeef', status: 'shown', }), ).rejects.toThrow('Failed to log transaction: 500'); diff --git a/packages/shield-controller/tests/utils.ts b/packages/shield-controller/tests/utils.ts index 70f00b0be8d..5243d32804c 100644 --- a/packages/shield-controller/tests/utils.ts +++ b/packages/shield-controller/tests/utils.ts @@ -34,6 +34,7 @@ export function generateMockTxMeta(): TransactionMeta { type: TransactionType.contractInteraction, origin: 'https://metamask.io', submittedTime: Date.now(), + rawTx: '0xdeadbeef', }; } From 14f5fa129beb5048c4d2019e53b476e8b1b15e54 Mon Sep 17 00:00:00 2001 From: lwin Date: Tue, 18 Nov 2025 18:10:09 +0800 Subject: [PATCH 3/5] chore: updated ChangeLog --- packages/shield-controller/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/shield-controller/CHANGELOG.md b/packages/shield-controller/CHANGELOG.md index 30fa896757b..bb32ac5b73e 100644 --- a/packages/shield-controller/CHANGELOG.md +++ b/packages/shield-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Added `transactionMeta.rawTx` to the `logTransaction` request body. ([#7178](https://github.com/MetaMask/core/pull/7178)) + ## [2.1.0] ### Added From 974556b89cc56783eab9df162eb6e126e985cb1e Mon Sep 17 00:00:00 2001 From: lwin Date: Tue, 18 Nov 2025 18:39:43 +0800 Subject: [PATCH 4/5] fix: skipped coverage call for transaction submitted event --- packages/shield-controller/src/ShieldController.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/shield-controller/src/ShieldController.ts b/packages/shield-controller/src/ShieldController.ts index 83d33f1cd2d..0c9a55f5f0e 100644 --- a/packages/shield-controller/src/ShieldController.ts +++ b/packages/shield-controller/src/ShieldController.ts @@ -289,15 +289,18 @@ export class ShieldController extends BaseController< // only check if the previous transaction has simulation data and if it has changed // this is to avoid checking coverage for the `TWICE` (once when it's added to the state and once when it's simulated for the first time). // we only need to update the coverage result when the simulation data has changed. - Boolean(previousTransaction?.simulationData) && + previousTransaction?.simulationData && !isEqual( transaction.simulationData, - previousTransaction?.simulationData, + previousTransaction.simulationData, ); // Check coverage if the transaction is new or if the simulation data has // changed. - if (!previousTransaction || simulationDataChanged) { + if ( + transaction.status !== TransactionStatus.submitted && // we don't need to check coverage for submitted transactions + (!previousTransaction || simulationDataChanged) + ) { this.checkCoverage(transaction).catch( // istanbul ignore next (error) => log('Error checking coverage:', error), From 5c33a25a2d2555ed80dbe441b59d463c95344265 Mon Sep 17 00:00:00 2001 From: lwin Date: Tue, 18 Nov 2025 19:21:48 +0800 Subject: [PATCH 5/5] feat: skipped checkCoverage when tx is submitted or confirmed --- packages/shield-controller/CHANGELOG.md | 4 ++ .../shield-controller/src/ShieldController.ts | 41 ++++++++++--------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/packages/shield-controller/CHANGELOG.md b/packages/shield-controller/CHANGELOG.md index bb32ac5b73e..a995521c9fe 100644 --- a/packages/shield-controller/CHANGELOG.md +++ b/packages/shield-controller/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added `transactionMeta.rawTx` to the `logTransaction` request body. ([#7178](https://github.com/MetaMask/core/pull/7178)) +### Changed + +- Skipped transaction coverage check when the transaction is `submitted` or `confirmed`. ([#7178](https://github.com/MetaMask/core/pull/7178)) + ## [2.1.0] ### Added diff --git a/packages/shield-controller/src/ShieldController.ts b/packages/shield-controller/src/ShieldController.ts index 0c9a55f5f0e..296df877717 100644 --- a/packages/shield-controller/src/ShieldController.ts +++ b/packages/shield-controller/src/ShieldController.ts @@ -284,27 +284,30 @@ export class ShieldController extends BaseController< for (const transaction of transactions) { const previousTransaction = previousTransactionsById.get(transaction.id); - // Check if the simulation data has changed. - const simulationDataChanged = - // only check if the previous transaction has simulation data and if it has changed - // this is to avoid checking coverage for the `TWICE` (once when it's added to the state and once when it's simulated for the first time). - // we only need to update the coverage result when the simulation data has changed. - previousTransaction?.simulationData && - !isEqual( - transaction.simulationData, - previousTransaction.simulationData, - ); - - // Check coverage if the transaction is new or if the simulation data has - // changed. if ( - transaction.status !== TransactionStatus.submitted && // we don't need to check coverage for submitted transactions - (!previousTransaction || simulationDataChanged) + // We don't need to check coverage for submitted or confirmed transactions. + transaction.status !== TransactionStatus.submitted && + transaction.status !== TransactionStatus.confirmed ) { - this.checkCoverage(transaction).catch( - // istanbul ignore next - (error) => log('Error checking coverage:', error), - ); + // Check if the simulation data has changed. + const simulationDataChanged = + // only check if the previous transaction has simulation data and if it has changed + // this is to avoid checking coverage for the `TWICE` (once when it's added to the state and once when it's simulated for the first time). + // we only need to update the coverage result when the simulation data has changed. + previousTransaction?.simulationData && + !isEqual( + transaction.simulationData, + previousTransaction.simulationData, + ); + + // Check coverage if the transaction is new or if the simulation data has + // changed. + if (!previousTransaction || simulationDataChanged) { + this.checkCoverage(transaction).catch( + // istanbul ignore next + (error) => log('Error checking coverage:', error), + ); + } } // Log transaction once it has been submitted.