Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/remote-feature-flag-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
"@metamask/controller-utils": "^11.16.0",
"@metamask/messenger": "^0.3.0",
"@metamask/utils": "^11.8.1",
"@noble/hashes": "^1.8.0",
"uuid": "^8.3.2"
},
"devDependencies": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,11 +321,13 @@ describe('RemoteFeatureFlagController', () => {
});
await controller.updateRemoteFeatureFlags();

// With MOCK_METRICS_ID + 'testFlagForThreshold' hashed:
// Threshold = 0.380673, which falls in groupB range (0.3 < t <= 0.5)
expect(
controller.state.remoteFeatureFlags.testFlagForThreshold,
).toStrictEqual({
name: 'groupC',
value: 'valueC',
name: 'groupB',
value: 'valueB',
});
});

Expand All @@ -343,6 +345,95 @@ describe('RemoteFeatureFlagController', () => {
controller.state.remoteFeatureFlags;
expect(nonThresholdFlags).toStrictEqual(MOCK_FLAGS);
});

it('assigns users to different groups for different feature flags', async () => {
// Arrange
const mockFlags = {
featureA: [
{
name: 'groupA1',
scope: { type: 'threshold', value: 0.5 },
value: 'A1',
},
{
name: 'groupA2',
scope: { type: 'threshold', value: 1.0 },
value: 'A2',
},
],
featureB: [
{
name: 'groupB1',
scope: { type: 'threshold', value: 0.5 },
value: 'B1',
},
{
name: 'groupB2',
scope: { type: 'threshold', value: 1.0 },
value: 'B2',
},
],
};
const clientConfigApiService = buildClientConfigApiService({
remoteFeatureFlags: mockFlags,
});
const controller = createController({
clientConfigApiService,
getMetaMetricsId: () => MOCK_METRICS_ID,
});

// Act
await controller.updateRemoteFeatureFlags();

// Assert - User gets different groups because each flag uses unique seed
const { featureA, featureB } = controller.state.remoteFeatureFlags;
// featureA: hash(MOCK_METRICS_ID + 'featureA') → threshold 0.966682 → groupA2
expect(featureA).toStrictEqual({ name: 'groupA2', value: 'A2' });
// featureB: hash(MOCK_METRICS_ID + 'featureB') → threshold 0.398654 → groupB1
expect(featureB).toStrictEqual({ name: 'groupB1', value: 'B1' });
// Different groups proves independence!
});

it('assigns users to same group for same feature flag on multiple calls', async () => {
// Arrange
const mockFlags = {
testFlag: [
{
name: 'control',
scope: { type: 'threshold', value: 0.5 },
value: false,
},
{
name: 'treatment',
scope: { type: 'threshold', value: 1.0 },
value: true,
},
],
};
const clientConfigApiService = buildClientConfigApiService({
remoteFeatureFlags: mockFlags,
});

// Act - Create two separate controllers with same metaMetricsId
const controller1 = createController({
clientConfigApiService,
getMetaMetricsId: () => MOCK_METRICS_ID,
});
await controller1.updateRemoteFeatureFlags();
const firstResult = controller1.state.remoteFeatureFlags.testFlag;

const controller2 = createController({
clientConfigApiService,
getMetaMetricsId: () => MOCK_METRICS_ID,
});
await controller2.updateRemoteFeatureFlags();
const secondResult = controller2.state.remoteFeatureFlags.testFlag;

// Assert - Same user always gets same group (deterministic)
// testFlag: hash(MOCK_METRICS_ID + 'testFlag') → threshold 0.496587 → control
expect(firstResult).toStrictEqual(secondResult);
expect(firstResult).toStrictEqual({ name: 'control', value: false });
});
});

