fix: verify classic assets by code and issuer, not issuer alone

extension/src/popup/helpers/__tests__/assetList.test.ts

@@ -12,19 +12,33 @@ const ISSUER = "GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN";
 const CONTRACT = "CCV3NAKLIBBNSJNNTV2AZVRX6VODUDWK4TVYILE5MW6R45SSQJS5VCAM";
 
 describe("assetMatchesListItem", () => {
-  // Issuer-alone matching is the long-standing behaviour of the verified-list
-  // membership tests, tracked in stellar/wallet-eng-monorepo#76. Pinned here as
-  // current behaviour, not as the intended contract — a fix for that issue is
-  // expected to require the code to match too, and to change this expectation.
-  it("matches on a shared issuer, without comparing the code", () => {
+  it("matches on a shared code and issuer", () => {
     expect(
       assetMatchesListItem(
-        { issuer: ISSUER },
-        { issuer: ISSUER, contract: undefined },
+        { code: "USDE", issuer: ISSUER },
+        { code: "USDE", issuer: ISSUER, contract: undefined },
       ),
     ).toBe(true);
   });
 
+  it("does not match a different code from the same issuer", () => {
+    expect(
+      assetMatchesListItem(
+        { code: "USDe", issuer: ISSUER },
+        { code: "USDE", issuer: ISSUER },
+      ),
+    ).toBe(false);
+  });
+
+  it("does not match on the issuer alone", () => {
+    expect(
+      assetMatchesListItem(
+        { issuer: ISSUER },
+        { code: "USDE", issuer: ISSUER },
+      ),
+    ).toBe(false);
+  });
+
   it("matches on a shared contract", () => {
     expect(
       assetMatchesListItem({ contract: CONTRACT }, { contract: CONTRACT }),
@@ -37,7 +51,10 @@ describe("assetMatchesListItem", () => {
 
   it("does not match a contract-less asset against a contract-less entry", () => {
     expect(
-      assetMatchesListItem({ issuer: ISSUER }, { issuer: "GDIFFERENT" }),
+      assetMatchesListItem(
+        { code: "USDE", issuer: ISSUER },
+        { code: "USDE", issuer: "GDIFFERENT" },
+      ),
     ).toBe(false);
   });
 });
@@ -100,4 +117,67 @@ describe("splitVerifiedAssetCurrency", () => {
     expect(verifiedAssets).toEqual([]);
     expect(unverifiedAssets).toEqual([xlmCodedRow]);
   });
+  it("verifies only the listed code from a listed issuer", async () => {
+    const listedRow = mapStellarExpertRecord(
+      { asset: `USDE-${ISSUER}-1` },
+      MAINNET,
+    );
+    const sameIssuerRow = mapStellarExpertRecord(
+      { asset: `USDe-${ISSUER}-1` },
+      MAINNET,
+    );
+    jest.spyOn(TokenListHelpers, "schemaValidatedAssetList").mockResolvedValue({
+      assets: [
+        {
+          code: "USDE",
+          issuer: ISSUER,
+          contract: CONTRACT,
+          domain: "",
+          icon: "",
+          decimals: 7,
+        },
+      ],
+      errors: null,
+    });
+
+    const { verifiedAssets, unverifiedAssets } =
+      await splitVerifiedAssetCurrency({
+        networkDetails: MAINNET,
+        assets: [listedRow, sameIssuerRow],
+        assetsListsDetails: {} as never,
+        cachedAssetLists: [emptyList],
+      });
+
+    expect(verifiedAssets).toEqual([listedRow]);
+    expect(unverifiedAssets).toEqual([sameIssuerRow]);
+  });
+
+  it("verifies a contract token listed by contract id", async () => {
+    const contractRow = mapStellarExpertRecord(
+      { asset: CONTRACT, code: "TKN" },
+      MAINNET,
+    );
+    jest.spyOn(TokenListHelpers, "schemaValidatedAssetList").mockResolvedValue({
+      assets: [
+        {
+          code: "TKN",
+          issuer: "",
+          contract: CONTRACT,
+          domain: "",
+          icon: "",
+          decimals: 7,
+        },
+      ],
+      errors: null,
+    });
+
+    const { verifiedAssets } = await splitVerifiedAssetCurrency({
+      networkDetails: MAINNET,
+      assets: [contractRow],
+      assetsListsDetails: {} as never,
+      cachedAssetLists: [emptyList],
+    });
+
+    expect(verifiedAssets).toEqual([contractRow]);
+  });
 });

extension/src/popup/helpers/assetList.ts

@@ -1,4 +1,5 @@
 import { NetworkDetails } from "@shared/constants/stellar";
+import { getCanonicalFromAsset } from "@shared/helpers/stellar";
 import {
   AssetsLists,
   AssetListReponseItem,
@@ -36,15 +37,10 @@ export const splitVerifiedAssetCurrency = async ({
       validatedAssets.push(...validatedList.assets);
     }
   }
-  // make a unique set of contract IDs and issuers
+  // make a unique set of the contract IDs and code:issuer pairs the lists vouch for
   const verifiedIds = new Set<string>();
   for (const validAsset of validatedAssets) {
-    if (validAsset.contract && !verifiedIds.has(validAsset.contract)) {
-      verifiedIds.add(validAsset.contract);
-    }
-    if (!verifiedIds.has(validAsset.issuer)) {
-      verifiedIds.add(validAsset.issuer);
-    }
+    getAssetIdentities(validAsset).forEach((id) => verifiedIds.add(id));
   }
 
   verifiedIds.add(nativeContractDetails.contract);
@@ -53,12 +49,7 @@ export const splitVerifiedAssetCurrency = async ({
     [typeof assets, typeof assets]
   >(
     ([inVerifiedList, notInVerifiedList], item) => {
-      if (item.issuer && verifiedIds.has(item.issuer)) {
-        inVerifiedList.push(item);
-        return [inVerifiedList, notInVerifiedList];
-      }
-
-      if (item.contract && verifiedIds.has(item.contract)) {
+      if (getAssetIdentities(item).some((id) => verifiedIds.has(id))) {
         inVerifiedList.push(item);
         return [inVerifiedList, notInVerifiedList];
       }
@@ -83,23 +74,44 @@ interface GetAssetListsForAssetParams {
 }
 
 /**
- * True when `asset` and `item` share an issuer, or share a contract id.
+ * The identities an asset (or an asset list entry) is known by: its contract
+ * id, and for a classic asset its code:issuer pair.
+ *
+ * A classic asset is the pair (code, issuer), so the issuer alone is never an
+ * identity: an issuer can mint any number of codes, and a list that vouches
+ * for one of them does not vouch for the others. Codes are case-sensitive, so
+ * USDE and USDe from the same issuer are different assets.
+ */
+export const getAssetIdentities = (asset: {
+  code?: string;
+  issuer?: string;
+  contract?: string;
+}): string[] => {
+  const identities = [] as string[];
+  if (asset.contract) {
+    identities.push(asset.contract);
+  }
+  if (asset.code && asset.issuer) {
+    identities.push(getCanonicalFromAsset(asset.code, asset.issuer));
+  }
+  return identities;
+};
+
+/**
+ * True when `asset` and `item` share a contract id, or share both code and
+ * issuer.
  *
  * Both halves of an identity are optional on these records, so each comparison
  * requires its own side to actually be present — two absent values are not a
  * match.
- *
- * Note this is not full identity: a classic asset is the pair (code, issuer),
- * and matching on the issuer alone accepts a different asset from the same
- * issuer. That is the long-standing behaviour of the verified-list membership
- * tests, tracked in stellar/wallet-eng-monorepo#76; this helper preserves it.
  */
 export const assetMatchesListItem = (
-  asset: { issuer?: string; contract?: string },
-  item: { issuer?: string; contract?: string },
-): boolean =>
-  (!!asset.issuer && asset.issuer === item.issuer) ||
-  (!!asset.contract && asset.contract === item.contract);
+  asset: { code?: string; issuer?: string; contract?: string },
+  item: { code?: string; issuer?: string; contract?: string },
+): boolean => {
+  const itemIdentities = getAssetIdentities(item);
+  return getAssetIdentities(asset).some((id) => itemIdentities.includes(id));
+};
 
 export const getAssetListsForAsset = async ({
   asset,