fix(network): throw on unsigned messages in dkgGossipMsgId (Codex PR #501)

Codex review feedback on PR #501 (round 4):

> For type: 'unsigned' this falls back to fromBytes = [] and seqno =
> 0n, so two different unsigned publishers sending the same payload
> on the same topic will produce the same msgId and one publish will
> be falsely deduplicated. Because this helper is now exported
> publicly, consumers can hit that collision even before core wires
> it in.

Real bug at the API surface level. The earlier draft handled
unsigned by zero-filling the publisher and seqno fields, which
preserves the upstream-default false-dedup property — fine if
nothing ever calls it with unsigned messages, but a footgun for
external consumers of the exported function.

V10 configures gossipsub with the StrictSign default, so unsigned
messages don't appear in the wild today. Throwing here makes the
"unsigned not supported in this msgId scheme" stance explicit:

  - any code path that tries to publish an unsigned message fails
    loudly (easy to debug),
  - external consumers of the exported function can't accidentally
    hit the false-dedup case,
  - a future PR that wants to support unsigned has to deliberately
    extend the scheme with a per-message identity (nonce / hash
    prefix / etc.), pinned by tests.

Implementation:
- New `DkgGossipUnsignedMessageError` class (also exported) with a
  message that explains the constraint and points at the workarounds.
- `dkgGossipMsgId` throws the error for `msg.type !== 'signed'` before
  building the hash input.
- Updated tests: replaced the "unsigned: length-framed sha256 with
  seqno=0n and empty from" case with "unsigned: throws
  DkgGossipUnsignedMessageError"; dropped the now-impossible
  "signed and unsigned with same topic/payload differ" case.

Tests:
- core gossip-msg-id 10/10 (was 11; net -1 because the
  signed-vs-unsigned diff test became unreachable)

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: 2 people

packages/core/src/index.ts

@@ -22,6 +22,7 @@ export {
   type ResolveOpts,
   PeerResolver,
   dkgGossipMsgId,
+  DkgGossipUnsignedMessageError,
 } from './network/index.js';
 export {
   ProtocolRouter,

packages/core/src/network/gossip-msg-id.ts

@@ -36,11 +36,28 @@
  * sequence, etc.) maps the same (topic, payload, from, seq) tuple
  * to the same hash.
  *
- * Unsigned messages don't carry a seqno; for them we use `0n`. The
- * upstream behaviour for unsigned was `sha256(data)` — pure dedup
- * by payload, which V10 doesn't rely on. We don't ship unsigned
- * publishes today; the branch exists so the function is total over
- * the `Message` union.
+ * Why throw on unsigned
+ * ---------------------
+ * Codex review feedback on PR #501 round 4: with `type: 'unsigned'`
+ * the message has no publisher identity (no `from`) and no seqno.
+ * The earlier draft fell back to `fromBytes = []` and `seqno = 0n`,
+ * which means two different publishers sending the same payload on
+ * the same topic produce the SAME msgId — one publish gets falsely
+ * deduplicated. (The upstream default for unsigned —
+ * `sha256(data)` — has the same property, but a public function
+ * shouldn't replicate that pitfall in a freshly-shipped contract.)
+ *
+ * V10 configures gossipsub with the StrictSign default, so unsigned
+ * messages don't appear in the wild today. Throwing here makes the
+ * "unsigned not supported in this msgId scheme" stance explicit:
+ *
+ *   - any code path that tries to publish an unsigned message
+ *     fails loudly (easy to debug),
+ *   - external consumers of the exported function can't accidentally
+ *     hit the false-dedup case,
+ *   - a future PR that wants to support unsigned has to deliberately
+ *     extend the scheme with a per-message identity (nonce / hash
+ *     prefix / etc.), pinned by tests.
  *
  * v1 ships only `LibP2PGossipBackend`, so this function only changes
  * which sha256 inputs gossipsub feeds itself; nothing observable on
@@ -50,8 +67,6 @@
 import { sha256 } from '@noble/hashes/sha2.js';
 import type { Message } from '@libp2p/gossipsub';
 
-const EMPTY = new Uint8Array(0);
-
 function u32be(n: number): Uint8Array {
   const b = new Uint8Array(4);
   new DataView(b.buffer).setUint32(0, n, false);
@@ -64,11 +79,28 @@ function u64be(n: bigint): Uint8Array {
   return b;
 }
 
+export class DkgGossipUnsignedMessageError extends Error {
+  constructor() {
+    super(
+      'dkgGossipMsgId: unsigned messages are not supported. The DKG msgId ' +
+        'scheme requires a publisher identity (`from`) and `sequenceNumber` ' +
+        'to avoid false dedup of payload-identical publishes from different ' +
+        'publishers. Use StrictSign (the gossipsub default) or extend the ' +
+        'scheme with a per-message nonce.',
+    );
+    this.name = 'DkgGossipUnsignedMessageError';
+  }
+}
+
 export function dkgGossipMsgId(msg: Message): Uint8Array {
+  if (msg.type !== 'signed') {
+    throw new DkgGossipUnsignedMessageError();
+  }
+
   const topicBytes = new TextEncoder().encode(msg.topic);
   const data = msg.data;
-  const fromBytes = msg.type === 'signed' ? msg.from.toMultihash().bytes : EMPTY;
-  const seqno = msg.type === 'signed' ? msg.sequenceNumber : 0n;
+  const fromBytes = msg.from.toMultihash().bytes;
+  const seqno = msg.sequenceNumber;
 
   const total =
     4 + topicBytes.length +

packages/core/src/network/index.ts

@@ -19,4 +19,4 @@ export type {
 } from './peer-resolver.js';
 export { PeerResolver } from './peer-resolver.js';
 
-export { dkgGossipMsgId } from './gossip-msg-id.js';
+export { dkgGossipMsgId, DkgGossipUnsignedMessageError } from './gossip-msg-id.js';

packages/core/test/gossip-msg-id.test.ts

@@ -1,6 +1,9 @@
 import { describe, it, expect } from 'vitest';
 import { sha256 } from '@noble/hashes/sha2.js';
-import { dkgGossipMsgId } from '../src/network/gossip-msg-id.js';
+import {
+  dkgGossipMsgId,
+  DkgGossipUnsignedMessageError,
+} from '../src/network/gossip-msg-id.js';
 
 /**
  * RFC 07 §5.4 — `dkgGossipMsgId` is the content-deterministic msgId
@@ -61,10 +64,14 @@ describe('dkgGossipMsgId (RFC 07 §5.4)', () => {
     expect(id.length).toBe(32);
   });
 
-  it('unsigned: length-framed sha256 with seqno=0n and empty from', () => {
-    const id = dkgGossipMsgId({ type: 'unsigned', topic: TOPIC, data: PAYLOAD });
-    expect(id).toEqual(expected(TOPIC, PAYLOAD, new Uint8Array(0), 0n));
-    expect(id.length).toBe(32);
+  // Codex review feedback on PR #501 round 4: unsigned messages have
+  // no publisher identity and no seqno, which would let two different
+  // publishers' identical payloads collapse to the same msgId. The
+  // function rejects them so the false-dedup case is impossible.
+  it('unsigned: throws DkgGossipUnsignedMessageError', () => {
+    expect(() =>
+      dkgGossipMsgId({ type: 'unsigned', topic: TOPIC, data: PAYLOAD }),
+    ).toThrow(DkgGossipUnsignedMessageError);
   });
 
   it('different topics → different msgIds (same payload + from + seqno)', () => {
@@ -107,15 +114,6 @@ describe('dkgGossipMsgId (RFC 07 §5.4)', () => {
     expect(a).not.toEqual(b);
   });
 
-  it('signed and unsigned with same topic/payload differ', () => {
-    const signed = dkgGossipMsgId({
-      type: 'signed', topic: TOPIC, data: PAYLOAD, from: fakePeerId,
-      sequenceNumber: 0n, signature: new Uint8Array(), key: {} as never,
-    });
-    const unsigned = dkgGossipMsgId({ type: 'unsigned', topic: TOPIC, data: PAYLOAD });
-    expect(signed).not.toEqual(unsigned);
-  });
-
   it('UTF-8 topics work', () => {
     const utf8Topic = 'cg/тема/1.0.0';
     const id = dkgGossipMsgId({