fix: getUserNameProofsByFid should include fname proofs (#1256)

* fix: getUserNameProofsByFid should return fname proofs as well

* changeset

* Fix tests

* determine latest schema version automatically

.changeset/eleven-mayflies-sell.md

@@ -0,0 +1,5 @@
+---
+"@farcaster/hubble": patch
+---
+
+fix: getUserNameProofsByFid should return fname proofs as well

apps/hubble/src/rpc/test/userDataService.test.ts

@@ -161,7 +161,7 @@ describe("getUserData", () => {
     expect(await engine.mergeMessage(ensNameProof)).toBeInstanceOf(Ok);
     const usernameProofs = await client.getUserNameProofsByFid(FidRequest.create({ fid }));
     expect(UsernameProofsResponse.toJSON(usernameProofs._unsafeUnwrap())).toEqual(
-      UsernameProofsResponse.toJSON({ proofs: [ensNameProof.data.usernameProofBody] }),
+      UsernameProofsResponse.toJSON({ proofs: [fnameProof, ensNameProof.data.usernameProofBody] }),
     );
 
     expect(await engine.mergeMessage(addEnsName)).toBeInstanceOf(Ok);

apps/hubble/src/storage/db/migrations/2.fnameproof.test.ts

@@ -0,0 +1,34 @@
+import RocksDB from "../rocksdb.js";
+import { performDbMigrations } from "./migrations.js";
+import { Factories, UserNameProof } from "@farcaster/hub-nodejs";
+import { getFNameProofByFid, makeFNameUserNameProofKey } from "../nameRegistryEvent.js";
+
+const dbName = "fnameproof.migration.test.db";
+
+describe("fnameproof migration", () => {
+  let db: RocksDB;
+
+  beforeAll(async () => {
+    db = new RocksDB(dbName);
+    await db.open();
+  });
+
+  afterAll(async () => {
+    await db.close();
+    await db.destroy();
+  });
+
+  test("should migrate fnameproof index", async () => {
+    // Write an fname proof without the index
+    const proof = Factories.UserNameProof.build();
+    const proofBuffer = Buffer.from(UserNameProof.encode(proof).finish());
+    await db.put(makeFNameUserNameProofKey(proof.name), proofBuffer);
+
+    await expect(getFNameProofByFid(db, proof.fid)).rejects.toThrow("NotFound");
+
+    const success = await performDbMigrations(db, 1, 2);
+    expect(success).toBe(true);
+
+    await expect(getFNameProofByFid(db, proof.fid)).resolves.toEqual(proof);
+  });
+});

apps/hubble/src/storage/db/migrations/2.fnameproof.ts

@@ -0,0 +1,39 @@
+/**
+ The FName User Name Proofs did not have an index by fid. In order to support getUserNameProofsByFid being able to return
+ fname proofs, we need to add an index by fid. This migration will go over all existing fname proofs and add an index
+ */
+
+import { logger } from "../../../utils/logger.js";
+import RocksDB from "../rocksdb.js";
+import { RootPrefix } from "../types.js";
+import { UserNameProof } from "@farcaster/hub-nodejs";
+import { makeFNameUserNameProofByFidKey } from "../nameRegistryEvent.js";
+
+const log = logger.child({ component: "FNameProofIndexMigration" });
+
+export async function fnameProofIndexMigration(db: RocksDB): Promise<boolean> {
+  log.info({}, "Starting fname proof index migration");
+  let count = 0;
+  const start = Date.now();
+
+  await db.forEachIteratorByPrefix(
+    Buffer.from([RootPrefix.FNameUserNameProof]),
+    async (key, value) => {
+      if (!key || !value) {
+        return;
+      }
+
+      const proof = UserNameProof.decode(new Uint8Array(value));
+      if (proof.fid) {
+        const secondaryKey = makeFNameUserNameProofByFidKey(proof.fid);
+        await db.put(secondaryKey, key);
+        count += 1;
+      }
+    },
+    {},
+    1 * 60 * 60 * 1000,
+  );
+
+  log.info({ count, duration: Date.now() - start }, "Finished fname proof index migration");
+  return true;
+}

apps/hubble/src/storage/db/migrations/migrations.ts

@@ -1,10 +1,7 @@
 import { ResultAsync } from "neverthrow";
 import RocksDB from "../rocksdb.js";
 import { usernameProofIndexMigration } from "./1.usernameproof.js";
-
-// The latest code version of the DB schema. This is the version that the DB will be
-// migrated to if the DB has an older schema version.
-export const LATEST_DB_SCHEMA_VERSION = 1;
+import { fnameProofIndexMigration } from "./2.fnameproof.js";
 
 type MigrationFunctionType = (db: RocksDB) => Promise<boolean>;
 const migrations = new Map<number, MigrationFunctionType>();
@@ -15,13 +12,20 @@ migrations.set(1, async (db: RocksDB) => {
   // to `UsernameProofMessageAdd`
   return await usernameProofIndexMigration(db);
 });
+migrations.set(2, async (db: RocksDB) => {
+  return await fnameProofIndexMigration(db);
+});
 
 // To Add a new migration
 // migrations.set(<next number>, async (db: RocksDB) => {
 //   <call migration script>
 //   return true; // or false if migration failed
 // });
 
+// The latest code version of the DB schema. This is the version that the DB will be
+// migrated to if the DB has an older schema version.
+export const LATEST_DB_SCHEMA_VERSION = migrations.size;
+
 export async function performDbMigrations(
   db: RocksDB,
   currentDbSchemaVersion: number,

apps/hubble/src/storage/db/nameRegistryEvent.ts

@@ -1,6 +1,7 @@
 import { NameRegistryEvent, UserNameProof } from "@farcaster/hub-nodejs";
 import RocksDB, { Iterator, Transaction } from "../db/rocksdb.js";
 import { RootPrefix } from "../db/types.js";
+import { makeFidKey } from "./message.js";
 
 const EXPIRY_BYTES = 4;
 
@@ -12,6 +13,10 @@ export const makeFNameUserNameProofKey = (name: Uint8Array): Buffer => {
   return Buffer.concat([Buffer.from([RootPrefix.FNameUserNameProof]), Buffer.from(name)]);
 };
 
+export const makeFNameUserNameProofByFidKey = (fid: number): Buffer => {
+  return Buffer.concat([Buffer.from([RootPrefix.FNameUserNameProofByFid]), makeFidKey(fid)]);
+};
+
 export const makeNameRegistryEventByExpiryKey = (expiry: number, fname?: Uint8Array): Buffer => {
   const buffer = Buffer.alloc(1 + EXPIRY_BYTES + (fname ? fname.length : 0));
   buffer.writeUint8(RootPrefix.NameRegistryEventsByExpiry, 0);
@@ -34,6 +39,13 @@ export const getUserNameProof = async (db: RocksDB, name: Uint8Array): Promise<U
   return UserNameProof.decode(new Uint8Array(buffer));
 };
 
+export const getFNameProofByFid = async (db: RocksDB, fid: number): Promise<UserNameProof> => {
+  const secondaryKey = makeFNameUserNameProofByFidKey(fid);
+  const primaryKey = await db.get(secondaryKey);
+  const buffer = await db.get(primaryKey);
+  return UserNameProof.decode(new Uint8Array(buffer));
+};
+
 export const getNameRegistryEventsByExpiryIterator = (db: RocksDB): Iterator => {
   const prefix = Buffer.from([RootPrefix.NameRegistryEventsByExpiry]);
   return db.iteratorByPrefix(prefix, { keys: false });
@@ -72,14 +84,18 @@ export const putUserNameProofTransaction = (txn: Transaction, usernameProof: Use
   const proofBuffer = Buffer.from(UserNameProof.encode(usernameProof).finish());
 
   const primaryKey = makeFNameUserNameProofKey(usernameProof.name);
+  const secondaryKey = makeFNameUserNameProofByFidKey(usernameProof.fid);
   const putTxn = txn.put(primaryKey, proofBuffer);
+  putTxn.put(secondaryKey, primaryKey);
 
   return putTxn;
 };
 
 export const deleteUserNameProofTransaction = (txn: Transaction, usernameProof: UserNameProof): Transaction => {
   const primaryKey = makeFNameUserNameProofKey(usernameProof.name);
+  const secondaryKey = makeFNameUserNameProofByFidKey(usernameProof.fid);
   const deleteTxn = txn.del(primaryKey);
+  deleteTxn.del(secondaryKey);
 
   return deleteTxn;
 };

apps/hubble/src/storage/db/types.ts

@@ -70,6 +70,9 @@ export enum RootPrefix {
 
   /** DB Schema version */
   DBSchemaVersion = 24,
+
+  /* Used to index fname username proofs by fid */
+  FNameUserNameProofByFid = 25,
 }
 
 /**

apps/hubble/src/storage/engine/index.test.ts

@@ -517,11 +517,13 @@ describe("mergeMessage", () => {
       expect(result).toMatchObject(err({ errCode: "bad_request.validation_failure" }));
       expect(result._unsafeUnwrapErr().message).toMatch("invalid ens name");
     });
+
     test("fails gracefully when resolving throws an error", async () => {
       const result = await engine.mergeMessage(await createProof("test.eth"));
       expect(result).toMatchObject(err({ errCode: "unavailable.network_failure" }));
       expect(result._unsafeUnwrapErr().message).toMatch("failed to resolve ens name");
     });
+
     test("fails gracefully when resolving an unregistered name", async () => {
       jest.spyOn(publicClient, "getEnsAddress").mockImplementation(() => {
         return Promise.resolve(null);
@@ -530,6 +532,7 @@ describe("mergeMessage", () => {
       expect(result).toMatchObject(err({ errCode: "bad_request.validation_failure" }));
       expect(result._unsafeUnwrapErr().message).toMatch("no valid address for akjsdhkhaasd.eth");
     });
+
     test("fails when resolved address does not match proof", async () => {
       const message = await createProof("test.eth");
       jest.spyOn(publicClient, "getEnsAddress").mockImplementation(() => {
@@ -539,6 +542,7 @@ describe("mergeMessage", () => {
       expect(result).toMatchObject(err({ errCode: "bad_request.validation_failure" }));
       expect(result._unsafeUnwrapErr().message).toMatch(`resolved address ${randomEthAddress} does not match proof`);
     });
+
     test("fails when resolved address does not match custody address or verification address", async () => {
       await engine.mergeMessage(verificationAdd);
       jest.spyOn(publicClient, "getEnsAddress").mockImplementation(() => {
@@ -549,6 +553,7 @@ describe("mergeMessage", () => {
       expect(result).toMatchObject(err({ errCode: "bad_request.validation_failure" }));
       expect(result._unsafeUnwrapErr().message).toMatch("ens name does not belong to fid");
     });
+
     test("succeeds for valid proof for custody address", async () => {
       const custodyAddress = bytesToHexString(custodyEvent.to)._unsafeUnwrap();
       jest.spyOn(publicClient, "getEnsAddress").mockImplementation(() => {
@@ -562,6 +567,7 @@ describe("mergeMessage", () => {
       expect(usernameProofEvents[0]?.usernameProof).toMatchObject(message.data.usernameProofBody);
       expect(usernameProofEvents[0]?.deletedUsernameProof).toBeUndefined();
     });
+
     test("succeeds for valid proof for verified eth address", async () => {
       await engine.mergeMessage(verificationAdd);
       const verificationAddress = bytesToHexString(
@@ -579,6 +585,29 @@ describe("mergeMessage", () => {
       expect(usernameProofEvents[0]?.deletedUsernameProof).toBeUndefined();
     });
 
+    test("getUsernameProofsByFid returns all proofs for fid including fname proofs", async () => {
+      const custodyAddress = bytesToHexString(custodyEvent.to)._unsafeUnwrap();
+      jest.spyOn(publicClient, "getEnsAddress").mockImplementation(() => {
+        return Promise.resolve(custodyAddress);
+      });
+      const message = await createProof("test.eth", null, custodyAddress);
+      const result = await engine.mergeMessage(message);
+      expect(result.isOk()).toBeTruthy();
+
+      const fnameProof = Factories.UserNameProof.build({ fid });
+      await engine.mergeUserNameProof(fnameProof);
+
+      const proofs = (await engine.getUserNameProofsByFid(fid)).unwrapOr([]);
+      expect(proofs.length).toBe(2);
+      expect(proofs).toContainEqual(message.data.usernameProofBody);
+      expect(proofs).toContainEqual(fnameProof);
+    });
+
+    test("getUsernameProofsByFid does not fail for empty results", async () => {
+      const proofs = (await engine.getUserNameProofsByFid(fid)).unwrapOr([]);
+      expect(proofs.length).toBe(0);
+    });
+
     describe("userDataAdd message with an ens name", () => {
       let test1Message: Message;
       let userDataAdd: Message;

apps/hubble/src/storage/engine/index.ts

@@ -839,8 +839,22 @@ class Engine {
     if (validatedFid.isErr()) {
       return err(validatedFid.error);
     }
-
-    return ResultAsync.fromPromise(this._usernameProofStore.getUsernameProofsByFid(fid), (e) => e as HubError);
+    const proofs: UserNameProof[] = [];
+    const fnameProof = await ResultAsync.fromPromise(
+      this._userDataStore.getUserNameProofByFid(fid),
+      (e) => e as HubError,
+    );
+    if (fnameProof.isOk()) {
+      proofs.push(fnameProof.value);
+    }
+    const ensProofs = await ResultAsync.fromPromise(
+      this._usernameProofStore.getUsernameProofsByFid(fid),
+      (e) => e as HubError,
+    );
+    if (ensProofs.isOk()) {
+      proofs.push(...ensProofs.value);
+    }
+    return ok(proofs);
   }
 
   /* -------------------------------------------------------------------------- */

apps/hubble/src/storage/stores/userDataStore.test.ts

@@ -63,6 +63,7 @@ describe("mergeUserNameProof", () => {
     const proof = await Factories.UserNameProof.build();
     await set.mergeUserNameProof(proof);
     await expect(set.getUserNameProof(proof.name)).resolves.toEqual(proof);
+    await expect(set.getUserNameProofByFid(proof.fid)).resolves.toEqual(proof);
   });
 
   test("replaces existing proof with proof of greater timestamp", async () => {
@@ -75,6 +76,8 @@ describe("mergeUserNameProof", () => {
     });
     await set.mergeUserNameProof(newProof);
     await expect(set.getUserNameProof(existingProof.name)).resolves.toEqual(newProof);
+    // Secondary index is updated
+    await expect(set.getUserNameProofByFid(existingProof.fid)).resolves.toEqual(newProof);
   });
 
   test("does not merge if existing timestamp is greater", async () => {
@@ -101,6 +104,7 @@ describe("mergeUserNameProof", () => {
     });
     await set.mergeUserNameProof(newProof);
     await expect(set.getUserNameProof(existingProof.name)).rejects.toThrowError("NotFound");
+    await expect(set.getUserNameProofByFid(existingProof.fid)).rejects.toThrowError("NotFound");
   });
 
   test("does not delete existing proof if fid is 0 and timestamp is less than existing", async () => {

apps/hubble/src/storage/stores/userDataStore.ts

@@ -17,6 +17,7 @@ import {
   getUserNameProof,
   putUserNameProofTransaction,
   deleteUserNameProofTransaction,
+  getFNameProofByFid,
 } from "../db/nameRegistryEvent.js";
 import { UserMessagePostfix, UserPostfix } from "../db/types.js";
 import { MessagesPage, PageOptions } from "../stores/types.js";
@@ -113,6 +114,10 @@ class UserDataStore extends Store<UserDataAddMessage, never> {
     return getUserNameProof(this._db, name);
   }
 
+  async getUserNameProofByFid(fid: number): Promise<UserNameProof> {
+    return getFNameProofByFid(this._db, fid);
+  }
+
   /**
    * Merges a NameRegistryEvent storing the causally latest event at the key:
    * <name registry root prefix byte, fname>