fix: don't penalize peer score for duplicate messages (#1644)

* fix: don't penalize peer score for duplicate messages

* Fail if no peerId in test

.changeset/calm-mails-notice.md

@@ -0,0 +1,5 @@
+---
+"@farcaster/hubble": patch
+---
+
+fix: Remove score penalty for duplicate gossip messages

apps/hubble/src/hubble.ts

@@ -1026,13 +1026,6 @@ export class Hub implements HubInterface {
       const result = await this.submitMessage(message, "gossip");
       if (result.isOk()) {
         this.gossipNode.reportValid(msgId, peerIdFromString(source.toString()).toBytes(), true);
-        const currentTime = getFarcasterTime().unwrapOr(0);
-        const messageCreatedTime = message.data?.timestamp ?? 0;
-        // The message time is user provided, so, while not ideal, it's still good enough to use most of the time
-        if (currentTime > 0 && messageCreatedTime > 0 && currentTime > messageCreatedTime) {
-          const diff = currentTime - messageCreatedTime;
-          statsd().timing("gossip.message_delay", diff);
-        }
       } else {
         log.info(
           {
@@ -1046,6 +1039,17 @@ export class Hub implements HubInterface {
         );
         this.gossipNode.reportValid(msgId, peerIdFromString(source.toString()).toBytes(), false);
       }
+
+      const currentTime = getFarcasterTime().unwrapOr(0);
+      const messageCreatedTime = message.data?.timestamp ?? 0;
+      // The message time is user provided, so, while not ideal, it's still good enough to use most of the time
+      if (currentTime > 0 && messageCreatedTime > 0 && currentTime > messageCreatedTime) {
+        const diff = currentTime - messageCreatedTime;
+        statsd().timing("gossip.message_delay", diff);
+        const mergeResult = result.isOk() ? "success" : "failure";
+        statsd().timing(`gossip.message_delay.${mergeResult}`, diff);
+      }
+
       return result.map(() => undefined);
     } else if (gossipMessage.contactInfoContent) {
       const result = await this.handleContactInfo(peerIdResult.value, gossipMessage.contactInfoContent);
@@ -1336,14 +1340,15 @@ export class Hub implements HubInterface {
     const start = Date.now();
 
     const message = ensureMessageData(submittedMessage);
+    const type = messageTypeToName(message.data?.type);
     const mergeResult = await this.engine.mergeMessage(message);
 
     mergeResult.match(
       (eventId) => {
         const logData = {
           eventId,
           fid: message.data?.fid,
-          type: messageTypeToName(message.data?.type),
+          type: type,
           submittedMessage: messageToLog(submittedMessage),
           source,
         };
@@ -1366,7 +1371,9 @@ export class Hub implements HubInterface {
       void this.gossipNode.gossipMessage(message);
     }
 
-    statsd().timing("hub.merge_message", Date.now() - start);
+    const now = Date.now();
+    statsd().timing("hub.merge_message", now - start);
+    statsd().timing(`hub.merge_message.${type}`, now - start);
 
     return mergeResult;
   }

apps/hubble/src/network/p2p/gossipNodeWorker.ts

@@ -417,7 +417,7 @@ export class LibP2PNode {
     this.gossip?.reportMessageValidationResult(
       messageId,
       propagationSource,
-      isValid ? TopicValidatorResult.Accept : TopicValidatorResult.Reject,
+      isValid ? TopicValidatorResult.Accept : TopicValidatorResult.Ignore,
     );
   }
 

apps/hubble/src/test/e2e/gossipNetwork.test.ts

@@ -105,7 +105,11 @@ describe("gossip network tests", () => {
       let numCastAddMessages = 0;
 
       nonSenderNodes.map((n) => {
-        const topics = messageStore.get(n.peerId()?.toString() ?? "");
+        const peerId = n.peerId();
+        if (!peerId) {
+          throw new Error(`peerId is undefined for node: ${n}`);
+        }
+        const topics = messageStore.get(peerId.toString());
         expect(topics).toBeDefined();
         expect(topics?.has(primaryTopic)).toBeTruthy();
         const topicMessages = topics?.get(primaryTopic) ?? [];