From 283aae4de35e8efcf72baf05b046a697fce76076 Mon Sep 17 00:00:00 2001 From: Andrew Taran Date: Mon, 24 Aug 2026 13:03:31 +0200 Subject: [PATCH 1/4] fix: solana dapp transactions expirity check --- eslint-suppressions.json | 3 - packages/solana-wallet-snap/CHANGELOG.md | 1 + packages/solana-wallet-snap/locales/en.json | 3 + packages/solana-wallet-snap/messages.json | 1 + .../solana-wallet-snap/snap.manifest.json | 2 +- .../refreshConfirmationEstimation.test.tsx | 116 ++++++++++++++- .../refreshConfirmationEstimation.tsx | 52 +++++-- .../src/core/services/signer/Signer.test.ts | 132 +++++++++++++++++- .../src/core/services/signer/Signer.ts | 25 ++-- .../buildExpiredScanResult.ts | 14 ++ .../isTransactionBlockhashExpired.test.ts | 43 ++++++ .../isTransactionBlockhashExpired.ts | 65 +++++++++ .../services/wallet/WalletService.test.ts | 8 +- .../src/core/services/wallet/WalletService.ts | 12 +- .../TransactionAlert/getErrorMessage.ts | 3 + .../ConfirmTransactionRequest.tsx | 8 +- 16 files changed, 444 insertions(+), 44 deletions(-) create mode 100644 packages/solana-wallet-snap/src/core/services/transaction-scan/buildExpiredScanResult.ts create mode 100644 packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.test.ts create mode 100644 packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.ts diff --git a/eslint-suppressions.json b/eslint-suppressions.json index c178c43f8..9cd788ff1 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -229,9 +229,6 @@ "packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.test.tsx": { "@typescript-eslint/explicit-function-return-type": { "count": 2 - }, - "@typescript-eslint/no-explicit-any": { - "count": 1 } }, "packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.tsx": { diff --git a/packages/solana-wallet-snap/CHANGELOG.md b/packages/solana-wallet-snap/CHANGELOG.md index 88585352d..91f8d49ac 100644 --- a/packages/solana-wallet-snap/CHANGELOG.md +++ b/packages/solana-wallet-snap/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - **BREAKING:** Preserve dapp-origin `signTransaction` and `signAndSendTransaction` payloads by signing the decoded transaction directly ([#156](https://github.com/MetaMask/internal-snaps/pull/156)) +- Prevent signing dapp transactions with expired blockhashes, and refresh the blockhash for MetaMask-originated transactions before signing. ([#183](https://github.com/MetaMask/internal-snaps/pull/183)) ## [6.0.0] diff --git a/packages/solana-wallet-snap/locales/en.json b/packages/solana-wallet-snap/locales/en.json index 9e4e4e7e5..c9febc192 100644 --- a/packages/solana-wallet-snap/locales/en.json +++ b/packages/solana-wallet-snap/locales/en.json @@ -280,6 +280,9 @@ "transactionScan.errors.slippageToleranceExceeded": { "message": "The transaction was reverted because the slippage tolerance was exceeded." }, + "transactionScan.errors.transactionBlockhashExpired": { + "message": "Please go back and try again" + }, "transactionScan.errors.unknownError": { "message": "An unknown error occurred." } diff --git a/packages/solana-wallet-snap/messages.json b/packages/solana-wallet-snap/messages.json index 5f8558d0c..dacda376d 100644 --- a/packages/solana-wallet-snap/messages.json +++ b/packages/solana-wallet-snap/messages.json @@ -92,5 +92,6 @@ "transactionScan.errors.accountAlreadyInUse": "An account with the same address already exists.", "transactionScan.errors.insufficientSol": "Account does not have enough SOL to perform the operation.", "transactionScan.errors.slippageToleranceExceeded": "The transaction was reverted because the slippage tolerance was exceeded.", + "transactionScan.errors.transactionBlockhashExpired": "Please go back and try again", "transactionScan.errors.unknownError": "An unknown error occurred." } diff --git a/packages/solana-wallet-snap/snap.manifest.json b/packages/solana-wallet-snap/snap.manifest.json index e832e7d66..eac1c5811 100644 --- a/packages/solana-wallet-snap/snap.manifest.json +++ b/packages/solana-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "lKgpeQgMXQzAgQ8GpHxIQ0ot/jc+D5HIZhrccB9fUjI=", + "shasum": "0x6xzJpFpEpHdgBknFqTb5EdjpTe4wkg06jNIBY7Okg=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.test.tsx b/packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.test.tsx index 001dbefb8..7273dd6f8 100644 --- a/packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.test.tsx +++ b/packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.test.tsx @@ -1,12 +1,22 @@ import { SolMethod } from '@metamask/keyring-api'; +import { JsonRpcParams, JsonRpcRequest } from '@metamask/utils'; import { transactionScanService, state } from '../../../../snapContext'; -import { Network } from '../../../constants/solana'; +import { METAMASK_ORIGIN, Network } from '../../../constants/solana'; import { serialize } from '../../../serialization/serialize'; +import { EXPIRED_TRANSACTION_SCAN } from '../../../services/transaction-scan/buildExpiredScanResult'; +import { isTransactionBlockhashExpired } from '../../../services/transaction-scan/isTransactionBlockhashExpired'; import { trackError } from '../../../utils/errors'; import { getInterfaceContext, updateInterface } from '../../../utils/interface'; import { refreshConfirmationEstimation } from './refreshConfirmationEstimation'; +jest.mock( + '../../../services/transaction-scan/isTransactionBlockhashExpired', + () => ({ + isTransactionBlockhashExpired: jest.fn().mockResolvedValue(false), + }), +); + jest.mock('../../../utils/errors', () => ({ trackError: jest.fn().mockResolvedValue('tracked-error-id'), })); @@ -29,6 +39,9 @@ jest.mock( ); jest.mock('../../../../snapContext', () => ({ + connection: { + getRpc: jest.fn(), + }, state: { getKey: jest.fn(), }, @@ -38,6 +51,8 @@ jest.mock('../../../../snapContext', () => ({ })); const setupTest = () => { + jest.clearAllMocks(); + const interfaceContext = { account: { address: 'BLw3RweJmfbTapJRgnPRvd962YDjFYAnVGd1p5hmZ5tP', @@ -58,9 +73,34 @@ const setupTest = () => { (getInterfaceContext as jest.Mock).mockResolvedValue(interfaceContext); (updateInterface as jest.Mock).mockResolvedValue(undefined); (serialize as jest.Mock).mockImplementation((value) => value); + jest.mocked(isTransactionBlockhashExpired).mockResolvedValue(false); + + return interfaceContext; }; describe('refreshConfirmationEstimation', () => { + it('disables confirmation with an expired scan result for an invalid blockhash', async () => { + setupTest(); + + (transactionScanService.scanTransaction as jest.Mock).mockResolvedValue({ + status: 'SUCCESS', + }); + jest.mocked(isTransactionBlockhashExpired).mockResolvedValue(true); + + await refreshConfirmationEstimation({ + request: {} as JsonRpcRequest, + }); + + expect(updateInterface).toHaveBeenCalledWith( + 'interface-id', + null, + expect.objectContaining({ + scan: EXPIRED_TRANSACTION_SCAN, + scanFetchStatus: 'fetched', + }), + ); + }); + it('tracks refresh failures and restores the fetched state', async () => { setupTest(); @@ -70,7 +110,9 @@ describe('refreshConfirmationEstimation', () => { error, ); - await refreshConfirmationEstimation({ request: {} as any }); + await refreshConfirmationEstimation({ + request: {} as JsonRpcRequest, + }); expect(trackError).toHaveBeenCalledWith(error); expect(updateInterface).toHaveBeenLastCalledWith( @@ -81,4 +123,74 @@ describe('refreshConfirmationEstimation', () => { }), ); }); + + it('checks blockhash expiry when simulation is disabled', async () => { + const interfaceContext = setupTest(); + + interfaceContext.preferences = { + simulateOnChainActions: false, + }; + (getInterfaceContext as jest.Mock).mockResolvedValue(interfaceContext); + jest.mocked(isTransactionBlockhashExpired).mockResolvedValue(true); + + await refreshConfirmationEstimation({ + request: {} as JsonRpcRequest, + }); + + expect(transactionScanService.scanTransaction).not.toHaveBeenCalled(); + expect(isTransactionBlockhashExpired).toHaveBeenCalled(); + expect(updateInterface).toHaveBeenCalledWith( + 'interface-id', + null, + expect.objectContaining({ + scan: EXPIRED_TRANSACTION_SCAN, + scanFetchStatus: 'fetched', + }), + ); + }); + + it('does not check blockhash expiry for MetaMask-origin transactions', async () => { + const interfaceContext = { + ...setupTest(), + origin: METAMASK_ORIGIN, + }; + (getInterfaceContext as jest.Mock).mockResolvedValue(interfaceContext); + + await refreshConfirmationEstimation({ + request: {} as JsonRpcRequest, + }); + + expect(isTransactionBlockhashExpired).not.toHaveBeenCalled(); + }); + + it('preserves the existing scan while checking a valid blockhash without simulation', async () => { + const existingScan = { status: 'SUCCESS' }; + const interfaceContext = { + ...setupTest(), + preferences: { simulateOnChainActions: false }, + scan: existingScan, + }; + + (getInterfaceContext as jest.Mock).mockResolvedValue(interfaceContext); + + await refreshConfirmationEstimation({ + request: {} as JsonRpcRequest, + }); + + expect(transactionScanService.scanTransaction).not.toHaveBeenCalled(); + expect(updateInterface).toHaveBeenNthCalledWith( + 1, + 'interface-id', + null, + expect.objectContaining({ scanFetchStatus: 'fetching' }), + ); + expect(updateInterface).toHaveBeenLastCalledWith( + 'interface-id', + null, + expect.objectContaining({ + scan: existingScan, + scanFetchStatus: 'fetched', + }), + ); + }); }); diff --git a/packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.tsx b/packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.tsx index fcb5b46dc..3af29b15d 100644 --- a/packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.tsx +++ b/packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.tsx @@ -2,9 +2,16 @@ import type { OnCronjobHandler } from '@metamask/snaps-sdk'; import { ConfirmTransactionRequest } from '../../../../features/confirmation/views/ConfirmTransactionRequest/ConfirmTransactionRequest'; import type { ConfirmTransactionRequestContext } from '../../../../features/confirmation/views/ConfirmTransactionRequest/types'; -import { state, transactionScanService } from '../../../../snapContext'; +import { + connection, + state, + transactionScanService, +} from '../../../../snapContext'; +import { METAMASK_ORIGIN } from '../../../constants/solana'; import { serialize } from '../../../serialization/serialize'; import type { UnencryptedStateValue } from '../../../services/state/State'; +import { EXPIRED_TRANSACTION_SCAN } from '../../../services/transaction-scan/buildExpiredScanResult'; +import { isTransactionBlockhashExpired } from '../../../services/transaction-scan/isTransactionBlockhashExpired'; import { trackError } from '../../../utils/errors'; import { CONFIRM_SIGN_AND_SEND_TRANSACTION_INTERFACE_NAME, @@ -12,6 +19,7 @@ import { updateInterface, } from '../../../utils/interface'; import baseLogger from '../../../utils/logger'; +import { ScheduleBackgroundEventMethod } from './ScheduleBackgroundEventMethod'; export const refreshConfirmationEstimation: OnCronjobHandler = async () => { const logger = baseLogger.withPrefix('[refreshConfirmationEstimation]'); @@ -58,9 +66,12 @@ export const refreshConfirmationEstimation: OnCronjobHandler = async () => { // Skip transaction simulation if the preference is disabled if (!interfaceContext.preferences?.simulateOnChainActions) { logger.info(`Transaction simulation is disabled in preferences`); - return; } + // MetaMask-originated transactions receive a fresh blockhash before signing. + const shouldSkipBlockhashCheck = + interfaceContext.origin === METAMASK_ORIGIN; + const fetchingConfirmationContext = { ...interfaceContext, scanFetchStatus: 'fetching', @@ -74,15 +85,23 @@ export const refreshConfirmationEstimation: OnCronjobHandler = async () => { fetchingConfirmationContext, ); - const [scan, updatedInterfaceContextFinal] = await Promise.all([ - transactionScanService.scanTransaction({ - method: interfaceContext.method, - accountAddress: interfaceContext.account.address, - transaction: interfaceContext.transaction, - scope: interfaceContext.scope, - origin: interfaceContext.origin, - account: interfaceContext.account, - }), + const [scan, isExpired, updatedInterfaceContextFinal] = await Promise.all([ + interfaceContext.preferences?.simulateOnChainActions + ? transactionScanService.scanTransaction({ + method: interfaceContext.method, + accountAddress: interfaceContext.account.address, + transaction: interfaceContext.transaction, + scope: interfaceContext.scope, + origin: interfaceContext.origin, + account: interfaceContext.account, + }) + : Promise.resolve(interfaceContext.scan), + shouldSkipBlockhashCheck + ? Promise.resolve(false) + : isTransactionBlockhashExpired( + interfaceContext.transaction, + connection.getRpc(interfaceContext.scope), + ), getInterfaceContext( confirmationInterfaceId, ), @@ -100,9 +119,12 @@ export const refreshConfirmationEstimation: OnCronjobHandler = async () => { const updatedInterfaceContext = { ...updatedInterfaceContextFinal, scanFetchStatus: 'fetched' as const, - scan, + scan: isExpired ? EXPIRED_TRANSACTION_SCAN : scan, }; - logger.info(`New scan fetched`); + + if (interfaceContext.preferences?.simulateOnChainActions) { + logger.info(`New scan fetched`); + } await updateInterface( confirmationInterfaceId, @@ -119,7 +141,9 @@ export const refreshConfirmationEstimation: OnCronjobHandler = async () => { method: 'snap_scheduleBackgroundEvent', params: { duration: 'PT20S', - request: { method: 'refreshConfirmationEstimation' }, + request: { + method: ScheduleBackgroundEventMethod.RefreshConfirmationEstimation, + }, }, }); } catch (error) { diff --git a/packages/solana-wallet-snap/src/core/services/signer/Signer.test.ts b/packages/solana-wallet-snap/src/core/services/signer/Signer.test.ts index 2a292b55e..fa96cd385 100644 --- a/packages/solana-wallet-snap/src/core/services/signer/Signer.test.ts +++ b/packages/solana-wallet-snap/src/core/services/signer/Signer.test.ts @@ -1,16 +1,32 @@ import { + address, + blockhash, + compileTransactionMessage, getBase64Encoder, + getCompiledTransactionMessageEncoder, getSignatureFromTransaction, isTransactionMessageWithBlockhashLifetime, } from '@solana/kit'; +import type { + Transaction, + TransactionMessageBytes, + TransactionMessageWithBlockhashLifetime, +} from '@solana/kit'; import { Network } from '../../constants/solana'; -import { fromBytesToCompilableTransactionMessage } from '../../sdk-extensions/codecs'; +import { + fromBytesToCompilableTransactionMessage, + fromTransactionToBase64String, +} from '../../sdk-extensions/codecs'; import { isTransactionMessageWithComputeUnitLimitInstruction, isTransactionMessageWithComputeUnitPriceInstruction, isTransactionMessageWithFeePayer, } from '../../sdk-extensions/transaction-messages'; +import { + MOCK_SOLANA_KEYRING_ACCOUNT_0, + MOCK_SOLANA_KEYRING_ACCOUNT_1, +} from '../../test/mocks/solana-keyring-accounts'; import { deriveSolanaKeypairMock } from '../../test/mocks/utils/deriveSolanaKeypair'; import logger from '../../utils/logger'; import { createMockConnection } from '../__mocks__/mockConnection'; @@ -129,7 +145,7 @@ describe('Signer', () => { fromAccount, scope, undefined, - true, + 'dapp', ); expect(result.messageBytes).toStrictEqual(originalMessageBytes); @@ -141,6 +157,118 @@ describe('Signer', () => { }); }); + describe('when a serialized multisig transaction has no signatures yet', () => { + it('preserves its message bytes while adding this wallet signature', async () => { + const walletAccount = MOCK_SOLANA_KEYRING_ACCOUNT_0; + const otherSigner = MOCK_SOLANA_KEYRING_ACCOUNT_1; + const walletAddress = address(walletAccount.address); + const otherSignerAddress = address(otherSigner.address); + const messageBytes = getCompiledTransactionMessageEncoder().encode( + compileTransactionMessage({ + version: 'legacy', + feePayer: { address: walletAddress }, + lifetimeConstraint: { + blockhash: blockhash( + 'GmfR6QBrCj6ypdyrJFpBNUjUMZaTazXHG9bVczYAWsVS', + ), + lastValidBlockHeight: 18446744073709551615n, + }, + instructions: [ + { + programAddress: address( + 'MemoSq4gqABAXKb96qnH8TysNcWxMyWCqXgDLGmfcHr', + ), + accounts: [ + { address: walletAddress, role: 3 }, + { address: otherSignerAddress, role: 3 }, + ], + data: new Uint8Array([109, 117, 108, 116, 105, 115, 105, 103]), + }, + ], + }), + ) as TransactionMessageBytes; + const unsignedMultisigTransaction: Transaction = { + messageBytes, + signatures: { + [walletAddress]: null, + [otherSignerAddress]: null, + }, + }; + + const result = await signer.partiallySignBase64String( + fromTransactionToBase64String(unsignedMultisigTransaction), + walletAccount, + mockScope, + undefined, + 'dapp', + ); + + expect(result.messageBytes).toStrictEqual(messageBytes); + expect(result.signatures[walletAddress]).not.toBeNull(); + expect(result.signatures[otherSignerAddress]).toBeNull(); + }); + }); + + describe('when refreshing the blockhash before signing', () => { + it('replaces the blockhash of an unsigned serialized transaction', async () => { + const walletAccount = MOCK_SOLANA_KEYRING_ACCOUNT_0; + const walletAddress = address(walletAccount.address); + const originalBlockhash = blockhash( + 'GmfR6QBrCj6ypdyrJFpBNUjUMZaTazXHG9bVczYAWsVS', + ); + const refreshedBlockhash = { + blockhash: blockhash('8vMXV3ERvs12BY8w1nSHutzwwMptAR5UvUSq5pH2QYsK'), + lastValidBlockHeight: 123n, + }; + jest + .spyOn(mockConnection, 'getLatestBlockhash') + .mockResolvedValue(refreshedBlockhash); + const messageBytes = getCompiledTransactionMessageEncoder().encode( + compileTransactionMessage({ + version: 'legacy', + feePayer: { address: walletAddress }, + lifetimeConstraint: { + blockhash: originalBlockhash, + lastValidBlockHeight: 18446744073709551615n, + }, + instructions: [ + { + programAddress: address( + 'MemoSq4gqABAXKb96qnH8TysNcWxMyWCqXgDLGmfcHr', + ), + data: new Uint8Array([114, 101, 102, 114, 101, 115, 104]), + }, + ], + }), + ) as TransactionMessageBytes; + const unsignedTransaction: Transaction = { + messageBytes, + signatures: { [walletAddress]: null }, + }; + + const result = await signer.partiallySignBase64String( + fromTransactionToBase64String(unsignedTransaction), + walletAccount, + mockScope, + undefined, + 'metamask', + ); + + const transactionMessage = await fromBytesToCompilableTransactionMessage( + result.messageBytes, + mockConnection.getRpc(mockScope), + ); + + const currentBlockhash = ( + transactionMessage as TransactionMessageWithBlockhashLifetime + ).lifetimeConstraint.blockhash; + + expect(mockConnection.getLatestBlockhash).toHaveBeenCalledWith(mockScope); + expect(currentBlockhash).toBe(refreshedBlockhash.blockhash); + expect(currentBlockhash).not.toBe(originalBlockhash); + }); + }); + describe('when the fee payer is different from the account', () => { it('signs the transaction message successfully', async () => { const { scope, transactionRequestBase64Encoded, userAccount } = diff --git a/packages/solana-wallet-snap/src/core/services/signer/Signer.ts b/packages/solana-wallet-snap/src/core/services/signer/Signer.ts index fa6256af6..85f8ab8ad 100644 --- a/packages/solana-wallet-snap/src/core/services/signer/Signer.ts +++ b/packages/solana-wallet-snap/src/core/services/signer/Signer.ts @@ -13,6 +13,7 @@ import { partiallySignTransaction, partiallySignTransactionMessageWithSigners, pipe, + setTransactionMessageLifetimeUsingBlockhash, } from '@solana/kit'; import type { SolanaKeyringAccount } from '../../../entities'; @@ -29,12 +30,13 @@ import { isTransactionMessageWithComputeUnitPriceInstruction, setComputeUnitPriceInstructionIfMissing, setTransactionMessageFeePayerIfMissing, - setTransactionMessageLifetimeUsingBlockhashIfMissing, } from '../../sdk-extensions/transaction-messages'; import { deriveSolanaKeypair } from '../../utils/deriveSolanaKeypair'; import type { Base64Struct } from '../../validation/structs'; import type { SolanaConnection } from '../connection'; +type TransactionSource = 'dapp' | 'metamask'; + /** * Signer class for signing transactions and transaction messages. */ @@ -61,7 +63,7 @@ export class Signer { * @param account - The account to sign the transaction or transaction message with. * @param network - The network on which the transaction is being sent. * @param config - The configuration for the request. - * @param preserveMessageBytes - Whether to preserve the original message bytes. + * @param transactionSource - The source of the transaction. * @returns The signed transaction. * @throws If the base64 string is not a valid transaction or transaction message. */ @@ -70,7 +72,7 @@ export class Signer { account: SolanaKeyringAccount, network: Network, config?: DecompileTransactionMessageFetchingLookupTablesConfig, - preserveMessageBytes = false, + transactionSource?: TransactionSource, ): Promise { this.#logger.log('Partially sign base64 string', { base64String, @@ -79,6 +81,9 @@ export class Signer { config, }); + const preserveMessageBytes = transactionSource === 'dapp'; + const refreshBlockhashBeforeSigning = transactionSource === 'metamask'; + if (preserveMessageBytes) { const transaction = await fromUnknownBase64StringToTransaction(base64String); @@ -102,6 +107,7 @@ export class Signer { transactionMessageOrTransaction, account, network, + transactionSource === 'metamask', ); } @@ -124,6 +130,7 @@ export class Signer { transactionMessageFromUnsignedTransaction, account, network, + refreshBlockhashBeforeSigning, ); } @@ -145,20 +152,23 @@ export class Signer { * @param transactionMessage - The transaction message to sign. * @param account - The account to sign the transaction message with. * @param scope - The network where the transaction is to be sent. + * @param refreshBlockhashBeforeSigning - Whether to refresh the blockhash before signing the transaction message. * @returns The partially signed transaction. */ async #prepareAndPartiallySignTransactionMessage( transactionMessage: BaseTransactionMessage, account: SolanaKeyringAccount, scope: Network, + refreshBlockhashBeforeSigning: boolean, ): Promise> { // First, make sure the transaction message has a fee payer, lifetime constraint and compute unit price const hasLifetimeConstraint = isTransactionMessageWithBlockhashLifetime(transactionMessage); - const blockhash = hasLifetimeConstraint - ? transactionMessage.lifetimeConstraint // Use any value, it won't be used - : await this.#connection.getLatestBlockhash(scope); + const blockhash = + hasLifetimeConstraint && !refreshBlockhashBeforeSigning + ? transactionMessage.lifetimeConstraint + : await this.#connection.getLatestBlockhash(scope); const hasComputeUnitPrice = isTransactionMessageWithComputeUnitPriceInstruction(transactionMessage); @@ -187,8 +197,7 @@ export class Signer { const compilableTransactionMessage = await pipe( transactionMessage, (tx) => setTransactionMessageFeePayerIfMissing(signer.address, tx), - (tx) => - setTransactionMessageLifetimeUsingBlockhashIfMissing(blockhash, tx), + (tx) => setTransactionMessageLifetimeUsingBlockhash(blockhash, tx), (tx) => setComputeUnitPriceInstructionIfMissing(tx, { microLamports, diff --git a/packages/solana-wallet-snap/src/core/services/transaction-scan/buildExpiredScanResult.ts b/packages/solana-wallet-snap/src/core/services/transaction-scan/buildExpiredScanResult.ts new file mode 100644 index 000000000..8647d3187 --- /dev/null +++ b/packages/solana-wallet-snap/src/core/services/transaction-scan/buildExpiredScanResult.ts @@ -0,0 +1,14 @@ +import type { TransactionScanResult } from './types'; + +export const TRANSACTION_BLOCKHASH_EXPIRED = 'TransactionBlockhashExpired'; + +/** A failed scan result used when a transaction's recent blockhash has expired. */ +export const EXPIRED_TRANSACTION_SCAN: TransactionScanResult = { + status: 'ERROR', + estimatedChanges: { assets: [] }, + validation: { type: 'Benign', reason: null }, + error: { + type: null, + code: TRANSACTION_BLOCKHASH_EXPIRED, + }, +}; diff --git a/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.test.ts b/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.test.ts new file mode 100644 index 000000000..78da9b178 --- /dev/null +++ b/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.test.ts @@ -0,0 +1,43 @@ +import type { Rpc, SolanaRpcApi } from '@solana/kit'; + +import { fromUnknowBase64StringToTransactionOrTransactionMessage } from '../../sdk-extensions/codecs'; +import { trackError } from '../../utils/errors'; +import { isTransactionBlockhashExpired } from './isTransactionBlockhashExpired'; + +jest.mock('../../sdk-extensions/codecs', () => ({ + fromUnknowBase64StringToTransactionOrTransactionMessage: jest.fn(), + fromBytesToCompilableTransactionMessage: jest.fn(), +})); + +jest.mock('../../utils/errors', () => ({ + trackError: jest.fn(), +})); + +jest.mock('../../utils/logger', () => ({ + __esModule: true, + default: { + warn: jest.fn(), + withPrefix: jest.fn().mockReturnValue({ + info: jest.fn(), + warn: jest.fn(), + }), + }, +})); + +describe('isTransactionBlockhashExpired', () => { + it('tracks a failure and treats the transaction as not expired', async () => { + const error = new Error('RPC unavailable'); + jest + .mocked(fromUnknowBase64StringToTransactionOrTransactionMessage) + .mockRejectedValue(error); + + expect( + await isTransactionBlockhashExpired( + 'transaction', + {} as Rpc, + ), + ).toBe(false); + + expect(trackError).toHaveBeenCalledWith(error); + }); +}); diff --git a/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.ts b/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.ts new file mode 100644 index 000000000..428a8c29a --- /dev/null +++ b/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.ts @@ -0,0 +1,65 @@ +import { isTransactionMessageWithBlockhashLifetime } from '@solana/kit'; +import type { + CompilableTransactionMessage, + Rpc, + SolanaRpcApi, + Transaction, +} from '@solana/kit'; + +import { + fromBytesToCompilableTransactionMessage, + fromUnknowBase64StringToTransactionOrTransactionMessage, +} from '../../sdk-extensions/codecs'; +import { trackError } from '../../utils/errors'; +import logger from '../../utils/logger'; + +const isCompilableTransactionMessage = ( + transactionOrMessage: Transaction | CompilableTransactionMessage, +): transactionOrMessage is CompilableTransactionMessage => + Object.hasOwn(transactionOrMessage, 'instructions'); + +/** + * Checks whether a transaction with a regular recent-blockhash lifetime has + * expired. Durable-nonce transactions are deliberately ignored because their + * lifetime is determined by their nonce account, not the recent-blockhash cache. + * + * @param transaction - The base64 encoded transaction or compiled message. + * @param rpc - The RPC client used to validate a recent blockhash. + * @returns Whether the transaction uses an expired regular blockhash. + */ +export const isTransactionBlockhashExpired = async ( + transaction: string, + rpc: Rpc, +): Promise => { + try { + const transactionOrMessage = + await fromUnknowBase64StringToTransactionOrTransactionMessage( + transaction, + rpc, + ); + + const message = isCompilableTransactionMessage(transactionOrMessage) + ? transactionOrMessage + : await fromBytesToCompilableTransactionMessage( + transactionOrMessage.messageBytes, + rpc, + ); + + const isBlockhashLifetime = + isTransactionMessageWithBlockhashLifetime(message); + + if (!isBlockhashLifetime) { + return false; + } + + const { value: isBlockhashValid } = await rpc + .isBlockhashValid(message.lifetimeConstraint.blockhash) + .send(); + + return !isBlockhashValid; + } catch (error) { + logger.warn({ error }, 'Could not check transaction blockhash lifetime'); + await trackError(error); + return false; + } +}; diff --git a/packages/solana-wallet-snap/src/core/services/wallet/WalletService.test.ts b/packages/solana-wallet-snap/src/core/services/wallet/WalletService.test.ts index 76a98f54f..a7e07412a 100644 --- a/packages/solana-wallet-snap/src/core/services/wallet/WalletService.test.ts +++ b/packages/solana-wallet-snap/src/core/services/wallet/WalletService.test.ts @@ -256,7 +256,7 @@ describe('WalletService', () => { fromAccount, scope, undefined, - true, + 'dapp', ); }); @@ -273,7 +273,7 @@ describe('WalletService', () => { fromAccount, scope, undefined, - false, + 'metamask', ); }); @@ -322,7 +322,7 @@ describe('WalletService', () => { fromAccount, scope, undefined, - true, + 'dapp', ); }); @@ -339,7 +339,7 @@ describe('WalletService', () => { fromAccount, scope, undefined, - false, + 'metamask', ); }); diff --git a/packages/solana-wallet-snap/src/core/services/wallet/WalletService.ts b/packages/solana-wallet-snap/src/core/services/wallet/WalletService.ts index 1a7182945..3acc9c944 100644 --- a/packages/solana-wallet-snap/src/core/services/wallet/WalletService.ts +++ b/packages/solana-wallet-snap/src/core/services/wallet/WalletService.ts @@ -181,9 +181,7 @@ export class WalletService { } : undefined; - // For transactions coming from DApps, preserve the original message bytes. - // Mutating them would change the signing payload and break multisig flows. - const shouldPreserveMessageBytes = origin !== METAMASK_ORIGIN; + const transactionSource = origin === METAMASK_ORIGIN ? 'metamask' : 'dapp'; const partiallySignedTransaction = await this.#signer.partiallySignBase64String( @@ -191,7 +189,7 @@ export class WalletService { account, scope, config, - shouldPreserveMessageBytes, + transactionSource, ); const signedTransactionBase64 = fromTransactionToBase64String( @@ -257,9 +255,7 @@ export class WalletService { } : undefined; - // For transactions coming from DApps, preserve the original message bytes. - // Mutating them would change the signing payload and break multisig flows. - const shouldPreserveMessageBytes = origin !== METAMASK_ORIGIN; + const transactionSource = origin === METAMASK_ORIGIN ? 'metamask' : 'dapp'; const partiallySignedTransaction = await this.#signer.partiallySignBase64String( @@ -267,7 +263,7 @@ export class WalletService { account, scope, signConfig, - shouldPreserveMessageBytes, + transactionSource, ); const signature = getSignatureFromTransaction(partiallySignedTransaction); diff --git a/packages/solana-wallet-snap/src/features/confirmation/components/TransactionAlert/getErrorMessage.ts b/packages/solana-wallet-snap/src/features/confirmation/components/TransactionAlert/getErrorMessage.ts index 9fa88746f..a35430e55 100644 --- a/packages/solana-wallet-snap/src/features/confirmation/components/TransactionAlert/getErrorMessage.ts +++ b/packages/solana-wallet-snap/src/features/confirmation/components/TransactionAlert/getErrorMessage.ts @@ -1,3 +1,4 @@ +import { TRANSACTION_BLOCKHASH_EXPIRED } from '../../../../core/services/transaction-scan/buildExpiredScanResult'; import type { TransactionScanError } from '../../../../core/services/transaction-scan/types'; import type { Preferences } from '../../../../core/types/snap'; import { i18n } from '../../../../core/utils/i18n'; @@ -11,6 +12,8 @@ const ERROR_MESSAGES: Record = { SlippageToleranceExceeded: 'transactionScan.errors.slippageToleranceExceeded', // Jupiter ExceededDesiredSlippageLimit: 'transactionScan.errors.slippageToleranceExceeded', // Raydium + [TRANSACTION_BLOCKHASH_EXPIRED]: + 'transactionScan.errors.transactionBlockhashExpired', }; /** diff --git a/packages/solana-wallet-snap/src/features/confirmation/views/ConfirmTransactionRequest/ConfirmTransactionRequest.tsx b/packages/solana-wallet-snap/src/features/confirmation/views/ConfirmTransactionRequest/ConfirmTransactionRequest.tsx index edae5378e..04d9ac2b5 100644 --- a/packages/solana-wallet-snap/src/features/confirmation/views/ConfirmTransactionRequest/ConfirmTransactionRequest.tsx +++ b/packages/solana-wallet-snap/src/features/confirmation/views/ConfirmTransactionRequest/ConfirmTransactionRequest.tsx @@ -26,13 +26,17 @@ export const ConfirmTransactionRequest = ({ const { nativeToken } = Networks[context.scope]; const nativePrice = context.tokenPrices[nativeToken.caip19Id]?.price ?? null; + const isScanError = context.scan?.status === 'ERROR'; + const shouldDisableConfirmButton = - context.scanFetchStatus === 'fetching' || context.scan?.status === 'ERROR'; + context.scanFetchStatus === 'fetching' || isScanError; + + const shouldShowAlert = context.preferences.useSecurityAlerts || isScanError; return ( - {context.preferences.useSecurityAlerts ? ( + {shouldShowAlert ? ( Date: Tue, 25 Aug 2026 15:37:36 +0200 Subject: [PATCH 2/4] fix: wrong imports --- packages/solana-wallet-snap/snap.manifest.json | 2 +- .../transaction-scan/isTransactionBlockhashExpired.test.ts | 6 +++--- .../transaction-scan/isTransactionBlockhashExpired.ts | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/solana-wallet-snap/snap.manifest.json b/packages/solana-wallet-snap/snap.manifest.json index eac1c5811..a00e776e7 100644 --- a/packages/solana-wallet-snap/snap.manifest.json +++ b/packages/solana-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "0x6xzJpFpEpHdgBknFqTb5EdjpTe4wkg06jNIBY7Okg=", + "shasum": "oXoiahUrkV7oLuS2LwK44vux9N9FxF3/9x696z1Pdcg=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.test.ts b/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.test.ts index 78da9b178..90e4250cd 100644 --- a/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.test.ts +++ b/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.test.ts @@ -1,11 +1,11 @@ import type { Rpc, SolanaRpcApi } from '@solana/kit'; -import { fromUnknowBase64StringToTransactionOrTransactionMessage } from '../../sdk-extensions/codecs'; +import { fromUnknownBase64StringToTransactionOrTransactionMessage } from '../../sdk-extensions/codecs'; import { trackError } from '../../utils/errors'; import { isTransactionBlockhashExpired } from './isTransactionBlockhashExpired'; jest.mock('../../sdk-extensions/codecs', () => ({ - fromUnknowBase64StringToTransactionOrTransactionMessage: jest.fn(), + fromUnknownBase64StringToTransactionOrTransactionMessage: jest.fn(), fromBytesToCompilableTransactionMessage: jest.fn(), })); @@ -28,7 +28,7 @@ describe('isTransactionBlockhashExpired', () => { it('tracks a failure and treats the transaction as not expired', async () => { const error = new Error('RPC unavailable'); jest - .mocked(fromUnknowBase64StringToTransactionOrTransactionMessage) + .mocked(fromUnknownBase64StringToTransactionOrTransactionMessage) .mockRejectedValue(error); expect( diff --git a/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.ts b/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.ts index 428a8c29a..7e517635c 100644 --- a/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.ts +++ b/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.ts @@ -8,7 +8,7 @@ import type { import { fromBytesToCompilableTransactionMessage, - fromUnknowBase64StringToTransactionOrTransactionMessage, + fromUnknownBase64StringToTransactionOrTransactionMessage, } from '../../sdk-extensions/codecs'; import { trackError } from '../../utils/errors'; import logger from '../../utils/logger'; @@ -33,7 +33,7 @@ export const isTransactionBlockhashExpired = async ( ): Promise => { try { const transactionOrMessage = - await fromUnknowBase64StringToTransactionOrTransactionMessage( + await fromUnknownBase64StringToTransactionOrTransactionMessage( transaction, rpc, ); From f09ee8db82dbffe26a897d86473a21af40987398 Mon Sep 17 00:00:00 2001 From: Andrew Taran Date: Tue, 25 Aug 2026 16:08:04 +0200 Subject: [PATCH 3/4] chore: types import over regular --- .../backgroundEvents/refreshConfirmationEstimation.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.test.tsx b/packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.test.tsx index 7273dd6f8..bf02d1cf9 100644 --- a/packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.test.tsx +++ b/packages/solana-wallet-snap/src/core/handlers/onCronjob/backgroundEvents/refreshConfirmationEstimation.test.tsx @@ -1,5 +1,5 @@ import { SolMethod } from '@metamask/keyring-api'; -import { JsonRpcParams, JsonRpcRequest } from '@metamask/utils'; +import type { JsonRpcParams, JsonRpcRequest } from '@metamask/utils'; import { transactionScanService, state } from '../../../../snapContext'; import { METAMASK_ORIGIN, Network } from '../../../constants/solana'; From b2ac2dd9987f7534da25eb22d25ae73f1bc1c0c4 Mon Sep 17 00:00:00 2001 From: Andrew Taran Date: Tue, 25 Aug 2026 16:17:45 +0200 Subject: [PATCH 4/4] chore: add more test cases --- .../isTransactionBlockhashExpired.test.ts | 106 +++++++++++++++++- 1 file changed, 104 insertions(+), 2 deletions(-) diff --git a/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.test.ts b/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.test.ts index 90e4250cd..37904c6a6 100644 --- a/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.test.ts +++ b/packages/solana-wallet-snap/src/core/services/transaction-scan/isTransactionBlockhashExpired.test.ts @@ -1,6 +1,25 @@ -import type { Rpc, SolanaRpcApi } from '@solana/kit'; +import type { + CompilableTransactionMessage, + Nonce, + Rpc, + SolanaRpcApi, + Transaction, + TransactionMessageBytes, +} from '@solana/kit'; +import { + address, + blockhash as toBlockhash, + createTransactionMessage, + pipe, + setTransactionMessageFeePayer, + setTransactionMessageLifetimeUsingBlockhash, + setTransactionMessageLifetimeUsingDurableNonce, +} from '@solana/kit'; -import { fromUnknownBase64StringToTransactionOrTransactionMessage } from '../../sdk-extensions/codecs'; +import { + fromBytesToCompilableTransactionMessage, + fromUnknownBase64StringToTransactionOrTransactionMessage, +} from '../../sdk-extensions/codecs'; import { trackError } from '../../utils/errors'; import { isTransactionBlockhashExpired } from './isTransactionBlockhashExpired'; @@ -25,6 +44,38 @@ jest.mock('../../utils/logger', () => ({ })); describe('isTransactionBlockhashExpired', () => { + const blockhash = toBlockhash('11111111111111111111111111111111'); + const feePayer = address('BLw3RweJmfbTapJRgnPRvd962YDjFYAnVGd1p5hmZ5tP'); + + const getRpc = (): Rpc => + ({ isBlockhashValid: jest.fn() }) as unknown as Rpc; + + const getMessageWithBlockhashLifetime = (): CompilableTransactionMessage => + pipe( + createTransactionMessage({ version: 0 }), + (message) => setTransactionMessageFeePayer(feePayer, message), + (message) => + setTransactionMessageLifetimeUsingBlockhash( + { blockhash, lastValidBlockHeight: 42n }, + message, + ), + ); + + const getMessageWithDurableNonceLifetime = (): CompilableTransactionMessage => + pipe( + createTransactionMessage({ version: 0 }), + (message) => setTransactionMessageFeePayer(feePayer, message), + (message) => + setTransactionMessageLifetimeUsingDurableNonce( + { + nonce: blockhash as unknown as Nonce, + nonceAccountAddress: feePayer, + nonceAuthorityAddress: feePayer, + }, + message, + ), + ); + it('tracks a failure and treats the transaction as not expired', async () => { const error = new Error('RPC unavailable'); jest @@ -40,4 +91,55 @@ describe('isTransactionBlockhashExpired', () => { expect(trackError).toHaveBeenCalledWith(error); }); + + it('returns false when a compilable message has a valid recent blockhash', async () => { + const rpc = getRpc(); + jest + .mocked(fromUnknownBase64StringToTransactionOrTransactionMessage) + .mockResolvedValue(getMessageWithBlockhashLifetime()); + const send = jest.fn().mockResolvedValue({ value: true }); + jest.mocked(rpc.isBlockhashValid).mockReturnValue({ send } as never); + + expect(await isTransactionBlockhashExpired('message', rpc)).toBe(false); + + expect(rpc.isBlockhashValid).toHaveBeenCalledWith(blockhash); + expect(send).toHaveBeenCalledTimes(1); + }); + + it('returns true when a transaction has an expired recent blockhash', async () => { + const rpc = getRpc(); + const transaction: Transaction = { + messageBytes: new Uint8Array() as unknown as TransactionMessageBytes, + signatures: {}, + }; + jest + .mocked(fromUnknownBase64StringToTransactionOrTransactionMessage) + .mockResolvedValue(transaction); + jest + .mocked(fromBytesToCompilableTransactionMessage) + .mockResolvedValue(getMessageWithBlockhashLifetime()); + const send = jest.fn().mockResolvedValue({ value: false }); + jest.mocked(rpc.isBlockhashValid).mockReturnValue({ send } as never); + + expect(await isTransactionBlockhashExpired('transaction', rpc)).toBe(true); + + expect(fromBytesToCompilableTransactionMessage).toHaveBeenCalledWith( + transaction.messageBytes, + rpc, + ); + expect(rpc.isBlockhashValid).toHaveBeenCalledWith(blockhash); + }); + + it('does not check a durable-nonce lifetime against the recent-blockhash cache', async () => { + const rpc = getRpc(); + jest + .mocked(fromUnknownBase64StringToTransactionOrTransactionMessage) + .mockResolvedValue(getMessageWithDurableNonceLifetime()); + + expect( + await isTransactionBlockhashExpired('durable-nonce-message', rpc), + ).toBe(false); + + expect(rpc.isBlockhashValid).not.toHaveBeenCalled(); + }); });