diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index d282f7db..0ee8959f 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-stellar-wallet.git" }, "source": { - "shasum": "0bXKXi1JKlz9Tl+t7sHnw2HInhPpNtkrTIrUdRD/n4I=", + "shasum": "z4mud5yiJBM62ns46mKRfzbW44lxw+ZOwH1nMTCq4CM=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/handler.test.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/handler.test.ts index 22fdaae0..9bcc3239 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/handler.test.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/handler.test.ts @@ -321,6 +321,68 @@ describe('RefreshConfirmationContextHandler', () => { ); }); + it('runs the transaction refresher first and feeds its patch to the remaining refreshers', async () => { + jest + .mocked(getInterfaceContextIfExists) + .mockResolvedValueOnce(baseContext) + .mockResolvedValueOnce(baseContext); + + const transactionRefresher = createMockRefresher( + ConfirmationContextRefresherKey.Transaction, + { + refresh: jest.fn().mockResolvedValue({ + result: { securityScanRequest: { transaction: 'FRESH_XDR' } }, + reschedule: false, + }), + }, + ); + const scanRefresher = createMockRefresher( + ConfirmationContextRefresherKey.Scan, + { + refresh: jest.fn().mockResolvedValue({ + result: { scanFetchStatus: FetchStatus.Fetched }, + reschedule: true, + }), + }, + ); + + // Register scan first to prove ordering is driven by key, not array order. + const { handler, updateConfirmation } = setup([ + scanRefresher, + transactionRefresher, + ]); + + await handler.handle({ + jsonrpc: '2.0', + id: '1', + method: BackgroundEventMethod.RefreshConfirmationContext, + params: { + ...confirmationContextRequestParams, + refresherKeys: [ + ConfirmationContextRefresherKey.Scan, + ConfirmationContextRefresherKey.Transaction, + ], + }, + }); + + // The transaction refresher sees the original context... + expect(transactionRefresher.refresh).toHaveBeenCalledWith(baseContext); + // ...and the scan refresher sees it already patched with the rebuilt envelope. + expect(scanRefresher.refresh).toHaveBeenCalledWith( + expect.objectContaining({ + securityScanRequest: { transaction: 'FRESH_XDR' }, + }), + ); + expect(updateConfirmation).toHaveBeenCalledWith( + expect.objectContaining({ + updatedContext: expect.objectContaining({ + securityScanRequest: { transaction: 'FRESH_XDR' }, + scanFetchStatus: FetchStatus.Fetched, + }), + }), + ); + }); + it('does not run a refresher when its key is omitted from refresherKeys', async () => { jest.mocked(getInterfaceContextIfExists).mockResolvedValue(baseContext); diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/handler.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/handler.ts index 6be1ebd3..6b43e442 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/handler.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/handler.ts @@ -1,9 +1,9 @@ import type { Json } from '@metamask/utils'; +import { ConfirmationContextRefresherKey } from './api'; import type { ConfirmationContextRefreshResult, ConfirmationContextRefreshers, - ConfirmationContextRefresherKey, ConfirmationDataContext, IConfirmationContextRefresher, } from './api'; @@ -170,8 +170,17 @@ export class RefreshConfirmationContextHandler extends CronjobBaseHandler { - const settled = await Promise.allSettled( - activeRefreshers.map(async (refresher) => - this.#runRefresher(refresher, ctx), - ), + const transactionRefresher = activeRefreshers.find( + (refresher) => + refresher.key === ConfirmationContextRefresherKey.Transaction, ); + const remainingRefreshers = activeRefreshers.filter( + (refresher) => refresher !== transactionRefresher, + ); + + const results: ConfirmationContextRefreshResult[] = []; + let workingContext = ctx; - return settled.map((outcome, index) => { - if (outcome.status === 'fulfilled') { - return outcome.value; + if (transactionRefresher) { + const transactionResult = await this.#settleRefresher( + transactionRefresher, + workingContext, + ); + results.push(transactionResult); + if (transactionResult?.result) { + workingContext = { ...workingContext, ...transactionResult.result }; } + } + + const remainingResults = await Promise.all( + remainingRefreshers.map(async (refresher) => + this.#settleRefresher(refresher, workingContext), + ), + ); + + return [...results, ...remainingResults]; + } + + /** + * Runs a single refresher and converts an unexpected rejection into `null`, + * so a failing refresher never blocks the others. + * + * @param refresher - The refresher to run. + * @param ctx - The context passed to the refresher. + * @returns The refresher result, or `null` when it rejected. + */ + async #settleRefresher( + refresher: IConfirmationContextRefresher, + ctx: ConfirmationDataContext, + ): Promise { + try { + return await this.#runRefresher(refresher, ctx); + } catch (error) { this.logger.error( - `Refresher "${activeRefreshers[index]?.key}" rejected unexpectedly`, - outcome.reason, + `Refresher "${refresher.key}" rejected unexpectedly`, + error, ); return null; - }); + } } async #runRefresher( diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/transactionRefresher.test.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/transactionRefresher.test.ts index 6b6961ac..8042ad7f 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/transactionRefresher.test.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/transactionRefresher.test.ts @@ -6,6 +6,7 @@ import { ConfirmationTransactionRefresher } from './transactionRefresher'; import { KnownCaip2ChainId } from '../../../api'; import type { AssetMetadataService } from '../../../services/asset-metadata'; import type { TransactionService } from '../../../services/transaction'; +import type { MockClassicOperation } from '../../../services/transaction/__mocks__/transaction.fixtures'; import { buildMockClassicTransaction } from '../../../services/transaction/__mocks__/transaction.fixtures'; import { FetchStatus } from '../../../ui/confirmation/api'; import { getSlip44AssetId } from '../../../utils'; @@ -21,17 +22,36 @@ describe('ConfirmationTransactionRefresher', () => { const accountId = '11111111-1111-4111-8111-111111111111'; const toAddress = 'GDPMFLKUGASUTWBN2XGYYKD27QGHCYH4BUFUTER4L23INYQ4JHDWFOIE'; - const transaction = buildMockClassicTransaction( - [ - { - type: 'payment', - params: { destination: toAddress, asset: 'native', amount: '1' }, - }, - ], - { networkPassphrase: Networks.TESTNET }, - ); + const paymentOperations: MockClassicOperation[] = [ + { + type: 'payment', + params: { destination: toAddress, asset: 'native', amount: '1' }, + }, + ]; + + const transaction = buildMockClassicTransaction(paymentOperations, { + networkPassphrase: Networks.TESTNET, + }); const transactionXdr = transaction.getRaw().toXDR(); + // A scan envelope far from expiry (kept as-is) versus one inside the refresh + // buffer (swapped for the rebuilt transaction). + const freshScanTransactionXdr = buildMockClassicTransaction( + paymentOperations, + { + networkPassphrase: Networks.TESTNET, + timeout: 600, + }, + ) + .getRaw() + .toXDR(); + const staleScanTransactionXdr = buildMockClassicTransaction( + paymentOperations, + { networkPassphrase: Networks.TESTNET, timeout: 30 }, + ) + .getRaw() + .toXDR(); + const sendRequest = { jsonrpc: '2.0' as const, id: 1, @@ -192,6 +212,97 @@ describe('ConfirmationTransactionRefresher', () => { }); }); + it('renews the security-scan transaction when the scanned envelope nears expiry', async () => { + const { refresher } = setup(); + const securityScanRequest = { + accountAddress: toAddress, + origin: 'https://dapp.example', + scope, + transaction: staleScanTransactionXdr, + }; + + const result = await refresher.refresh( + createTransactionContext({ securityScanRequest }), + ); + + expect(result).toStrictEqual({ + result: { + securityScanRequest: { + ...securityScanRequest, + transaction: transactionXdr, + }, + }, + reschedule: false, + }); + }); + + it('renews the security-scan transaction for change-trust flows', async () => { + const { refresher } = setup(); + const securityScanRequest = { + accountAddress: toAddress, + origin: 'https://dapp.example', + scope, + transaction: staleScanTransactionXdr, + }; + + const result = await refresher.refresh( + createTransactionContext({ + request: changeTrustAddRequest, + securityScanRequest, + }), + ); + + expect(result).toStrictEqual({ + result: { + securityScanRequest: { + ...securityScanRequest, + transaction: transactionXdr, + }, + }, + reschedule: false, + }); + }); + + it('keeps the scanned envelope while it is still far from expiry', async () => { + const { refresher } = setup(); + const securityScanRequest = { + accountAddress: toAddress, + origin: 'https://dapp.example', + scope, + transaction: freshScanTransactionXdr, + }; + + const result = await refresher.refresh( + createTransactionContext({ securityScanRequest }), + ); + + expect(result).toBeNull(); + }); + + it('renews the security-scan transaction when the scanned envelope is unparseable', async () => { + const { refresher } = setup(); + const securityScanRequest = { + accountAddress: toAddress, + origin: 'https://dapp.example', + scope, + transaction: 'OUTDATED_XDR', + }; + + const result = await refresher.refresh( + createTransactionContext({ securityScanRequest }), + ); + + expect(result).toStrictEqual({ + result: { + securityScanRequest: { + ...securityScanRequest, + transaction: transactionXdr, + }, + }, + reschedule: false, + }); + }); + it('marks the transaction invalid when the original envelope has expired', async () => { const { refresher, transactionService } = setup(); // The stored XDR being signed is expired, even though the rebuilt draft would be valid. diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/transactionRefresher.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/transactionRefresher.ts index e54b1379..20135376 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/transactionRefresher.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/transactionRefresher.ts @@ -7,13 +7,17 @@ import { type ConfirmationDataContext, type IConfirmationContextRefresher, } from './api'; +import type { KnownCaip2ChainId } from '../../../api'; import type { AssetMetadataService } from '../../../services/asset-metadata'; import { Transaction, type TransactionService, } from '../../../services/transaction'; import { assertTransactionTimeBound } from '../../../services/transaction/utils'; -import type { ContextWithTransactionValidation } from '../../../ui/confirmation/api'; +import type { + ContextWithSecurityScan, + ContextWithTransactionValidation, +} from '../../../ui/confirmation/api'; import { ContextWithTransactionValidationStruct, FetchStatus, @@ -31,6 +35,15 @@ import { type TransactionValidationContext = ConfirmationDataContext & ContextWithTransactionValidation; +/** + * Keep scanning the current envelope until it is within this window of expiry, + * then swap in a freshly rebuilt one. Comfortably larger than the ~20s refresh + * cadence so the scanned transaction is always renewed before it can expire + * between cycles, while avoiding a needless swap on every cycle. Mirrors TRON's + * `hasFreshExpiration` buffer. + */ +const SCAN_TRANSACTION_REFRESH_BUFFER_SECONDS = 60; + /** * Re-validates the pending transaction while the sign confirmation dialog is open. * Transaction slice of the confirmation context refresh pipeline; periodically @@ -128,6 +141,7 @@ export class ConfirmationTransactionRefresher implements IConfirmationContextRef // envelope. It can miss divergence (payment vs createAccount on a deactivated // destination, stale Soroban footprint). Seq drift is covered by the submit-time // txBadSeq retry. For full fidelity, validate the stored envelope itself. + let rebuiltTransaction: Transaction; switch (request.method) { case ClientRequestMethod.ConfirmSend: { const assetMetadata = await this.#assetMetadataService.resolve( @@ -139,34 +153,41 @@ export class ConfirmationTransactionRefresher implements IConfirmationContextRef decimals, ); // Throws on insufficient balance, inactive destination, or fee estimate failure. - await this.#transactionService.createValidatedSendTransaction({ - onChainAccount, - scope, - assetId: request.params.assetId, - destination: request.params.toAddress, - amount, - }); + rebuiltTransaction = + await this.#transactionService.createValidatedSendTransaction({ + onChainAccount, + scope, + assetId: request.params.assetId, + destination: request.params.toAddress, + amount, + }); break; } case ClientRequestMethod.ChangeTrustOpt: // Throws when change-trust limits or account state are no longer valid. - await this.#transactionService.createValidatedChangeTrustTransaction({ - onChainAccount, - scope, - assetId: request.params.assetId, - limit: - request.params.action === ChangeTrustOptAction.Delete - ? '0' - : request.params.limit, - }); + rebuiltTransaction = + await this.#transactionService.createValidatedChangeTrustTransaction( + { + onChainAccount, + scope, + assetId: request.params.assetId, + limit: + request.params.action === ChangeTrustOptAction.Delete + ? '0' + : request.params.limit, + }, + ); break; default: throw new Error('Unsupported request method for transaction refresh'); } - // Still valid: nothing to write. The status stays Fetched and we don't drive a - // reschedule ourselves (other refreshers keep the cron alive while the dialog is open). - return null; + // Feed the rebuilt envelope (with a fresh time bound) to the security scan + // so simulation/validation never runs against an expired transaction. The + // user-facing `transaction` is intentionally left untouched: it is what the + // dialog shows and what `assertTransactionTimeBound` guards above; the + // signable envelope is rebuilt again at confirm time. + return this.#renewScanTransaction(validationCtx, rebuiltTransaction); } catch (error) { this.#logger.error( 'Error re-validating confirmation transaction:', @@ -182,4 +203,72 @@ export class ConfirmationTransactionRefresher implements IConfirmationContextRef isValidContext(ctx: Record): boolean { return ContextWithTransactionValidationStruct.is(ctx); } + + /** + * Writes the rebuilt transaction into the security-scan request so the scan + * refresher (which runs after this one) scans the renewed envelope. + * + * Skips the swap while the currently-scanned envelope is still comfortably + * far from expiry, keeping the Blockaid scan stable on a single envelope + * instead of churning onto a freshly rebuilt one every cycle. + * + * Returns `null` when the flow has no security scan attached (or the scanned + * envelope is still fresh), leaving the transaction status untouched (it stays + * `Fetched`). + * + * @param ctx - The current confirmation context. + * @param rebuiltTransaction - The freshly rebuilt transaction. + * @returns A patch updating the scan request, or `null` when there is nothing to propagate. + */ + #renewScanTransaction( + ctx: TransactionValidationContext, + rebuiltTransaction: Transaction, + ): ConfirmationContextRefreshResult { + const { securityScanRequest } = ctx as TransactionValidationContext & + Partial; + if (!securityScanRequest) { + return null; + } + + if ( + this.#isScanTransactionFresh(securityScanRequest.transaction, ctx.scope) + ) { + return null; + } + + return { + result: { + securityScanRequest: { + ...securityScanRequest, + transaction: rebuiltTransaction.getRaw().toXDR(), + }, + }, + reschedule: false, + }; + } + + /** + * Whether the currently-scanned envelope still has comfortable time before + * expiry and can keep being scanned as-is. + * + * @param xdr - The envelope currently held in the security-scan request. + * @param scope - The network scope used to deserialize the envelope. + * @returns True when the envelope is fresh enough to keep scanning. + */ + #isScanTransactionFresh(xdr: string, scope: KnownCaip2ChainId): boolean { + try { + const { expirationTime } = Transaction.fromXdr({ xdr, scope }); + // No upper bound: the envelope never expires, so keep scanning it. + if (expirationTime === undefined) { + return true; + } + const nowSeconds = Math.floor(Date.now() / 1000); + return ( + expirationTime > nowSeconds + SCAN_TRANSACTION_REFRESH_BUFFER_SECONDS + ); + } catch { + // Unparseable scan envelope: refresh it. + return false; + } + } }