fix(p2p): frame gossipsub msgId and restrict allowedTopics (A-1256)

[PhilWindle]

Fixes A-1256.

Problem

The gossipsub full message id was SHA256(topic || data)[0..20] with no framing between the topic string and the message bytes, and LibP2PService set no allowedTopics. Raw concatenation isn't injective: for a real message (T, D), a peer can craft T' = T + D[0], D' = D[1:] so T' || D' is byte-identical to T || D → same msgId. ChainSafe gossipsub transforms the arbitrary-topic message and inserts that id into seenCache before the subscription check (it isn't delivered to us, since we're not subscribed to T'). When the genuine proposal/attestation arrives on T, gossipsub drops it as a duplicate before application validation, peer scoring, or handling — suppressing a time-sensitive consensus message within the slot.

Fix (defense in depth — either alone breaks the attack)

1. Frame the msgId input as uint32be(topicLen) || topic || data (encoding.ts). The topic length pins the (topic, data) boundary, so a boundary-shifted pair no longer collides. (getMsgIdFn's parameter is narrowed to Pick<Message, 'topic' | 'data'> — the only fields it reads — which stays assignable to gossipsub's msgIdFn slot.)
2. Set exact allowedTopics (libp2p_service.ts) to the subscribed Aztec topic strings. Verified against the installed gossipsub: the allowlist is enforced in handleReceivedRpc before handleReceivedMessage/seenCache.put, so an unsubscribed-topic message is dropped before transform / msgId / seenCache.

Test

encoding.test.ts: builds a real P2PMessage.toMessageData() buffer (confirming data[0] === 0x00), constructs the shifted (T', D'), and asserts the two msg ids now differ (they collided before the framing change); plus a determinism check.

Compatibility

msgId is computed locally for dedup; changing the function doesn't change any on-wire format. During a rolling upgrade, mixed nodes briefly compute different ids for the same message (minor IHAVE/IWANT inefficiency), with no correctness impact.

[fcarreiro]

Are any changes needed to fastMsgIdFn or msgIdToStrFn above?

I'm a bit unclear on when those are used (and why it's called fast, since I think using webcrypto.subtle is faster)

[PhilWindle]

Yes, you are right. Though I actually think the best course of action is to just remove the fastMsgId. It's an optional optimisation that actually makes the message caching non-collision resistant.