fix: do not let deleted fname events allow old fname events to be re-…

…added (#2203)

## Why is this change needed?

The fname events are not coming through correctly for a few names, and
by consequence we're seeing a few old fname events being endlessly
replayed. The deletion of the fname record (when fid = 0 in proof)
completely removes the record rather than its association with a fid,
and by consequence replaying the older event re-adds the fname. This
resolves the hub side of the behavior by correctly holding on to the new
fname event, but deleting the association with a fid.

## Merge Checklist

_Choose all relevant options below by adding an `x` now or at any time
before submitting for review_

- [x] PR title adheres to the [conventional
commits](https://www.conventionalcommits.org/en/v1.0.0/) standard
- [x] PR has a
[changeset](https://github.com/farcasterxyz/hub-monorepo/blob/main/CONTRIBUTING.md#35-adding-changesets)
- [x] PR has been tagged with a change label(s) (i.e. documentation,
feature, bugfix, or chore)
- [x] PR includes
[documentation](https://github.com/farcasterxyz/hub-monorepo/blob/main/CONTRIBUTING.md#32-writing-docs)
if necessary.

<!-- start pr-codex -->

---

## PR-Codex overview
The focus of this PR is to enhance the `hubble` app by fixing issues
related to username proof handling and syncing historical events.

### Detailed summary
- Fixed issue where deleted fname events allowed re-adding old fname
events
- Improved syncing historical events in `syncEngine.ts`
- Updated `mergeUserNameProof` functionality in tests
- Added `rsGetUserNameProof` import in `nameRegistryEvent.ts`
- Enhanced `UserDataStore` functionality in `user_data_store.rs`

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your
question}`

<!-- end pr-codex -->

.changeset/serious-falcons-drum.md

@@ -0,0 +1,5 @@
+---
+"@farcaster/hubble": patch
+---
+
+fix: do not let deleted fname events allow old fname events to be re-added

apps/hubble/src/addon/src/store/name_registry_events.rs

@@ -75,10 +75,15 @@ pub fn put_username_proof_transaction(
 pub fn delete_username_proof_transaction(
     txn: &mut RocksDbTransactionBatch,
     username_proof: &UserNameProof,
+    existing_fid: Option<u32>,
 ) {
+    let buf = username_proof.encode_to_vec();
+
     let primary_key = make_fname_username_proof_key(&username_proof.name);
-    txn.delete(primary_key);
+    txn.put(primary_key.clone(), buf);
 
-    let secondary_key = make_fname_username_proof_by_fid_key(username_proof.fid as u32);
-    txn.delete(secondary_key);
+    if existing_fid.is_some() {
+        let secondary_key = make_fname_username_proof_by_fid_key(existing_fid.unwrap());
+        txn.delete(secondary_key);
+    }
 }

apps/hubble/src/addon/src/store/user_data_store.rs

@@ -263,7 +263,13 @@ impl UserDataStore {
         let name = name_buffer.as_slice(&mut cx);
 
         let result = match Self::get_username_proof(&store, &name) {
-            Ok(Some(proof)) => proof.encode_to_vec(),
+            Ok(Some(proof)) => match proof.fid {
+                0 => cx.throw_error(format!(
+                    "{}/{}",
+                    "not_found", "NotFound: UserDataAdd message not found"
+                ))?,
+                _ => proof.encode_to_vec(),
+            },
             Ok(None) => cx.throw_error(format!(
                 "{}/{}",
                 "not_found", "NotFound: UserDataAdd message not found"
@@ -318,6 +324,7 @@ impl UserDataStore {
         username_proof: &protos::UserNameProof,
     ) -> Result<Vec<u8>, HubError> {
         let existing_proof = get_username_proof(&store.db(), &username_proof.name)?;
+        let mut existing_fid: Option<u32> = None;
 
         if existing_proof.is_some() {
             let cmp =
@@ -335,6 +342,7 @@ impl UserDataStore {
                     message: "event conflicts with a more recent UserNameProof".to_string(),
                 });
             }
+            existing_fid = Some(existing_proof.as_ref().unwrap().fid as u32);
         }
 
         if existing_proof.is_none() && username_proof.fid == 0 {
@@ -346,7 +354,7 @@ impl UserDataStore {
 
         let mut txn = RocksDbTransactionBatch::new();
         if username_proof.fid == 0 {
-            delete_username_proof_transaction(&mut txn, username_proof);
+            delete_username_proof_transaction(&mut txn, username_proof, existing_fid);
         } else {
             put_username_proof_transaction(&mut txn, username_proof);
         }

apps/hubble/src/eth/fnameRegistryEventsProvider.test.ts

@@ -1,17 +1,13 @@
-import {
-  FNameRegistryClientInterface,
-  FNameRegistryEventsProvider,
-  FNameTransfer,
-  FNameTransferRequest,
-} from "./fnameRegistryEventsProvider.js";
+import { FNameRegistryEventsProvider, FNameTransfer } from "./fnameRegistryEventsProvider.js";
 import { jestRocksDB } from "../storage/db/jestUtils.js";
 import Engine from "../storage/engine/index.js";
 import { FarcasterNetwork } from "@farcaster/core";
 import { MockFnameRegistryClient, MockHub } from "../test/mocks.js";
-import { getUserNameProof } from "../storage/db/nameRegistryEvent.js";
 import { utf8ToBytes } from "@noble/curves/abstract/utils";
 import { generatePrivateKey, privateKeyToAccount, Address } from "viem/accounts";
 import { HubState } from "@farcaster/hub-nodejs";
+import StoreEventHandler from "storage/stores/storeEventHandler.js";
+import UserDataStore from "storage/stores/userDataStore.js";
 
 describe("fnameRegistryEventsProvider", () => {
   const db = jestRocksDB("protobufs.fnameRegistry.test");
@@ -77,17 +73,19 @@ describe("fnameRegistryEventsProvider", () => {
   describe("syncHistoricalEvents", () => {
     it("fetches all events from the beginning", async () => {
       await provider.start();
-      expect(await getUserNameProof(db, utf8ToBytes("test1"))).toBeTruthy();
-      expect(await getUserNameProof(db, utf8ToBytes("test2"))).toBeTruthy();
-      await expect(getUserNameProof(db, utf8ToBytes("test4"))).rejects.toThrowError("NotFound");
+      expect(await engine.getUserNameProof(utf8ToBytes("test1"))).toBeTruthy();
+      expect(await engine.getUserNameProof(utf8ToBytes("test2"))).toBeTruthy();
+      const failCase = await engine.getUserNameProof(utf8ToBytes("test4"));
+      expect(failCase.isErr()).toBeTruthy();
+      expect(failCase._unsafeUnwrapErr().errCode).toEqual("not_found");
       expect((await hub.getHubState())._unsafeUnwrap().lastFnameProof).toEqual(transferEvents[3]?.id);
     });
 
     it("fetches events from where it left off", async () => {
       await hub.putHubState(HubState.create({ lastFnameProof: transferEvents[0]?.id ?? 0 }));
       mockFnameRegistryClient.setMinimumSince(transferEvents[0]?.id ?? 0);
       await provider.start();
-      expect(await getUserNameProof(db, utf8ToBytes("test2"))).toBeTruthy();
+      expect(await engine.getUserNameProof(utf8ToBytes("test2"))).toBeTruthy();
       expect((await hub.getHubState())._unsafeUnwrap().lastFnameProof).toEqual(transferEvents[3]?.id);
     });
 
@@ -100,14 +98,16 @@ describe("fnameRegistryEventsProvider", () => {
   describe("mergeTransfers", () => {
     it("deletes a proof from the db when a username is unregistered", async () => {
       await provider.start();
-      await expect(getUserNameProof(db, utf8ToBytes("test3"))).rejects.toThrowError("NotFound");
+      const failCase = await engine.getUserNameProof(utf8ToBytes("test3"));
+      expect(failCase.isErr()).toBeTruthy();
+      expect(failCase._unsafeUnwrapErr().errCode).toEqual("not_found");
     });
 
     it("does not fail if there are errors merging events", async () => {
       // Return duplicate events
       mockFnameRegistryClient.setTransfersToReturn([transferEvents, transferEvents]);
       await provider.start();
-      expect(await getUserNameProof(db, utf8ToBytes("test2"))).toBeTruthy();
+      expect(await engine.getUserNameProof(utf8ToBytes("test2"))).toBeTruthy();
     });
 
     it("does not merge when the signature is invalid", async () => {
@@ -123,7 +123,9 @@ describe("fnameRegistryEventsProvider", () => {
       invalidEvent.server_signature = "0x8773442740c17c9d0f0b87022c722f9a136206ed";
       mockFnameRegistryClient.setTransfersToReturn([[invalidEvent]]);
       await provider.start();
-      await expect(getUserNameProof(db, utf8ToBytes("test1"))).rejects.toThrowError("NotFound");
+      const failCase = await engine.getUserNameProof(utf8ToBytes("test1"));
+      expect(failCase.isErr()).toBeTruthy();
+      expect(failCase._unsafeUnwrapErr().errCode).toEqual("not_found");
     });
 
     it("succeeds for a known proof", async () => {
@@ -143,7 +145,7 @@ describe("fnameRegistryEventsProvider", () => {
       mockFnameRegistryClient.setTransfersToReturn([[proof]]);
       mockFnameRegistryClient.setSignerAddress("0xBc5274eFc266311015793d89E9B591fa46294741");
       await provider.start();
-      expect(await getUserNameProof(db, utf8ToBytes("farcaster"))).toBeTruthy();
+      expect(await engine.getUserNameProof(utf8ToBytes("farcaster"))).toBeTruthy();
     });
   });
 

apps/hubble/src/network/sync/syncEngine.ts

@@ -50,7 +50,6 @@ import { finishAllProgressBars } from "../../utils/progressBars.js";
 import { FNameRegistryEventsProvider } from "../../eth/fnameRegistryEventsProvider.js";
 import { PeerScore, PeerScorer } from "./peerScore.js";
 import { getOnChainEvent } from "../../storage/db/onChainEvent.js";
-import { getUserNameProof } from "../../storage/db/nameRegistryEvent.js";
 import { MaxPriorityQueue } from "@datastructures-js/priority-queue";
 import { TTLMap } from "../../utils/ttl_map.js";
 import * as buffer from "node:buffer";
@@ -1851,7 +1850,7 @@ class SyncEngine extends TypedEmitter<SyncEvents> {
           revokedSyncIds += 1;
         }
       } else if (unpacked.type === SyncIdType.FName) {
-        const result = await ResultAsync.fromPromise(getUserNameProof(this._db, unpacked.name), (e) => e as HubError);
+        const result = await this._hub.engine.getUserNameProof(unpacked.name);
         let validSyncId = result.isOk();
         if (result.isOk()) {
           validSyncId = result.value.fid === unpacked.fid;

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

@@ -2,6 +2,7 @@ import { UserNameProof } from "@farcaster/hub-nodejs";
 import RocksDB, { RocksDbTransaction } from "../db/rocksdb.js";
 import { RootPrefix } from "../db/types.js";
 import { makeFidKey } from "./message.js";
+import { rsGetUserNameProof } from "rustfunctions.js";
 
 export const makeFNameUserNameProofKey = (name: Uint8Array): Buffer => {
   return Buffer.concat([Buffer.from([RootPrefix.FNameUserNameProof]), Buffer.from(name)]);
@@ -11,12 +12,6 @@ export const makeFNameUserNameProofByFidKey = (fid: number): Buffer => {
   return Buffer.concat([Buffer.from([RootPrefix.FNameUserNameProofByFid]), makeFidKey(fid)]);
 };
 
-export const getUserNameProof = async (db: RocksDB, name: Uint8Array): Promise<UserNameProof> => {
-  const primaryKey = makeFNameUserNameProofKey(name);
-  const buffer = await db.get(primaryKey);
-  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);

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

@@ -41,7 +41,6 @@ import { getMessage, makeTsHash, typeToSetPostfix } from "../db/message.js";
 import { StoreEvents } from "../stores/storeEventHandler.js";
 import { IdRegisterOnChainEvent, makeVerificationAddressClaim } from "@farcaster/core";
 import { setReferenceDateForTest } from "../../utils/versions.js";
-import { getUserNameProof } from "../db/nameRegistryEvent.js";
 import { publicClient } from "../../test/utils.js";
 import { jest } from "@jest/globals";
 import { FNameRegistryEventsProvider, FNameTransfer } from "../../eth/fnameRegistryEventsProvider.js";
@@ -164,7 +163,7 @@ describe("mergeUserNameProof", () => {
   test("succeeds", async () => {
     const userNameProof = Factories.UserNameProof.build();
     await expect(engine.mergeUserNameProof(userNameProof)).resolves.toBeInstanceOf(Ok);
-    expect(await getUserNameProof(db, userNameProof.name)).toBeTruthy();
+    expect(await engine.getUserNameProof(userNameProof.name)).toBeTruthy();
   });
 });
 

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

@@ -144,6 +144,24 @@ describe("mergeUserNameProof", () => {
     expect(proofEvents).toEqual([[newProof, existingProof]]);
   });
 
+  test("does not merge if deleted existing proof and timestamp is lesser", async () => {
+    const existingProof = await Factories.UserNameProof.build();
+    await set.mergeUserNameProof(existingProof);
+    const newProof = await Factories.UserNameProof.build({
+      timestamp: existingProof.timestamp + 10,
+      name: existingProof.name,
+      fid: 0,
+    });
+    proofEvents = [];
+    await set.mergeUserNameProof(newProof);
+    await expect(set.getUserNameProof(existingProof.name)).rejects.toThrowError("NotFound");
+    await expect(set.getUserNameProofByFid(existingProof.fid)).rejects.toThrowError("NotFound");
+    expect(proofEvents).toEqual([[newProof, existingProof]]);
+    await expect(set.mergeUserNameProof(existingProof)).rejects.toThrowError(
+      "event conflicts with a more recent UserNameProof",
+    );
+  });
+
   test("does not emit an event if there is no existing proof and new proof is to fid 0", async () => {
     const proof = await Factories.UserNameProof.build({
       fid: 0,