From c650ada58e0fed5311120a1cbc880815feae391b Mon Sep 17 00:00:00 2001 From: dddddanica Date: Tue, 7 Jan 2025 21:59:07 +0000 Subject: [PATCH 1/5] fix(3742): improve user segmentation with BigInt-based random generation --- .../src/utils/user-segmentation-utils.test.ts | 112 +++++++++++++++--- .../src/utils/user-segmentation-utils.ts | 40 +++++-- 2 files changed, 126 insertions(+), 26 deletions(-) diff --git a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts index 6ec5933e02..cef18cdb9e 100644 --- a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts +++ b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts @@ -1,14 +1,19 @@ +import { v4 as uuidV4 } from 'uuid'; + import { generateDeterministicRandomNumber, isFeatureFlagWithScopeValue, } from './user-segmentation-utils'; -const MOCK_METRICS_IDS = [ - '123e4567-e89b-4456-a456-426614174000', - '987fcdeb-51a2-4c4b-9876-543210fedcba', - 'a1b2c3d4-e5f6-4890-abcd-ef1234567890', - 'f9e8d7c6-b5a4-4210-9876-543210fedcba', -]; +const MOCK_METRICS_IDS = { + MOBILE_VALID: '123e4567-e89b-4456-a456-426614174000', + EXTENSION_VALID: + '0x86bacb9b2bf9a7e8d2b147eadb95ac9aaa26842327cd24afc8bd4b3c1d136420', + MOBILE_MIN: '00000000-0000-4000-8000-000000000000', + MOBILE_MAX: 'ffffffff-ffff-4fff-bfff-ffffffffffff', + EXTENSION_MIN: `0x${'0'.repeat(64) as string}`, + EXTENSION_MAX: `0x${'f'.repeat(64) as string}`, +}; const MOCK_FEATURE_FLAGS = { VALID: { @@ -28,26 +33,97 @@ const MOCK_FEATURE_FLAGS = { describe('user-segmentation-utils', () => { describe('generateDeterministicRandomNumber', () => { - it('generates consistent numbers for the same input', () => { - const result1 = generateDeterministicRandomNumber(MOCK_METRICS_IDS[0]); - const result2 = generateDeterministicRandomNumber(MOCK_METRICS_IDS[0]); + describe('Mobile client new implementation (uuidv4)', () => { + it('generates consistent results for same uuidv4', () => { + const result1 = generateDeterministicRandomNumber( + MOCK_METRICS_IDS.MOBILE_VALID, + ); + const result2 = generateDeterministicRandomNumber( + MOCK_METRICS_IDS.MOBILE_VALID, + ); + expect(result1).toBe(result2); + }); - expect(result1).toBe(result2); - }); + it('handles minimum uuidv4 value', () => { + const result = generateDeterministicRandomNumber( + MOCK_METRICS_IDS.MOBILE_MIN, + ); + expect(result).toBe(0); + }); - it('generates numbers between 0 and 1', () => { - MOCK_METRICS_IDS.forEach((id) => { - const result = generateDeterministicRandomNumber(id); + it('handles maximum uuidv4 value', () => { + const result = generateDeterministicRandomNumber( + MOCK_METRICS_IDS.MOBILE_MAX, + ); + // For practical purposes, 0.999999 is functionally equivalent to 1 in this context + // the small deviation from exactly 1.0 is a limitation of floating-point arithmetic, not a bug in the logic. + expect(result).toBeCloseTo(1, 5); + }); + + it('results a random number between 0 and 1', () => { + const result = generateDeterministicRandomNumber( + MOCK_METRICS_IDS.MOBILE_VALID, + ); expect(result).toBeGreaterThanOrEqual(0); expect(result).toBeLessThanOrEqual(1); }); }); - it('generates different numbers for different inputs', () => { - const result1 = generateDeterministicRandomNumber(MOCK_METRICS_IDS[0]); - const result2 = generateDeterministicRandomNumber(MOCK_METRICS_IDS[1]); + describe('Mobile client old implementation and Extension client (hex string)', () => { + it('generates consistent results for same hex', () => { + const result1 = generateDeterministicRandomNumber( + MOCK_METRICS_IDS.EXTENSION_VALID, + ); + const result2 = generateDeterministicRandomNumber( + MOCK_METRICS_IDS.EXTENSION_VALID, + ); + expect(result1).toBe(result2); + }); - expect(result1).not.toBe(result2); + it('handles minimum hex value', () => { + const result = generateDeterministicRandomNumber( + MOCK_METRICS_IDS.EXTENSION_MIN, + ); + expect(result).toBe(0); + }); + + it('handles maximum hex value', () => { + const result = generateDeterministicRandomNumber( + MOCK_METRICS_IDS.EXTENSION_MAX, + ); + expect(result).toBe(1); + }); + }); + + describe('Distribution validation', () => { + it('produces uniform distribution across 1000 samples', () => { + const samples = 1000; + const buckets = 10; + const tolerance = 0.3; + const distribution = new Array(buckets).fill(0); + + // Generate samples using valid UUIDs + Array.from({ length: samples }).forEach(() => { + const uuid = uuidV4(); + const value = generateDeterministicRandomNumber(uuid); + const bucketIndex = Math.floor(value * buckets); + // Handle edge case where value === 1 + distribution[ + bucketIndex === buckets ? buckets - 1 : bucketIndex + ] += 1; + }); + + // Check distribution + const expectedPerBucket = samples / buckets; + const allowedDeviation = expectedPerBucket * tolerance; + + distribution.forEach((count) => { + const minExpected = Math.floor(expectedPerBucket - allowedDeviation); + const maxExpected = Math.ceil(expectedPerBucket + allowedDeviation); + expect(count).toBeGreaterThanOrEqual(minExpected); + expect(count).toBeLessThanOrEqual(maxExpected); + }); + }); }); }); diff --git a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts index 481d0d21c0..a18b2747fe 100644 --- a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts +++ b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts @@ -1,25 +1,49 @@ import type { Json } from '@metamask/utils'; +import { validate as uuidValidate, version as uuidVersion } from 'uuid'; import type { FeatureFlagScopeValue } from '../remote-feature-flag-controller-types'; -/* eslint-disable no-bitwise */ /** * Generates a deterministic random number between 0 and 1 based on a metaMetricsId. * This is useful for A/B testing and feature flag rollouts where we want * consistent group assignment for the same user. * - * @param metaMetricsId - The unique identifier used to generate the deterministic random number + * Supports two metaMetricsId formats: + * - UUIDv4 format (new mobile implementation) + * - Hex format with 0x prefix (extension or old mobile implementation) + * + * For UUIDv4 format, the following normalizations are applied: + * - Replaces version (4) bits with 'f' to normalize range + * - Replaces variant bits (8-b) with 'f' to normalize range + * - Removes all dashes from the UUID + * + * For hex format: + * - Expects a hex string with '0x' prefix (e.g., '0x1234abcd') + * - Removes the '0x' prefix before conversion + * - Converts the remaining hex string to a BigInt for calculation + * + * @param metaMetricsId - The unique identifier used to generate the deterministic random number, can be a UUIDv4 or hex string * @returns A number between 0 and 1 that is deterministic for the given metaMetricsId */ export function generateDeterministicRandomNumber( metaMetricsId: string, ): number { - const hash = [...metaMetricsId].reduce((acc, char) => { - const chr = char.charCodeAt(0); - return ((acc << 5) - acc + chr) | 0; - }, 0); - - return (hash >>> 0) / 0xffffffff; + let cleanId: string, value: bigint; + // uuidv4 format + if (uuidValidate(metaMetricsId) && uuidVersion(metaMetricsId) === 4) { + cleanId = metaMetricsId + .replace(/^(.{12})4/u, '$1f') + .replace(/(.{16})[89ab]/u, '$1f') + .replace(/-/gu, ''); + value = BigInt(`0x${cleanId}`); + } else { + // hex format with 0x prefix + cleanId = metaMetricsId.slice(2); + value = BigInt(`0x${cleanId}`); + } + const maxValue = BigInt(`0x${'f'.repeat(cleanId.length)}`); + // Use BigInt division first, then convert to number to maintain precision + return Number((value * BigInt(1000000)) / maxValue) / 1000000; } /** From 3e155c15d643408138f5b90c48da640265b03e57 Mon Sep 17 00:00:00 2001 From: dddddanica Date: Wed, 8 Jan 2025 14:33:15 +0000 Subject: [PATCH 2/5] fix(3742): remove unnecessary normalization of bit --- .../src/utils/user-segmentation-utils.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts index a18b2747fe..8517eab046 100644 --- a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts +++ b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts @@ -31,10 +31,7 @@ export function generateDeterministicRandomNumber( let cleanId: string, value: bigint; // uuidv4 format if (uuidValidate(metaMetricsId) && uuidVersion(metaMetricsId) === 4) { - cleanId = metaMetricsId - .replace(/^(.{12})4/u, '$1f') - .replace(/(.{16})[89ab]/u, '$1f') - .replace(/-/gu, ''); + cleanId = metaMetricsId.replace(/^(.{12})4/u, '$1f').replace(/-/gu, ''); value = BigInt(`0x${cleanId}`); } else { // hex format with 0x prefix From 87cd8f5e0b8d2613ea22415f7e4b74f5fbe3a2bb Mon Sep 17 00:00:00 2001 From: dddddanica Date: Wed, 8 Jan 2025 18:19:50 +0000 Subject: [PATCH 3/5] fix(3742): add readbility to large number and fix the logic to remove 4 in uuidv4 properly --- .../src/utils/user-segmentation-utils.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts index 8517eab046..d2aeec451f 100644 --- a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts +++ b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts @@ -13,9 +13,9 @@ import type { FeatureFlagScopeValue } from '../remote-feature-flag-controller-ty * - Hex format with 0x prefix (extension or old mobile implementation) * * For UUIDv4 format, the following normalizations are applied: - * - Replaces version (4) bits with 'f' to normalize range - * - Replaces variant bits (8-b) with 'f' to normalize range * - Removes all dashes from the UUID + * - Remove version (4) bits and replace with 'f' + * - Converts the remaining hex string to a BigInt for calculation * * For hex format: * - Expects a hex string with '0x' prefix (e.g., '0x1234abcd') @@ -28,19 +28,18 @@ import type { FeatureFlagScopeValue } from '../remote-feature-flag-controller-ty export function generateDeterministicRandomNumber( metaMetricsId: string, ): number { - let cleanId: string, value: bigint; + let cleanId: string; // uuidv4 format if (uuidValidate(metaMetricsId) && uuidVersion(metaMetricsId) === 4) { - cleanId = metaMetricsId.replace(/^(.{12})4/u, '$1f').replace(/-/gu, ''); - value = BigInt(`0x${cleanId}`); + cleanId = metaMetricsId.replace(/-/gu, '').replace(/^(.{12})4/u, '$1f'); } else { // hex format with 0x prefix cleanId = metaMetricsId.slice(2); - value = BigInt(`0x${cleanId}`); } + const value = BigInt(`0x${cleanId}`); const maxValue = BigInt(`0x${'f'.repeat(cleanId.length)}`); // Use BigInt division first, then convert to number to maintain precision - return Number((value * BigInt(1000000)) / maxValue) / 1000000; + return Number((value * BigInt(1_000_000)) / maxValue) / 1_000_000; } /** From 8724cc343651cff2936732bb9e42be391c08422b Mon Sep 17 00:00:00 2001 From: dddddanica Date: Thu, 9 Jan 2025 12:25:20 +0000 Subject: [PATCH 4/5] fix(3742): improve UUID random number distribution --- .../src/utils/user-segmentation-utils.ts | 55 +++++++++++-------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts index d2aeec451f..f0d62cf65e 100644 --- a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts +++ b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts @@ -3,43 +3,52 @@ import { validate as uuidValidate, version as uuidVersion } from 'uuid'; import type { FeatureFlagScopeValue } from '../remote-feature-flag-controller-types'; +/** + * Converts a UUID string to a BigInt by removing dashes and converting to hexadecimal. + * @param uuid - The UUID string to convert + * @returns The UUID as a BigInt value + */ +function uuidStringToBigInt(uuid: string): bigint { + return BigInt(`0x${uuid.replace(/-/gu, '')}`); +} + +const MIN_UUID_V4 = '00000000-0000-4000-8000-000000000000'; +const MAX_UUID_V4 = 'ffffffff-ffff-4fff-bfff-ffffffffffff'; +const MIN_UUID_V4_BIGINT = uuidStringToBigInt(MIN_UUID_V4); +const MAX_UUID_V4_BIGINT = uuidStringToBigInt(MAX_UUID_V4); +const UUID_V4_VALUE_RANGE_BIGINT = + MAX_UUID_V4_BIGINT - MIN_UUID_V4_BIGINT; + /** * Generates a deterministic random number between 0 and 1 based on a metaMetricsId. * This is useful for A/B testing and feature flag rollouts where we want * consistent group assignment for the same user. - * - * Supports two metaMetricsId formats: - * - UUIDv4 format (new mobile implementation) - * - Hex format with 0x prefix (extension or old mobile implementation) - * - * For UUIDv4 format, the following normalizations are applied: - * - Removes all dashes from the UUID - * - Remove version (4) bits and replace with 'f' - * - Converts the remaining hex string to a BigInt for calculation - * - * For hex format: - * - Expects a hex string with '0x' prefix (e.g., '0x1234abcd') - * - Removes the '0x' prefix before conversion - * - Converts the remaining hex string to a BigInt for calculation - * - * @param metaMetricsId - The unique identifier used to generate the deterministic random number, can be a UUIDv4 or hex string - * @returns A number between 0 and 1 that is deterministic for the given metaMetricsId + * @param metaMetricsId - The unique identifier used to generate the deterministic random number. Must be either: + * - A UUIDv4 string (e.g., '123e4567-e89b-12d3-a456-426614174000') + * - A hex string with '0x' prefix (e.g., '0x86bacb9b2bf9a7e8d2b147eadb95ac9aaa26842327cd24afc8bd4b3c1d136420') + * @returns A number between 0 and 1, deterministically generated from the input ID. + * The same input will always produce the same output. */ export function generateDeterministicRandomNumber( metaMetricsId: string, ): number { - let cleanId: string; + let idValue: bigint; + let maxValue: bigint; // uuidv4 format if (uuidValidate(metaMetricsId) && uuidVersion(metaMetricsId) === 4) { - cleanId = metaMetricsId.replace(/-/gu, '').replace(/^(.{12})4/u, '$1f'); + // Normalize the UUIDv4 range to start from 0 by subtracting MIN_UUID_V4_BIGINT. + // This ensures uniform distribution across the entire range, since UUIDv4 + // has restricted bits for version (4) and variant (8-b) that would otherwise skew the distribution + idValue = uuidStringToBigInt(metaMetricsId) - MIN_UUID_V4_BIGINT; + maxValue = UUID_V4_VALUE_RANGE_BIGINT; } else { // hex format with 0x prefix - cleanId = metaMetricsId.slice(2); + const cleanId = metaMetricsId.slice(2); + idValue = BigInt(`0x${cleanId}`); + maxValue = BigInt(`0x${'f'.repeat(cleanId.length)}`); } - const value = BigInt(`0x${cleanId}`); - const maxValue = BigInt(`0x${'f'.repeat(cleanId.length)}`); // Use BigInt division first, then convert to number to maintain precision - return Number((value * BigInt(1_000_000)) / maxValue) / 1_000_000; + return Number((idValue * BigInt(1_000_000)) / maxValue) / 1_000_000; } /** From 3ae4dd85c499f9f05d40d84425ad6235757493dd Mon Sep 17 00:00:00 2001 From: dddddanica Date: Thu, 9 Jan 2025 12:46:14 +0000 Subject: [PATCH 5/5] fix(3742): add validation to metametricsId --- .../src/utils/user-segmentation-utils.test.ts | 51 +++++++++++++++++++ .../src/utils/user-segmentation-utils.ts | 41 +++++++++++---- 2 files changed, 83 insertions(+), 9 deletions(-) diff --git a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts index cef18cdb9e..6a8b96f3cd 100644 --- a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts +++ b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts @@ -13,6 +13,15 @@ const MOCK_METRICS_IDS = { MOBILE_MAX: 'ffffffff-ffff-4fff-bfff-ffffffffffff', EXTENSION_MIN: `0x${'0'.repeat(64) as string}`, EXTENSION_MAX: `0x${'f'.repeat(64) as string}`, + UUID_V3: '00000000-0000-3000-8000-000000000000', + INVALID_HEX_NO_PREFIX: + '86bacb9b2bf9a7e8d2b147eadb95ac9aaa26842327cd24afc8bd4b3c1d136420', + INVALID_HEX_SHORT: + '0x86bacb9b2bf9a7e8d2b147eadb95ac9aaa26842327cd24afc8bd4b3c1d13642', + INVALID_HEX_LONG: + '0x86bacb9b2bf9a7e8d2b147eadb95ac9aaa26842327cd24afc8bd4b3c1d1364200', + INVALID_HEX_INVALID_CHARS: + '0x86bacb9b2bf9a7e8d2b147eadb95ac9aaa26842327cd24afc8bd4b3c1d13642g', }; const MOCK_FEATURE_FLAGS = { @@ -125,6 +134,48 @@ describe('user-segmentation-utils', () => { }); }); }); + + describe('MetaMetrics ID validation', () => { + it('throws an error if the MetaMetrics ID is empty', () => { + expect(() => generateDeterministicRandomNumber('')).toThrow( + 'MetaMetrics ID cannot be empty', + ); + }); + + it('throws an error if the MetaMetrics ID is not a valid UUIDv4', () => { + expect(() => + generateDeterministicRandomNumber(MOCK_METRICS_IDS.UUID_V3), + ).toThrow('Invalid UUID version. Expected v4, got v3'); + }); + + it('throws an error if the MetaMetrics ID is not a valid hex string', () => { + expect(() => + generateDeterministicRandomNumber( + MOCK_METRICS_IDS.INVALID_HEX_NO_PREFIX, + ), + ).toThrow('Hex ID must start with 0x prefix'); + }); + + it('throws an error if the MetaMetrics ID is a short hex string', () => { + expect(() => + generateDeterministicRandomNumber(MOCK_METRICS_IDS.INVALID_HEX_SHORT), + ).toThrow('Invalid hex ID length. Expected 64 characters, got 63'); + }); + + it('throws an error if the MetaMetrics ID is a long hex string', () => { + expect(() => + generateDeterministicRandomNumber(MOCK_METRICS_IDS.INVALID_HEX_LONG), + ).toThrow('Invalid hex ID length. Expected 64 characters, got 65'); + }); + + it('throws an error if the MetaMetrics ID contains invalid hex characters', () => { + expect(() => + generateDeterministicRandomNumber( + MOCK_METRICS_IDS.INVALID_HEX_INVALID_CHARS, + ), + ).toThrow('Hex ID contains invalid characters'); + }); + }); }); describe('isFeatureFlagWithScopeValue', () => { diff --git a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts index f0d62cf65e..b14056c453 100644 --- a/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts +++ b/packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts @@ -16,37 +16,60 @@ const MIN_UUID_V4 = '00000000-0000-4000-8000-000000000000'; const MAX_UUID_V4 = 'ffffffff-ffff-4fff-bfff-ffffffffffff'; const MIN_UUID_V4_BIGINT = uuidStringToBigInt(MIN_UUID_V4); const MAX_UUID_V4_BIGINT = uuidStringToBigInt(MAX_UUID_V4); -const UUID_V4_VALUE_RANGE_BIGINT = - MAX_UUID_V4_BIGINT - MIN_UUID_V4_BIGINT; +const UUID_V4_VALUE_RANGE_BIGINT = MAX_UUID_V4_BIGINT - MIN_UUID_V4_BIGINT; /** * Generates a deterministic random number between 0 and 1 based on a metaMetricsId. * This is useful for A/B testing and feature flag rollouts where we want * consistent group assignment for the same user. * @param metaMetricsId - The unique identifier used to generate the deterministic random number. Must be either: - * - A UUIDv4 string (e.g., '123e4567-e89b-12d3-a456-426614174000') - * - A hex string with '0x' prefix (e.g., '0x86bacb9b2bf9a7e8d2b147eadb95ac9aaa26842327cd24afc8bd4b3c1d136420') + * - A UUIDv4 string (e.g., '123e4567-e89b-12d3-a456-426614174000' + * - A hex string with '0x' prefix (e.g., '0x86bacb9b2bf9a7e8d2b147eadb95ac9aaa26842327cd24afc8bd4b3c1d136420') * @returns A number between 0 and 1, deterministically generated from the input ID. - * The same input will always produce the same output. + * The same input will always produce the same output. */ export function generateDeterministicRandomNumber( metaMetricsId: string, ): number { + if (!metaMetricsId) { + throw new Error('MetaMetrics ID cannot be empty'); + } + let idValue: bigint; let maxValue: bigint; + // uuidv4 format - if (uuidValidate(metaMetricsId) && uuidVersion(metaMetricsId) === 4) { - // Normalize the UUIDv4 range to start from 0 by subtracting MIN_UUID_V4_BIGINT. - // This ensures uniform distribution across the entire range, since UUIDv4 - // has restricted bits for version (4) and variant (8-b) that would otherwise skew the distribution + if (uuidValidate(metaMetricsId)) { + if (uuidVersion(metaMetricsId) !== 4) { + throw new Error( + `Invalid UUID version. Expected v4, got v${uuidVersion(metaMetricsId)}`, + ); + } idValue = uuidStringToBigInt(metaMetricsId) - MIN_UUID_V4_BIGINT; maxValue = UUID_V4_VALUE_RANGE_BIGINT; } else { // hex format with 0x prefix + if (!metaMetricsId.startsWith('0x')) { + throw new Error('Hex ID must start with 0x prefix'); + } + const cleanId = metaMetricsId.slice(2); + const EXPECTED_HEX_LENGTH = 64; // 32 bytes = 64 hex characters + + if (cleanId.length !== EXPECTED_HEX_LENGTH) { + throw new Error( + `Invalid hex ID length. Expected ${EXPECTED_HEX_LENGTH} characters, got ${cleanId.length}`, + ); + } + + if (!/^[0-9a-f]+$/iu.test(cleanId)) { + throw new Error('Hex ID contains invalid characters'); + } + idValue = BigInt(`0x${cleanId}`); maxValue = BigInt(`0x${'f'.repeat(cleanId.length)}`); } + // Use BigInt division first, then convert to number to maintain precision return Number((idValue * BigInt(1_000_000)) / maxValue) / 1_000_000; }