Fix minMeshMessageDeliveriesWindow (#218)

* Fix minMeshMessageDeliveriesWindow

* Fix gossipsubIWantFollowupTime for metric

* Fix tracer prune()

* Change to maxMeshMessageDeliveriesWindow

Author: twoeths

ts/constants.ts

@@ -238,3 +238,8 @@ export const ACCEPT_FROM_WHITELIST_MAX_MESSAGES = 128
  * this peer up to this time duration.
  */
 export const ACCEPT_FROM_WHITELIST_DURATION_MS = 1000
+
+/**
+ * The default MeshMessageDeliveriesWindow to be used in metrics.
+ */
+export const DEFAULT_METRIC_MESH_MESSAGE_DELIVERIES_WINDOWS = 1000

ts/index.ts

@@ -397,14 +397,18 @@ export default class Gossipsub extends EventEmitter {
         throw Error('Must set metricsTopicStrToLabel with metrics')
       }
 
+      // in theory, each topic has its own meshMessageDeliveriesWindow param
+      // however in lodestar, we configure it mostly the same so just pick the max of positive ones
+      // (some topics have meshMessageDeliveriesWindow as 0)
+      const maxMeshMessageDeliveriesWindow = Math.max(
+        ...Object.values(opts.scoreParams.topics).map((topicParam) => topicParam.meshMessageDeliveriesWindow),
+        constants.DEFAULT_METRIC_MESH_MESSAGE_DELIVERIES_WINDOWS
+      )
+
       const metrics = getMetrics(options.metricsRegister, options.metricsTopicStrToLabel, {
-        gossipPromiseExpireSec: constants.GossipsubIWantFollowupTime / 1000,
+        gossipPromiseExpireSec: this.opts.gossipsubIWantFollowupTime / 1000,
         behaviourPenaltyThreshold: opts.scoreParams.behaviourPenaltyThreshold,
-        // in theory, each topic has its own meshMessageDeliveriesWindow param
-        // however in lodestar, we configure it the same so just pick the min one
-        minMeshMessageDeliveriesWindow: Math.min(
-          ...Object.values(opts.scoreParams.topics).map((topicParam) => topicParam.meshMessageDeliveriesWindow)
-        )
+        maxMeshMessageDeliveriesWindow
       })
 
       metrics.mcacheSize.addCollect(() => this.onScrapeMetrics(metrics))

ts/metrics.ts

@@ -161,7 +161,7 @@ export type Metrics = ReturnType<typeof getMetrics>
 export function getMetrics(
   register: MetricsRegister,
   topicStrToLabel: TopicStrToLabel,
-  opts: { gossipPromiseExpireSec: number; behaviourPenaltyThreshold: number; minMeshMessageDeliveriesWindow: number }
+  opts: { gossipPromiseExpireSec: number; behaviourPenaltyThreshold: number; maxMeshMessageDeliveriesWindow: number }
 ) {
   // Using function style instead of class to prevent having to re-declare all MetricsPrometheus types.
 
@@ -346,11 +346,11 @@ export function getMetrics(
       help: 'Time since the 1st duplicated message validated',
       labelNames: ['topic'],
       buckets: [
-        0.25 * opts.minMeshMessageDeliveriesWindow,
-        0.5 * opts.minMeshMessageDeliveriesWindow,
-        1 * opts.minMeshMessageDeliveriesWindow,
-        2 * opts.minMeshMessageDeliveriesWindow,
-        4 * opts.minMeshMessageDeliveriesWindow
+        0.25 * opts.maxMeshMessageDeliveriesWindow,
+        0.5 * opts.maxMeshMessageDeliveriesWindow,
+        1 * opts.maxMeshMessageDeliveriesWindow,
+        2 * opts.maxMeshMessageDeliveriesWindow,
+        4 * opts.maxMeshMessageDeliveriesWindow
       ]
     }),
     /** Total count of late msg delivery total by topic */

ts/tracer.ts

@@ -143,9 +143,11 @@ export class IWantTracer {
     const maxMs = Date.now() - this.requestMsByMsgExpire
 
     for (const [k, v] of this.requestMsByMsg.entries()) {
-      if (v < maxMs) {
+      if (v >= maxMs) {
+        // messages that stay too long in the requestMsByMsg map, delete
         this.requestMsByMsg.delete(k)
       } else {
+        // recent messages, keep them
         // sort by insertion order
         break
       }