Gossipsub: message hash field in a message

[jamesray1]

The gossipsub spec refers to message IDs, but doesn't actually include a message ID field in the protobuf file (and neither does the pubsub spec). Having such a field would be useful for implementing a conversion (using a From trait with Rust) between a message ID and a message, e.g. like so:

impl From<GMessage> for MsgId {
    #[inline]
    fn from(message: GMessage) -> Self {
        message.id
    }
}

For more details see message.rs (as part of libp2p/rust-libp2p#767).

cc @vyzo, @mgoelzer, @whyrusleeping

[jamesray1]

On second thought, like libp2p/rust-libp2p#473, it may be better to just use a message hash, and not use a message ID, although if the latter is already implemented then it can be used in gossipsub 1.0, and the former in 2.0. Of course, it can be done independently of the protobuf file, but if this feature is included in one implementation, it should probably be included in all implementations, and thus in the protobuf file for over the wire interoperability.

[vyzo]

The message ID should be in the pubsub spec; if it's missing, it's a bug there.

[jamesray1]

There are messageIDs in ControlIHave and ControlIWant, but not in Message. I'll make a PR.

[jamesray1]

libp2p/go-libp2p-pubsub#137

[vyzo]

The message ID field already exists in the RPC envelope, this duplicates.
What I meant is that it should be in the pubsub spec if it's not already there.

[vyzo]

It's called seqno in the RPC envelope. The message ID is synthetic, constructed by the peer ID concatenated with seqno. If we want to make it a hash, it will have to change there.

[vyzo]

Note that the type is already bytes, so there shouldn't be a compatibility issue.
We can also already use a hash instead of an atomic counter, that's up to the peer.

[jamesray1]

OK I found in the spec that it says "(Notably the 'timecache' in use by the floodsub implementation uses the concatenation of the seqno and the from field.)" Still, the link between this and the message ID in the spec could be clearer.

[vyzo]

Yes, that might be worth improving (and explicitly saying what is the message id and how it is derived in a more prominent place).

A potential caveat with using hashes instead of seqnos: the peer won't be able to send identical messages (eg keepalives) within the timecache interval, as they will get rejected as duplicates.

[jamesray1]

#117

[jamesray1]

A potential caveat with using hashes instead of seqnos: the peer won't be able to send identical messages (eg keepalives) within the timecache interval, as they will get rejected as duplicates.

However, I think a MsgRep enum that contains either a MsgHash or MsgId may be a solution for this, and the MsgHash is constructed from the whole message, not just the seq_no and source fields.

For more details, see libp2p/rust-libp2p#767.

[jamesray1]

A potential caveat with using hashes instead of seqnos: the peer won't be able to send identical messages (eg keepalives) within the timecache interval, as they will get rejected as duplicates.

However, I think a MsgRep enum that contains either a MsgHash or MsgId may be a solution for this, and the MsgHash is constructed from the whole message, not just the seq_no and source fields. But implementing these are causing errors at the moment, at least one of which is caused by an upstream issue with rust-protobuf.

For more details, see libp2p/rust-libp2p#767.

[jamesray1]

On further analysis, it seems that a MsgId is more cumbersome than a MsgHash. It is relatively simple to convert a MsgHash back into a message, without needing to search through state, which does not appear to be the case for a MsgId. Therefore I intend to not use a message ID and thus neither a MsgRep in my implementation.

// Unlike a `MsgHash`, we can't rebuild a `GMessage` from a `MsgId`(or
// at least it isn't as easy).
// We have to fetch it from somewhere it is stored. In this context,
// this would be the `MCache`, although messages are only stored for a
// few seconds / heartbeat intervals, hence implementing `From` won't work.
// Using `TryFrom` also adds complications.
// An alternative is to reconstruct a `GMessage` from a `MsgId` by searching
// for a message that has the same seq_no and peer ID in all the state, but
// this is resource-intensive. Why do that when it seems simpler to just use
// a `MsgHash`? It is therefore recommended to do that: just use a MsgHash,
// and not use and probably remove a `MsgId` and `MsgRep`, which will also
// simplify implementation.

[mxinden]

This seems to be fixed with #117, thus I am closing here. Feel free to comment in case you would like to continue the discussion.