Normalize addresses in name controller (#1732)

Normalize addresses and chain IDs in the NameController state by storing them in lowercase.
Replace the getChainId callback with the variation option in the SetNameRequest and UpdateProposedNamesRequest.

Author: matthewwalsh0

packages/name-controller/src/NameController.test.ts

@@ -11,6 +11,7 @@ import type {
 import { NameType } from './types';
 
 const DEFAULT_UPDATE_DELAY = 60 * 2; // 2 Minutes
+const DEFAULT_VARIATION = '';
 
 const controllerName = 'NameController';
 
@@ -71,7 +72,6 @@ export type NameControllerMessenger = RestrictedControllerMessenger<
 >;
 
 export type NameControllerOptions = {
-  getChainId: () => string;
   messenger: NameControllerMessenger;
   providers: NameProvider[];
   state?: Partial<NameControllerState>;
@@ -83,6 +83,7 @@ export type UpdateProposedNamesRequest = {
   type: NameType;
   sourceIds?: string[];
   onlyUpdateAfterDelay?: boolean;
+  variation?: string;
 };
 
 export type UpdateProposedNamesResult = {
@@ -94,6 +95,7 @@ export type SetNameRequest = {
   type: NameType;
   name: string | null;
   sourceId?: string;
+  variation?: string;
 };
 
 /**
@@ -104,8 +106,6 @@ export class NameController extends BaseControllerV2<
   NameControllerState,
   NameControllerMessenger
 > {
-  #getChainId: () => string;
-
   #providers: NameProvider[];
 
   #updateDelay: number;
@@ -114,14 +114,12 @@ export class NameController extends BaseControllerV2<
    * Construct a Name controller.
    *
    * @param options - Controller options.
-   * @param options.getChainId - Callback that returns the chain ID of the current network.
    * @param options.messenger - Restricted controller messenger for the name controller.
    * @param options.providers - Array of name provider instances to propose names.
    * @param options.state - Initial state to set on the controller.
    * @param options.updateDelay - The delay in seconds before a new request to a source should be made.
    */
   constructor({
-    getChainId,
     messenger,
     providers,
     state,
@@ -134,7 +132,6 @@ export class NameController extends BaseControllerV2<
       state: { ...getDefaultState(), ...state },
     });
 
-    this.#getChainId = getChainId;
     this.#providers = providers;
     this.#updateDelay = updateDelay ?? DEFAULT_UPDATE_DELAY;
   }
@@ -147,14 +144,15 @@ export class NameController extends BaseControllerV2<
    * @param request.sourceId - Optional ID of the source of the proposed name.
    * @param request.type - Type of value to set the name for.
    * @param request.value - Value to set the name for.
+   * @param request.variation - Variation of the raw value to set the name for. The chain ID if the type is Ethereum address.
    */
   setName(request: SetNameRequest) {
     this.#validateSetNameRequest(request);
 
-    const { value, type, name, sourceId: requestSourceId } = request;
+    const { value, type, name, sourceId: requestSourceId, variation } = request;
     const sourceId = requestSourceId ?? null;
 
-    this.#updateEntry(value, type, (entry: NameEntry) => {
+    this.#updateEntry(value, type, variation, (entry: NameEntry) => {
       entry.name = name;
       entry.sourceId = sourceId;
     });
@@ -167,19 +165,18 @@ export class NameController extends BaseControllerV2<
    * @param request.value - Value to update the proposed names for.
    * @param request.type - Type of value to update the proposed names for.
    * @param request.sourceIds - Optional array of source IDs to limit which sources are used by the providers. If not provided, all sources in all providers will be used.
+   * @param request.variation - Variation of the raw value to update proposed names for. The chain ID if the type is Ethereum address.
    * @returns The updated proposed names for the value.
    */
   async updateProposedNames(
     request: UpdateProposedNamesRequest,
   ): Promise<UpdateProposedNamesResult> {
     this.#validateUpdateProposedNamesRequest(request);
 
-    const chainId = this.#getChainId();
-
     const providerResponses = (
       await Promise.all(
         this.#providers.map((provider) =>
-          this.#getProviderResponse(request, chainId, provider),
+          this.#getProviderResponse(request, provider),
         ),
       )
     ).filter((response) => Boolean(response)) as NameProviderResult[];
@@ -194,10 +191,10 @@ export class NameController extends BaseControllerV2<
     request: UpdateProposedNamesRequest,
     providerResponses: NameProviderResult[],
   ) {
-    const { value, type } = request;
+    const { value, type, variation } = request;
     const currentTime = this.#getCurrentTimeSeconds();
 
-    this.#updateEntry(value, type, (entry: NameEntry) => {
+    this.#updateEntry(value, type, variation, (entry: NameEntry) => {
       this.#removeDormantProposedNames(entry.proposedNames, type);
 
       for (const providerResponse of providerResponses) {
@@ -268,27 +265,30 @@ export class NameController extends BaseControllerV2<
 
   async #getProviderResponse(
     request: UpdateProposedNamesRequest,
-    chainId: string,
     provider: NameProvider,
   ): Promise<NameProviderResult | undefined> {
     const {
       value,
       type,
       sourceIds: requestedSourceIds,
       onlyUpdateAfterDelay,
+      variation,
     } = request;
 
-    const variationKey = this.#getTypeVariationKey(type);
+    /* istanbul ignore next */
+    const variationKey = variation ?? DEFAULT_VARIATION;
     const supportedSourceIds = this.#getSourceIds(provider, type);
     const currentTime = this.#getCurrentTimeSeconds();
+    const normalizedValue = this.#normalizeValue(value, type);
 
     const matchingSourceIds = supportedSourceIds.filter((sourceId) => {
       if (requestedSourceIds && !requestedSourceIds.includes(sourceId)) {
         return false;
       }
 
       if (onlyUpdateAfterDelay) {
-        const entry = this.state.names[type]?.[value]?.[variationKey] ?? {};
+        const entry =
+          this.state.names[type]?.[normalizedValue]?.[variationKey] ?? {};
         const proposedNamesEntry = entry.proposedNames?.[sourceId] ?? {};
         const lastRequestTime = proposedNamesEntry.lastRequestTime ?? 0;
         const updateDelay = proposedNamesEntry.updateDelay ?? this.#updateDelay;
@@ -306,10 +306,10 @@ export class NameController extends BaseControllerV2<
     }
 
     const providerRequest: NameProviderRequest = {
-      chainId,
-      value,
+      value: this.#normalizeValue(value, type),
       type,
       sourceIds: requestedSourceIds ? matchingSourceIds : undefined,
+      variation: this.#normalizeVariation(variationKey, type),
     };
 
     let responseError: unknown | undefined;
@@ -374,65 +374,85 @@ export class NameController extends BaseControllerV2<
     };
   }
 
+  #normalizeValue(value: string, type: NameType): string {
+    /* istanbul ignore next */
+    switch (type) {
+      case NameType.ETHEREUM_ADDRESS:
+        return value.toLowerCase();
+
+      default:
+        return value;
+    }
+  }
+
+  #normalizeVariation(variation: string, type: NameType): string {
+    /* istanbul ignore next */
+    switch (type) {
+      case NameType.ETHEREUM_ADDRESS:
+        return variation.toLowerCase();
+
+      default:
+        return variation;
+    }
+  }
+
   #updateEntry(
     value: string,
     type: NameType,
+    variation: string | undefined,
     callback: (entry: NameEntry) => void,
   ) {
-    const variationKey = this.#getTypeVariationKey(type);
+    /* istanbul ignore next */
+    const variationKey = variation ?? DEFAULT_VARIATION;
+    const normalizedValue = this.#normalizeValue(value, type);
+    const normalizedVariation = this.#normalizeVariation(variationKey, type);
 
     this.update((state) => {
       const typeEntries = state.names[type] || {};
       state.names[type] = typeEntries;
 
-      const variationEntries = typeEntries[value] || {};
-      typeEntries[value] = variationEntries;
+      const variationEntries = typeEntries[normalizedValue] || {};
+      typeEntries[normalizedValue] = variationEntries;
 
-      const entry = variationEntries[variationKey] ?? {
+      const entry = variationEntries[normalizedVariation] ?? {
         proposedNames: {},
         name: null,
         sourceId: null,
       };
-      variationEntries[variationKey] = entry;
+      variationEntries[normalizedVariation] = entry;
 
       callback(entry);
     });
   }
 
-  #getTypeVariationKey(type: NameType): string {
-    switch (type) {
-      default: {
-        return this.#getChainId();
-      }
-    }
-  }
-
   #getCurrentTimeSeconds(): number {
     return Math.round(Date.now() / 1000);
   }
 
   #validateSetNameRequest(request: SetNameRequest) {
