go-libp2p-pubsub icon indicating copy to clipboard operation
go-libp2p-pubsub copied to clipboard

Broadcast Loops

Open Stebalien opened this issue 1 year ago • 17 comments

In F3, we've run into some pretty severe broadcast loops. From what we can tell, the time cache is basically useless.

The core issue is that we have NO TTLs and/or expiration on messages. Personally, I'd recommend a simple expiration to all messages that's strictly less than the time cache duration. We can even treat the time cache duration as a network parameter, refusing to propagate messages that have an expiration more than that duration in the future.

Stebalien avatar Aug 14 '24 17:08 Stebalien

I completely agree.

I think we should have ttls in utc seconds, and the signature should probably include them when present.

I think we need a pubsub spec change for this, i'll be happy to support you in the interest group in specs.

vyzo avatar Aug 14 '24 17:08 vyzo

actually expiration in the envelope.

vyzo avatar Aug 14 '24 17:08 vyzo

hrm, including in the signature maybe backwards incompatible, so it can only be advisory as it may be tampered with.

vyzo avatar Aug 14 '24 17:08 vyzo

Make it a per-topic option.

Stebalien avatar Aug 14 '24 17:08 Stebalien

Specifically, repurpose the seqno?

Stebalien avatar Aug 14 '24 17:08 Stebalien

I think this would break some systems that rely on it.

vyzo avatar Aug 14 '24 17:08 vyzo

Make it a per-topic option.

Sure.

vyzo avatar Aug 14 '24 17:08 vyzo

I think this would break some systems that rely on it.

Rely on it being sequential?

Stebalien avatar Aug 14 '24 18:08 Stebalien

But yeah, making this a per-topic option would go a long way.

Stebalien avatar Aug 14 '24 18:08 Stebalien

I think this would break some systems that rely on it.

Rely on it being sequential?

Rather monotonically increasing.

vyzo avatar Aug 14 '24 18:08 vyzo

Rather monotonically increasing.

unixnanos? unixnanos with a counter in case we try to go backwards?

Stebalien avatar Aug 14 '24 18:08 Stebalien

Rather monotonically increasing.

unixnanos? unixnanos with a counter in case we try to go backwards?

unixnano init and then sequentially increasing. I don't see anything wrong with allowing an initializer, but we just do unixnano without an initializer for now i think.

vyzo avatar Aug 14 '24 18:08 vyzo

Well, specifically I was thinking about having a guaranteed monotonically increasing clock. I.e.:

  1. Get unix nanos.
  2. Look at the last sequence number.
  3. If less than or equal to the last sequence number, use the last sequence number plus 1.

We aren't going to broadcast one message per nanosecond, but I like being extra paranoid.

Stebalien avatar Aug 15 '24 04:08 Stebalien

Yes -- the part that is missing is knowledge of the last seqno basically. The application would need to store it somewhere, load it on restart, and pass it in the constructor.

vyzo avatar Aug 15 '24 06:08 vyzo

The application would need to store it somewhere, load it on restart, and pass it in the constructor.

Can we not just initialize to the current time on reboot? According to kuba, that's what we already do. To deal with fast reboots, we can insert a very tiny sleep before sending the first message.

Stebalien avatar Aug 15 '24 15:08 Stebalien

sure.

vyzo avatar Aug 15 '24 15:08 vyzo

MSL default is around 2 minutes. IMO, in P2P/gossipsub nets, 'Max_network_dissemination_time + MSL' should suffice unless some in-network duplications happen.

In that case, using a timestamp and including it in the message signature can be a good option (can guard against message replays as well). We can have a topic-based time threshold to process messages. The downsides: 1) backward incompatibility 2) Same message generated by different peers will be considered different messages.

ufarooqstatus avatar Jan 22 '25 08:01 ufarooqstatus