From 93213b14313c54fe5aa9e04eda026ef1f5b21f04 Mon Sep 17 00:00:00 2001 From: gomesalexandre Date: Tue, 1 Sep 2026 22:24:31 +0200 Subject: [PATCH] fix(compliance-controller): reconcile compliance-status writes case-insensitively ComplianceController wrote a wallet's compliance status to walletComplianceStatusMap keyed by the caller's raw address casing, but reads went through getWalletComplianceStatus (added in #8820), which tries an exact match first and falls back to a case-insensitive scan. Writing a status for an already-cached wallet under a different casing therefore created a second, stale entry instead of updating the existing one - and depending on which casing a later read used, it could see either the fresh or the stale status for the same wallet. setWalletComplianceStatus now collects every key that case-insensitively matches the incoming address, writes the new status under the first (existing) one, and deletes the rest - healing duplicates already present in persisted state, not just preventing new ones. Both write sites in ComplianceController route through it. --- packages/compliance-controller/CHANGELOG.md | 4 + .../src/ComplianceController.test.ts | 156 ++++++++++++++++++ .../src/ComplianceController.ts | 17 +- packages/compliance-controller/src/utils.ts | 45 +++++ 4 files changed, 219 insertions(+), 3 deletions(-) diff --git a/packages/compliance-controller/CHANGELOG.md b/packages/compliance-controller/CHANGELOG.md index dcac21a245e..d517f9eba34 100644 --- a/packages/compliance-controller/CHANGELOG.md +++ b/packages/compliance-controller/CHANGELOG.md @@ -14,6 +14,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/messenger` from `^1.2.0` to `^2.0.0` ([#9392](https://github.com/MetaMask/core/pull/9392)) - Bump `@metamask/superstruct` from `^3.1.0` to `^3.4.1` ([#9754](https://github.com/MetaMask/core/pull/9754)) +### Fixed + +- Fix `checkWalletCompliance`/`checkWalletsCompliance` writing a wallet's compliance status under the caller's raw address casing, which could leave a stale duplicate cache entry for a wallet already cached under a different casing (or produce two persisted entries for the same wallet if one was ever checked under two casings). Writes now reconcile with any existing case-insensitively-matching entry, healing duplicates already present in persisted state. + ## [2.1.0] ### Added diff --git a/packages/compliance-controller/src/ComplianceController.test.ts b/packages/compliance-controller/src/ComplianceController.test.ts index 72f276bf094..34ede70779d 100644 --- a/packages/compliance-controller/src/ComplianceController.test.ts +++ b/packages/compliance-controller/src/ComplianceController.test.ts @@ -338,6 +338,94 @@ describe('ComplianceController', () => { }); }); }); + + it('reconciles a later write for the same address under a different casing into the existing cache entry, instead of creating a stale duplicate', async () => { + await withController(async ({ controller, rootMessenger }) => { + let blocked = false; + rootMessenger.registerActionHandler( + 'ComplianceService:checkWalletCompliance', + async (address) => ({ + address, + blocked, + }), + ); + + // First check: not blocked, written under the checksummed casing. + await controller.checkWalletCompliance(CHECKSUM_EVM_ADDRESS); + + // The wallet becomes sanctioned; the second check is written under a + // different (lowercase) casing for the very same address. + blocked = true; + await controller.checkWalletCompliance(LOWERCASE_EVM_ADDRESS); + + // The cache must reconcile to a single entry per address, not one + // stale entry per casing ever seen. + expect( + Object.keys(controller.state.walletComplianceStatusMap), + ).toHaveLength(1); + + // Every casing must now resolve to the fresh, blocked status. + expect( + selectIsWalletBlocked(CHECKSUM_EVM_ADDRESS)(controller.state), + ).toBe(true); + expect( + selectIsWalletBlocked(LOWERCASE_EVM_ADDRESS)(controller.state), + ).toBe(true); + + // The existing key's casing is preserved rather than being replaced + // by the caller's casing. + expect( + Object.prototype.hasOwnProperty.call( + controller.state.walletComplianceStatusMap, + CHECKSUM_EVM_ADDRESS, + ), + ).toBe(true); + }); + }); + + it('heals a duplicate entry that already exists under two casings for the same address (e.g. state persisted before this reconciliation shipped)', async () => { + await withController( + { + options: { + state: { + walletComplianceStatusMap: { + [CHECKSUM_EVM_ADDRESS]: { + address: CHECKSUM_EVM_ADDRESS, + blocked: false, + checkedAt: '2026-01-01T00:00:00.000Z', + }, + [LOWERCASE_EVM_ADDRESS]: { + address: LOWERCASE_EVM_ADDRESS, + blocked: false, + checkedAt: '2026-01-01T00:00:00.000Z', + }, + }, + }, + }, + }, + async ({ controller, rootMessenger }) => { + rootMessenger.registerActionHandler( + 'ComplianceService:checkWalletCompliance', + async (address) => ({ address, blocked: true }), + ); + + // A write under EITHER pre-existing casing must collapse both + // stale duplicate entries into one, not just update the one that + // happens to exact-match the caller's casing. + await controller.checkWalletCompliance(LOWERCASE_EVM_ADDRESS); + + expect( + Object.keys(controller.state.walletComplianceStatusMap), + ).toHaveLength(1); + expect( + selectIsWalletBlocked(CHECKSUM_EVM_ADDRESS)(controller.state), + ).toBe(true); + expect( + selectIsWalletBlocked(LOWERCASE_EVM_ADDRESS)(controller.state), + ).toBe(true); + }, + ); + }); }); describe('ComplianceController:checkWalletsCompliance', () => { @@ -349,6 +437,74 @@ describe('ComplianceController', () => { jest.useRealTimers(); }); + it('reconciles a batch write for an address already cached under a different casing, instead of creating a stale duplicate', async () => { + await withController( + { + options: { + state: { + walletComplianceStatusMap: { + [CHECKSUM_EVM_ADDRESS]: { + address: CHECKSUM_EVM_ADDRESS, + blocked: false, + checkedAt: '2026-01-01T00:00:00.000Z', + }, + }, + }, + }, + }, + async ({ controller, rootMessenger }) => { + rootMessenger.registerActionHandler( + 'ComplianceService:checkWalletsCompliance', + async (addresses) => + addresses.map((addr) => ({ address: addr, blocked: true })), + ); + + await rootMessenger.call( + 'ComplianceController:checkWalletsCompliance', + [LOWERCASE_EVM_ADDRESS], + ); + + expect( + Object.keys(controller.state.walletComplianceStatusMap), + ).toHaveLength(1); + expect( + selectIsWalletBlocked(CHECKSUM_EVM_ADDRESS)(controller.state), + ).toBe(true); + expect( + selectIsWalletBlocked(LOWERCASE_EVM_ADDRESS)(controller.state), + ).toBe(true); + }, + ); + }); + + it('reconciles two aliases of the same address requested within a single batch call into one entry, with the later result winning', async () => { + await withController(async ({ controller, rootMessenger }) => { + rootMessenger.registerActionHandler( + 'ComplianceService:checkWalletsCompliance', + async (addresses) => + addresses.map((addr) => ({ + address: addr, + blocked: addr === LOWERCASE_EVM_ADDRESS, + })), + ); + + await rootMessenger.call( + 'ComplianceController:checkWalletsCompliance', + [CHECKSUM_EVM_ADDRESS, LOWERCASE_EVM_ADDRESS], + ); + + expect( + Object.keys(controller.state.walletComplianceStatusMap), + ).toStrictEqual([CHECKSUM_EVM_ADDRESS]); + expect( + selectIsWalletBlocked(CHECKSUM_EVM_ADDRESS)(controller.state), + ).toBe(true); + expect( + selectIsWalletBlocked(LOWERCASE_EVM_ADDRESS)(controller.state), + ).toBe(true); + }); + }); + it('calls the service, persists all results to state, and returns statuses', async () => { await withController(async ({ controller, rootMessenger }) => { rootMessenger.registerActionHandler( diff --git a/packages/compliance-controller/src/ComplianceController.ts b/packages/compliance-controller/src/ComplianceController.ts index 720441dde30..418983e3cef 100644 --- a/packages/compliance-controller/src/ComplianceController.ts +++ b/packages/compliance-controller/src/ComplianceController.ts @@ -12,7 +12,10 @@ import type { ComplianceServiceCheckWalletsComplianceAction, } from './ComplianceService-method-action-types.js'; import type { WalletComplianceStatus } from './types.js'; -import { getWalletComplianceStatus } from './utils.js'; +import { + getWalletComplianceStatus, + setWalletComplianceStatus, +} from './utils.js'; // === GENERAL === @@ -203,7 +206,11 @@ export class ComplianceController extends BaseController< }; this.update((draftState) => { - draftState.walletComplianceStatusMap[address] = status; + setWalletComplianceStatus( + draftState.walletComplianceStatusMap, + address, + status, + ); draftState.lastCheckedAt = now; }); @@ -249,7 +256,11 @@ export class ComplianceController extends BaseController< this.update((draftState) => { for (let idx = 0; idx < statuses.length; idx++) { const callerAddress = addresses[idx]; - draftState.walletComplianceStatusMap[callerAddress] = statuses[idx]; + setWalletComplianceStatus( + draftState.walletComplianceStatusMap, + callerAddress, + statuses[idx], + ); } draftState.lastCheckedAt = now; }); diff --git a/packages/compliance-controller/src/utils.ts b/packages/compliance-controller/src/utils.ts index cfc996186c0..70fb9c71be2 100644 --- a/packages/compliance-controller/src/utils.ts +++ b/packages/compliance-controller/src/utils.ts @@ -23,3 +23,48 @@ export const getWalletComplianceStatus = ( return matchingAddress ? statusMap[matchingAddress] : undefined; }; + +/** + * Writes a wallet's compliance status into the status map, reconciling it + * with any existing entry (or entries) for the same address under a + * different casing instead of leaving or creating a stale duplicate. + * + * A wallet's cache entry can only ever live under one key at a time, + * regardless of the casing used across separate calls. All keys that + * case-insensitively match the incoming address (the exact key, if present, + * included) are collected; the new status is written under the FIRST such + * key found (preferring key insertion order, so already-persisted state + * keeps its existing key rather than being rekeyed under the caller's + * casing), and every OTHER matching key is deleted. This also heals + * duplicate entries that predate this reconciliation (e.g. state persisted + * before this fix shipped, where the same wallet could have ended up cached + * under two different casings) the next time either casing is written. + * + * @param statusMap - The status map to write into, in place. + * @param address - The wallet address being written. + * @param status - The compliance status to store for the address. + */ +export const setWalletComplianceStatus = ( + statusMap: Record, + address: string, + status: WalletComplianceStatus, +): void => { + if (!isValidHexAddress(address, { allowNonPrefixed: false })) { + statusMap[address] = status; + return; + } + + const matchingAddresses = Object.keys(statusMap).filter( + (cachedAddress) => + isValidHexAddress(cachedAddress, { allowNonPrefixed: false }) && + isEqualCaseInsensitive(cachedAddress, address), + ); + + const [canonicalAddress, ...staleAddresses] = matchingAddresses; + + for (const staleAddress of staleAddresses) { + delete statusMap[staleAddress]; + } + + statusMap[canonicalAddress ?? address] = status; +};