-    const { name, value, type, sourceId } = request;
+    const { name, value, type, sourceId, variation } = request;
     const errorMessages: string[] = [];
 
     this.#validateValue(value, errorMessages);
     this.#validateType(type, errorMessages);
     this.#validateName(name, errorMessages);
     this.#validateSourceId(sourceId, type, name, errorMessages);
+    this.#validateVariation(variation, type, errorMessages);
 
     if (errorMessages.length) {
       throw new Error(errorMessages.join(' '));
     }
   }
 
   #validateUpdateProposedNamesRequest(request: UpdateProposedNamesRequest) {
-    const { value, type, sourceIds } = request;
+    const { value, type, sourceIds, variation } = request;
     const errorMessages: string[] = [];
 
     this.#validateValue(value, errorMessages);
     this.#validateType(type, errorMessages);
     this.#validateSourceIds(sourceIds, type, errorMessages);
     this.#validateDuplicateSourceIds(type, errorMessages);
+    this.#validateVariation(variation, type, errorMessages);
 
     if (errorMessages.length) {
       throw new Error(errorMessages.join(' '));
@@ -536,6 +556,26 @@ export class NameController extends BaseControllerV2<
     }
   }
 
+  #validateVariation(
+    variation: string | undefined,
+    type: string,
+    errorMessages: string[],
+  ) {
+    if (type !== NameType.ETHEREUM_ADDRESS) {
+      return;
+    }
+
+    if (
+      !variation?.length ||
+      typeof variation !== 'string' ||
+      !variation.match(/^0x[0-9A-Fa-f]+$/u)
+    ) {
+      errorMessages.push(
+        `Must specify a chain ID in hexidecimal format for variation when using '${type}' type.`,
+      );
+    }
+  }
+
   #getAllSourceIds(type: NameType): string[] {
     return (
       this.#providers

packages/name-controller/src/NameController.ts

@@ -39,7 +39,7 @@ describe('ENSNameProvider', () => {
 
       const response = await provider.getProposedNames({
         value: VALUE_MOCK,
-        chainId: CHAIN_ID_MOCK,
+        variation: CHAIN_ID_MOCK,
         type: NameType.ETHEREUM_ADDRESS,
       });
 
@@ -59,7 +59,7 @@ describe('ENSNameProvider', () => {
 
       await provider.getProposedNames({
         value: VALUE_MOCK,
-        chainId: CHAIN_ID_MOCK,
+        variation: CHAIN_ID_MOCK,
         type: NameType.ETHEREUM_ADDRESS,
       });
 
@@ -75,7 +75,7 @@ describe('ENSNameProvider', () => {
 
       const response = await provider.getProposedNames({
         value: VALUE_MOCK,
-        chainId: CHAIN_ID_MOCK,
+        variation: CHAIN_ID_MOCK,
         type: NameType.ETHEREUM_ADDRESS,
       });
 
@@ -96,7 +96,7 @@ describe('ENSNameProvider', () => {
       await expect(
         provider.getProposedNames({
           value: VALUE_MOCK,
-          chainId: CHAIN_ID_MOCK,
+          variation: CHAIN_ID_MOCK,
           type: NameType.ETHEREUM_ADDRESS,
         }),
       ).rejects.toThrow('TestError');

packages/name-controller/src/providers/ens.test.ts

@@ -55,7 +55,7 @@ export class ENSNameProvider implements NameProvider {
       };
     }
 
-    const { value, chainId } = request;
+    const { value, variation: chainId } = request;
 
     log('Invoking callback', { value, chainId });