describe('enable and disable', () => {
Expand Down Expand Up @@ -632,10 +723,11 @@ describe('RemoteFeatureFlagController', () => {
const { multiVersionABFlag, regularFlag } =
controller.state.remoteFeatureFlags;
// Should select 13.1.0 version and then apply A/B testing to that array
// With MOCK_METRICS_ID threshold, should select groupC (threshold 1.0)
// With MOCK_METRICS_ID + 'multiVersionABFlag' hashed:
// Threshold = 0.094878, which falls in groupA range (t <= 0.3)
expect(multiVersionABFlag).toStrictEqual({
name: 'groupC',
value: { feature: 'C', enabled: true },
name: 'groupA',
value: { feature: 'A', enabled: true },
});
expect(regularFlag).toBe(true);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
FeatureFlagScopeValue,
} from './remote-feature-flag-controller-types';
import {
createDeterministicSeed,
generateDeterministicRandomNumber,
isFeatureFlagWithScopeValue,
} from './utils/user-segmentation-utils';
Expand Down Expand Up @@ -278,7 +279,6 @@ export class RemoteFeatureFlagController extends BaseController<
): Promise<FeatureFlags> {
const processedRemoteFeatureFlags: FeatureFlags = {};
const metaMetricsId = this.#getMetaMetricsId();
const thresholdValue = generateDeterministicRandomNumber(metaMetricsId);

for (const [
remoteFeatureFlagName,
Expand All @@ -291,7 +291,13 @@ export class RemoteFeatureFlagController extends BaseController<
continue;
}

if (Array.isArray(processedValue) && thresholdValue) {
if (Array.isArray(processedValue)) {
const deterministicSeed = createDeterministicSeed(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a slow cryptographic operation and it runs once per flag, but we don't need to run this for all flags. It's unnecessary unless the flag has a threshold.

Perhaps we can calculate this later, after we've determined that there is a threshold on this specific flag? That way it can be skipped for non-threshold flags, which is the majority of them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also not ideal to run this every time flags get updated 🤔.

metaMetricsId,
remoteFeatureFlagName,
);
const thresholdValue =
generateDeterministicRandomNumber(deterministicSeed);
const selectedGroup = processedValue.find(
(featureFlag): featureFlag is FeatureFlagScopeValue => {
if (!isFeatureFlagWithScopeValue(featureFlag)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { v4 as uuidV4 } from 'uuid';

import {
createDeterministicSeed,
generateDeterministicRandomNumber,
isFeatureFlagWithScopeValue,
} from './user-segmentation-utils';
Expand Down Expand Up @@ -41,6 +42,94 @@ const MOCK_FEATURE_FLAGS = {
};

describe('user-segmentation-utils', () => {
describe('createDeterministicSeed', () => {
it('generates deterministic hash from same input', () => {
// Arrange
const metaMetricsId = 'f9e8d7c6-b5a4-4210-9876-543210fedcba';
const flagName = 'testFlag';

// Act
const hash1 = createDeterministicSeed(metaMetricsId, flagName);
const hash2 = createDeterministicSeed(metaMetricsId, flagName);

// Assert
expect(hash1).toBe(hash2);
expect(hash1).toMatch(/^0x[0-9a-f]{64}$/u);
});

it('generates different hashes for different inputs', () => {
// Arrange
const metaMetricsId1 = 'f9e8d7c6-b5a4-4210-9876-543210fedcba';
const metaMetricsId2 = '123e4567-e89b-12d3-a456-426614174000';
const flagName = 'testFlag';

// Act
const hash1 = createDeterministicSeed(metaMetricsId1, flagName);
const hash2 = createDeterministicSeed(metaMetricsId2, flagName);

// Assert
expect(hash1).not.toBe(hash2);
});

it('generates different hashes when concatenating different flag names', () => {
// Arrange
const metaMetricsId = 'f9e8d7c6-b5a4-4210-9876-543210fedcba';
const flagName1 = 'featureA';
const flagName2 = 'featureB';

// Act
const seed1 = createDeterministicSeed(metaMetricsId, flagName1);
const seed2 = createDeterministicSeed(metaMetricsId, flagName2);

// Assert
expect(seed1).not.toBe(seed2);
expect(seed1).toMatch(/^0x[0-9a-f]{64}$/u);
expect(seed2).toMatch(/^0x[0-9a-f]{64}$/u);
});

it('produces valid hex format for generateDeterministicRandomNumber', () => {
// Arrange
const metaMetricsId = 'f9e8d7c6-b5a4-4210-9876-543210fedcba';
const flagName = 'anyFlagName123';

// Act
const hash = createDeterministicSeed(metaMetricsId, flagName);
const randomNumber = generateDeterministicRandomNumber(hash);

// Assert
expect(randomNumber).toBeGreaterThanOrEqual(0);
expect(randomNumber).toBeLessThanOrEqual(1);
});

it('throws error when metaMetricsId is empty', () => {
// Arrange
const emptyMetaMetricsId = '';
const flagName = 'testFlag';

// Act & Assert
expect(() =>
createDeterministicSeed(emptyMetaMetricsId, flagName),
).toThrow('MetaMetrics ID cannot be empty');
});

it('produces same hash regardless of input casing', () => {
// Arrange
const metaMetricsIdLower = 'f9e8d7c6-b5a4-4210-9876-543210fedcba';
const metaMetricsIdUpper = 'F9E8D7C6-B5A4-4210-9876-543210FEDCBA';
const metaMetricsIdMixed = 'F9e8D7c6-B5a4-4210-9876-543210FedCBA';
const flagName = 'testFlag';

// Act
const hashLower = createDeterministicSeed(metaMetricsIdLower, flagName);
const hashUpper = createDeterministicSeed(metaMetricsIdUpper, flagName);
const hashMixed = createDeterministicSeed(metaMetricsIdMixed, flagName);

// Assert - All should produce same hash (case-insensitive)
expect(hashLower).toBe(hashUpper);
expect(hashLower).toBe(hashMixed);
});
});

describe('generateDeterministicRandomNumber', () => {
describe('Mobile client new implementation (uuidv4)', () => {
it('generates consistent results for same uuidv4', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import type { Json } from '@metamask/utils';
import { sha256 } from '@noble/hashes/sha256';
import { bytesToHex } from '@noble/hashes/utils';
import { validate as uuidValidate, version as uuidVersion } from 'uuid';

import type { FeatureFlagScopeValue } from '../remote-feature-flag-controller-types';
Expand All @@ -19,6 +21,33 @@ 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;

/**
* Creates a deterministic hex ID by hashing the input seed.
* This ensures consistent group assignment for A/B testing across different feature flags.
* Normalizes inputs to lowercase to ensure case-insensitive consistency.
*
* @param metaMetricsId - The user's MetaMetrics ID (must be non-empty)
* @param featureFlagName - The feature flag name to create unique seed per flag
* @returns A hex string with '0x' prefix suitable for generateDeterministicRandomNumber
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 0x-prefixed input for generateDeterministicRandomNumber is meant to support legacy metrics IDs. We had hoped to remove that functionality when dropping support for those legacy IDs. Migrating to a format we planned to deprecate is not ideal.

* @throws Error if metaMetricsId is empty
*/
export function createDeterministicSeed(
metaMetricsId: string,
featureFlagName: string,
): string {
if (!metaMetricsId) {
throw new Error('MetaMetrics ID cannot be empty');
}

// Normalize to lowercase to ensure case-insensitive consistency
const normalizedId = metaMetricsId.toLowerCase();
const normalizedFlagName = featureFlagName.toLowerCase();
const seed = normalizedId + normalizedFlagName;
const hashBuffer = sha256(seed);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If async is OK here we should consider using crypto.subtle.digest

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just upstreamed this which may be useful here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FrederikBolding can you expand on that ?

I've used sha256 from noble/hashes to keep consistency with what I've noticed in other packages.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem to be published/exported yet in the current version of metamask/utils

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was published earlier today!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHA-256 using native code will perform better across mobile and extension! We automatically fall back to noble if unavailable

const hash = bytesToHex(hashBuffer);
return `0x${hash}`;
}

/**
* 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
Expand Down
1 change: 1 addition & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4632,6 +4632,7 @@ __metadata:
"@metamask/controller-utils": "npm:^11.16.0"
"@metamask/messenger": "npm:^0.3.0"
"@metamask/utils": "npm:^11.8.1"
"@noble/hashes": "npm:^1.8.0"
"@ts-bridge/cli": "npm:^0.6.4"
"@types/jest": "npm:^27.4.1"
deepmerge: "npm:^4.2.2"
Expand Down
Loading