diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index bcb2b7c9..e148ee31 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -2222,6 +2222,9 @@ async function withController( getFeatureFlags: jest.fn(), updateTransaction: jest.fn(), ...options, + getRemoteFeatureFlags: + options?.getRemoteFeatureFlags ?? + (() => ({ transactionsTxHashInAnalytics: false })), }); function triggerNetworStateChange(state: NetworkState) { diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index ac3aa45f..3996d2b4 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -206,6 +206,7 @@ type SmartTransactionsControllerOptions = { getMetaMetricsProps: () => Promise; getFeatureFlags: () => FeatureFlags; updateTransaction: (transaction: TransactionMeta, note: string) => void; + getRemoteFeatureFlags: () => { transactionsTxHashInAnalytics?: boolean }; }; export type SmartTransactionsControllerPollingInput = { @@ -245,6 +246,8 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo #updateTransaction: SmartTransactionsControllerOptions['updateTransaction']; + #getRemoteFeatureFlags: () => { transactionsTxHashInAnalytics?: boolean }; + /* istanbul ignore next */ async #fetch(request: string, options?: RequestInit) { const fetchOptions = { @@ -272,6 +275,7 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo getMetaMetricsProps, getFeatureFlags, updateTransaction, + getRemoteFeatureFlags, }: SmartTransactionsControllerOptions) { super({ name: controllerName, @@ -319,6 +323,7 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo `${controllerName}:stateChange`, (currentState) => this.checkPoll(currentState), ); + this.#getRemoteFeatureFlags = getRemoteFeatureFlags; } async _executePoll({ @@ -411,6 +416,7 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo properties: getSmartTransactionMetricsProperties(updatedSmartTransaction), sensitiveProperties: getSmartTransactionMetricsSensitiveProperties( updatedSmartTransaction, + this.#getRemoteFeatureFlags, ), }); } @@ -733,8 +739,10 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo event: MetaMetricsEventName.StxConfirmed, category: MetaMetricsEventCategory.Transactions, properties: getSmartTransactionMetricsProperties(smartTransaction), - sensitiveProperties: - getSmartTransactionMetricsSensitiveProperties(smartTransaction), + sensitiveProperties: getSmartTransactionMetricsSensitiveProperties( + smartTransaction, + this.#getRemoteFeatureFlags, + ), }); this.#updateSmartTransaction( { ...smartTransaction, confirmed: true }, diff --git a/src/index.test.ts b/src/index.test.ts index f04e5557..011b14cb 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -38,6 +38,7 @@ describe('default export', () => { getFeatureFlags: jest.fn(), updateTransaction: jest.fn(), clientId: ClientId.Extension, + getRemoteFeatureFlags: () => ({ transactionsTxHashInAnalytics: false }), }); expect(controller).toBeInstanceOf(SmartTransactionsController); jest.clearAllTimers(); diff --git a/src/utils.test.ts b/src/utils.test.ts index 5d06b53f..149e8f60 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -356,6 +356,11 @@ describe('src/utils.js', () => { }); describe('getTxHash', () => { + /** + * Core functionality test - verifies that the function correctly generates + * a deterministic hash from a valid signed transaction. This is the primary + * use case for transaction tracking in MetaMetrics. + */ it('returns a transaction hash from a signed transaction', () => { const expectedTxHash = '0x0302b75dfb9fd9eb34056af031efcaee2a8cbd799ea054a85966165cd82a7356'; @@ -363,17 +368,69 @@ describe('src/utils.js', () => { expect(txHash).toBe(expectedTxHash); }); + /** + * Safety check - ensures the function handles empty input gracefully + * instead of throwing errors. Important for defensive programming and + * preventing crashes when tracking transaction hashes. + */ it('returns an empty string if there is no signed transaction', () => { const expectedTxHash = ''; const txHash = utils.getTxHash(''); expect(txHash).toBe(expectedTxHash); }); + /** + * Error handling test - verifies that malformed transaction data + * results in appropriate error messaging rather than silent failures + * or incorrect hash generation. + */ it('throws an error with an incorrect signed transaction', () => { expect(() => { utils.getTxHash('0x0302b75dfb9fd9eb34056af0'); }).toThrow('kzg instance required to instantiate blob tx'); }); + + /** + * Format handling test - ensures the function works correctly with both + * prefixed (0x) and unprefixed hex strings. This is crucial as transaction + * data might come in either format from different parts of the system. + */ + it('handles transaction hex without 0x prefix', () => { + const withPrefix = createSignedTransaction(); + const withoutPrefix = createSignedTransaction().slice(2); + expect(utils.getTxHash(withPrefix)).toBe(utils.getTxHash(withoutPrefix)); + }); + + /** + * Consistency test - verifies that the function generates the same hash + * regardless of hex string casing. This ensures reliable transaction + * tracking even if the hex representation varies across the system. + */ + it('maintains consistent hash with different string representations', () => { + const original = createSignedTransaction(); + // Convert to upper/lower case but preserve the 0x prefix + const upperCase = `0x${original.slice(2).toUpperCase()}`; + const lowerCase = `0x${original.slice(2).toLowerCase()}`; + const hash = utils.getTxHash(original); + + expect(utils.getTxHash(upperCase)).toBe(hash); + expect(utils.getTxHash(lowerCase)).toBe(hash); + }); + + /** + * Case insensitivity test - verifies that the function handles both uppercase + * and lowercase hex characters correctly by normalizing the input. This is + * important because hex strings might be represented in different cases + * across the system, but should still produce the same hash. + */ + it('maintains consistent hash regardless of hex string case', () => { + const original = createSignedTransaction(); + // Convert the middle portion to uppercase, keeping 0x prefix lowercase + const mixedCase = `0x${original.slice(2).toUpperCase()}`; + const hash = utils.getTxHash(original); + + expect(utils.getTxHash(mixedCase)).toBe(hash); + }); }); describe('getReturnTxHashAsap', () => { diff --git a/src/utils.ts b/src/utils.ts index 1fd34882..92726352 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,5 +1,5 @@ import { TransactionFactory } from '@ethereumjs/tx'; -import { bytesToHex } from '@ethereumjs/util'; +import { bytesToHex, hexToBytes } from '@ethereumjs/util'; import { hexlify } from '@ethersproject/bytes'; import type { TransactionMeta } from '@metamask/transaction-controller'; import { TransactionStatus } from '@metamask/transaction-controller'; @@ -224,13 +224,20 @@ export const incrementNonceInHex = (nonceInHex: string): string => { return hexlify(Number(nonceInDec) + 1); }; -export const getTxHash = (signedTxHex: any) => { +export const getTxHash = (signedTxHex: string) => { if (!signedTxHex) { return ''; } + // First normalize the entire string to lowercase + const normalizedInput = signedTxHex.toLowerCase(); + // Then remove 0x prefix if it exists + const hexWithoutPrefix = normalizedInput.startsWith('0x') + ? normalizedInput.slice(2) + : normalizedInput; + // Add single 0x prefix + const prefixedHex = `0x${hexWithoutPrefix}`; const txHashBytes = TransactionFactory.fromSerializedData( - // eslint-disable-next-line no-restricted-globals - Buffer.from(signedTxHex.slice(2), 'hex'), + hexToBytes(prefixedHex), ).hash(); return bytesToHex(txHashBytes); }; @@ -256,19 +263,41 @@ export const getSmartTransactionMetricsProperties = ( }; }; +type SmartTransactionSensitiveProperties = { + token_from_symbol?: string; + token_to_symbol?: string; + account_hardware_type?: string; + account_type?: string; + device_model?: string; + transaction_hash?: string; +}; + export const getSmartTransactionMetricsSensitiveProperties = ( smartTransaction: SmartTransaction, -) => { + getRemoteFeatureFlags?: () => { transactionsTxHashInAnalytics?: boolean }, +): SmartTransactionSensitiveProperties => { if (!smartTransaction) { return {}; } - return { + + const sensitiveProperties: SmartTransactionSensitiveProperties = { token_from_symbol: smartTransaction.sourceTokenSymbol, token_to_symbol: smartTransaction.destinationTokenSymbol, account_hardware_type: smartTransaction.accountHardwareType, account_type: smartTransaction.accountType, device_model: smartTransaction.deviceModel, }; + + // Add transaction hash if feature flag is enabled and user has opted in + if ( + getRemoteFeatureFlags?.()?.transactionsTxHashInAnalytics && + smartTransaction.statusMetadata?.minedHash + ) { + sensitiveProperties.transaction_hash = + smartTransaction.statusMetadata.minedHash; + } + + return sensitiveProperties; }; export const getReturnTxHashAsap = (