From 435e8dd4027c7ffa9517ce712100731d492894ec Mon Sep 17 00:00:00 2001
From: achingbrain <alex@achingbrain.net>
Date: Fri, 2 Feb 2024 14:19:54 +0100
Subject: [PATCH] fix!: make PublishError.InsufficientPeers more
 self-explanatory

People continually mistake the `InsufficientPeers` error for something
that can be "worked around" by setting `allowPublishToZeroPeers` to true,
and they then expect other network nodes to still recieve their messages.

This PR renames the option to `allowPublishToZeroTopicPeers` - this makes
it clear(er) that it's not that you don't have any peers, it's that no
peers you have are listening on the topic.

It also improves the JSDoc comment on the option and changes the `Error`
message from `PublishError.InsufficientPeers` to `PublishError.NoPeersSubscribedToTopic`
which (I hope) more accurately describes what's wrong.

BREAKING CHANGE: The `allowPublishToZeroPeers` option has been renamed to `allowPublishToZeroTopicPeers`

Fixes #472
---
 src/index.ts            | 17 ++++++++++++-----
 src/types.ts            | 10 +++++++++-
 test/2-nodes.spec.ts    |  2 +-
 test/compliance.spec.ts |  2 +-
 test/gossip.spec.ts     |  4 ++--
 5 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/src/index.ts b/src/index.ts
index f3278b32..e190b44f 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -104,8 +104,15 @@ export interface GossipsubOpts extends GossipsubOptsSpec, PubSubInit {
    * reportMessageValidationResult() after the message is dropped from mcache won't forward the message.
    */
   asyncValidation: boolean
-  /** Do not throw `InsufficientPeers` error if publishing to zero peers */
-  allowPublishToZeroPeers: boolean
+  /**
+   * Do not throw `PublishError.NoPeersSubscribedToTopic` error if there are no
+   * peers listening on the topic.
+   *
+   * N.B. if you sent this option to true, and you publish a message on a topic
+   * with no peers listening on that topic, no other network node will ever
+   * receive the message.
+   */
+  allowPublishToZeroTopicPeers: boolean
   /** Do not throw `PublishError.Duplicate` if publishing duplicate messages */
   ignoreDuplicatePublishError: boolean
   /** For a single stream, await processing each RPC before processing the next */
@@ -2095,10 +2102,10 @@ export class GossipSub extends TypedEventEmitter<GossipsubEvents> implements Pub
     const willSendToSelf = this.opts.emitSelf && this.subscriptions.has(topic)
 
     // Current publish opt takes precedence global opts, while preserving false value
-    const allowPublishToZeroPeers = opts?.allowPublishToZeroPeers ?? this.opts.allowPublishToZeroPeers
+    const allowPublishToZeroTopicPeers = opts?.allowPublishToZeroTopicPeers ?? this.opts.allowPublishToZeroTopicPeers
 
-    if (tosend.size === 0 && !allowPublishToZeroPeers && !willSendToSelf) {
-      throw Error('PublishError.InsufficientPeers')
+    if (tosend.size === 0 && !allowPublishToZeroTopicPeers && !willSendToSelf) {
+      throw Error('PublishError.NoPeersSubscribedToTopic')
     }
 
     // If the message isn't a duplicate and we have sent it to some peers add it to the
diff --git a/src/types.ts b/src/types.ts
index c1c623c4..1dca09b2 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -71,7 +71,15 @@ export enum SignaturePolicy {
 }
 
 export interface PublishOpts {
-  allowPublishToZeroPeers?: boolean
+  /**
+   * Do not throw `PublishError.NoPeersSubscribedToTopic` error if there are no
+   * peers listening on the topic.
+   *
+   * N.B. if you sent this option to true, and you publish a message on a topic
+   * with no peers listening on that topic, no other network node will ever
+   * receive the message.
+   */
+  allowPublishToZeroTopicPeers?: boolean
   ignoreDuplicatePublishError?: boolean
   /** serialize message once and send to all peers without control messages */
   batchPublish?: boolean
diff --git a/test/2-nodes.spec.ts b/test/2-nodes.spec.ts
index 97a710a9..82a3d9df 100644
--- a/test/2-nodes.spec.ts
+++ b/test/2-nodes.spec.ts
@@ -243,7 +243,7 @@ describe('2 nodes', () => {
     // Create pubsub nodes
     beforeEach(async () => {
       mockNetwork.reset()
-      nodes = await createComponentsArray({ number: 2, init: { allowPublishToZeroPeers: true } })
+      nodes = await createComponentsArray({ number: 2, init: { allowPublishToZeroTopicPeers: true } })
       await connectAllPubSubNodes(nodes)
 
       // Create subscriptions
diff --git a/test/compliance.spec.ts b/test/compliance.spec.ts
index 5e3da4d4..d5e364ac 100644
--- a/test/compliance.spec.ts
+++ b/test/compliance.spec.ts
@@ -29,7 +29,7 @@ describe.skip('interface compliance', function () {
           ...args.init,
           // libp2p-interfaces-compliance-tests in test 'can subscribe and unsubscribe correctly' publishes to no peers
           // Disable check to allow passing tests
-          allowPublishToZeroPeers: true
+          allowPublishToZeroTopicPeers: true
         }
       )
 
diff --git a/test/gossip.spec.ts b/test/gossip.spec.ts
index 9a34a4ce..73011bb9 100644
--- a/test/gossip.spec.ts
+++ b/test/gossip.spec.ts
@@ -27,7 +27,7 @@ describe('gossip', () => {
           IPColocationFactorThreshold: GossipsubDhi + 3
         },
         maxInboundDataLength: 4000000,
-        allowPublishToZeroPeers: false
+        allowPublishToZeroTopicPeers: false
       }
     })
   })
@@ -89,7 +89,7 @@ describe('gossip', () => {
     const topic = 'Z'
 
     const publishResult = await nodeA.pubsub.publish(topic, uint8ArrayFromString('hey'), {
-      allowPublishToZeroPeers: true
+      allowPublishToZeroTopicPeers: true
     })
 
     // gossip happens during the heartbeat