Possible upgrade to lighthouse's IDONTWANT GossipSub implementation

[cortze]

Description

After last Thursday's discussion on the bandwidth savings that IDONTWANT messages could bring, we've been (the ProbeLab team) monitoring the behaviour of GossipSub after the addition of these into go-libp2p.

We've used our light-network-client Hermes to trace down interactions at the libp2p level, and they have shown that lighthouse's libp2p gossipsub implementation could be more efficient.

Version

I could narrow down the behaviour to v5.3.0-d6ba8c3 from a remote peer's AgentVersion (Lighthouse/v5.3.0-d6ba8c3/x86_64-linux). However, the code seems to be present at the stable branch as well.

Present Behaviour

The current logic to process an incoming message will ALWAYS send an IDONTWANT message when a message is received.
As far as I could see in the following code snippet, there is no filter that would prevent the host from sending an IDONTWANT after the arrival of any message (even if it's duplicated).

fn handle_received_message(
        ...
        // Broadcast IDONTWANT messages.
        self.send_idontwant(&raw_message, &msg_id, propagation_source);

        // Check the validity of the message
        // Peers get penalized if this message is invalid. We don't add it to the duplicate cache
        // and instead continually penalize peers that repeatedly send this message.
        if !self.message_is_valid(&msg_id, &mut raw_message, propagation_source) {
            return;
        }

        if !self.duplicate_cache.insert(msg_id.clone()) {
            tracing::debug!(message=%msg_id, "Message already received, ignoring");
            if let Some((peer_score, ..)) = &mut self.peer_score {
                peer_score.duplicated_message(propagation_source, &msg_id, &message.topic);
            }
            self.mcache.observe_duplicate(&msg_id, propagation_source);
            return;
        }
        tracing::debug!(
            message=%msg_id,
            "Put message in duplicate_cache and resolve promises"
        );
        ...

Expected Behaviour

The main advantage of using IDONTWANT messages is to reduce the overall bandwidth by reducing the number of duplicates that a node receives. However, this reduction is mostly effective when the size of the messages is "big enough". From the GossipSub 1.2 spec:

The IDONTWANT may have negative effect on small messages as it may increase the overall traffic and CPU load. Thus it is better to utilize IDONTWANT for messages of a larger size. The exact policy of IDONTWANT appliance is outside of the spec scope. Every implementation MAY choose whatever is more appropriate for it. Possible options are either choose a message size threshold and broadcast IDONTWANT on per message basis when the size is exceeded or just use IDONTWANT for all messages on selected topics.

With the current implementation, the node is not applying any of the filters that could be applied:

1. limit the node to send IDONTWANTs on specific topics (as the number of topics with "larger" msg sizes are already known)
2. limit the node to send IDONTWANTs only if the message size is over a threshold (go-libp2p-pubsub does it this way)
3. limit the node to send IDONTWANTs only the first time it sees a message (this seems to be the minimum)

Steps to resolve

A simple enough solution could be applying the upper 2 and 3 solutions.
Leaving the fn handle_received_message() logic as follows:

1. check if the message was seen previously (even if it's invalid, this could prevent spending resources validating a duplicated message in the future)
2. send an IDONTWANT to our mesh only if the msg size is bigger than an IdownwantMessageSizeThreshold configurable constant or similar. This prevents others from sending duplicates (and it should only be done once)
3. validate the message to penalize the remote peer or notify the app layer

I'm not a rust expert, but let me know if I can help somehow ✌️

[AgeManning]

Hey, thanks for this!

These are great suggestions. I think the best benefit and easiest to implement here would be to do 2 and 3.

3. Should be as simple as moving the send_idontwant() down a few lines so its below the duplicate_cache check.

i.e below:

        if !self.duplicate_cache.insert(msg_id.clone()) {
            tracing::debug!(message=%msg_id, "Message already received, ignoring");
            if let Some((peer_score, ..)) = &mut self.peer_score {
                peer_score.duplicated_message(propagation_source, &msg_id, &message.topic);
            }
            self.mcache.observe_duplicate(&msg_id, propagation_source);
            return;
        }

2. Is easy enough to implement with a size configuration. I guess we have to decide what size to base it on, the raw compressed size or the uncompressed size. I think it makes sense to base it on the raw size.

I believe one of us should have a PR for these up soon :).

[jimmygchen]

Completed in #6456 🎉