From f2a7457fde501b1c0d083cd10a2e7c50be97a418 Mon Sep 17 00:00:00 2001
From: Darius Clark <dariusc93@users.noreply.github.com>
Date: Tue, 4 Apr 2023 06:52:16 -0400
Subject: [PATCH] fix(gossipsub): prevent erroneously duplicate message IDs

Previously, we only mutably borrowed the `last_seq_no` in the current scope but did not modify the underlying number. This is because `u64` is copy and calling `wrapping_add` consumes `self` so the compiler just copied it. We introduce a new-type instead that is not `Copy`.

Additionally, `wrapping_add` and initializing with a random u64 might actually warp the number and thus not give us sequential numbers as intended in #3551. To solve this, we initialize with the current unix timestamp in nanoseconds. This allows a node to publish 1000000 messages a second and still not reuse sequence numbers even after a restart / re-initialization of the configuration. This is also what the go implementation does.

Resolves #3714.

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>

Pull-Request: #3716.
---
 Cargo.lock                           |  2 +-
 protocols/gossipsub/CHANGELOG.md     |  6 +++++
 protocols/gossipsub/Cargo.toml       |  2 +-
 protocols/gossipsub/src/behaviour.rs | 40 +++++++++++++++++++++-------
 4 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 9924e84c..313fa3df 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2442,7 +2442,7 @@ dependencies = [
 
 [[package]]
 name = "libp2p-gossipsub"
-version = "0.44.2"
+version = "0.44.3"
 dependencies = [
  "async-std",
  "asynchronous-codec",
diff --git a/protocols/gossipsub/CHANGELOG.md b/protocols/gossipsub/CHANGELOG.md
index c834ebd5..6f68b5fe 100644
--- a/protocols/gossipsub/CHANGELOG.md
+++ b/protocols/gossipsub/CHANGELOG.md
@@ -1,3 +1,9 @@
+## 0.44.3 - unreleased
+
+- Fix erroneously duplicate message IDs. See [PR 3716].
+
+[PR 3716]: https://github.com/libp2p/rust-libp2p/pull/3716
+
 ## 0.44.2
 
 - Signed messages now use sequential integers in the sequence number field.
diff --git a/protocols/gossipsub/Cargo.toml b/protocols/gossipsub/Cargo.toml
index bcf47730..d8088436 100644
--- a/protocols/gossipsub/Cargo.toml
+++ b/protocols/gossipsub/Cargo.toml
@@ -3,7 +3,7 @@ name = "libp2p-gossipsub"
 edition = "2021"
 rust-version = "1.62.0"
 description = "Gossipsub protocol for libp2p"
-version = "0.44.2"
+version = "0.44.3"
 authors = ["Age Manning <Age@AgeManning.com>"]
 license = "MIT"
 repository = "https://github.com/libp2p/rust-libp2p"
diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs
index 4b358afd..256390ad 100644
--- a/protocols/gossipsub/src/behaviour.rs
+++ b/protocols/gossipsub/src/behaviour.rs
@@ -64,6 +64,7 @@ use crate::types::{
 use crate::types::{PeerConnections, PeerKind, Rpc};
 use crate::{rpc_proto::proto, TopicScoreParams};
 use crate::{PublishError, SubscriptionError, ValidationError};
+use instant::SystemTime;
 use quick_protobuf::{MessageWrite, Writer};
 use std::{cmp::Ordering::Equal, fmt::Debug};
 use wasm_timer::Interval;
@@ -149,20 +150,44 @@ pub enum Event {
 /// A data structure for storing configuration for publishing messages. See [`MessageAuthenticity`]
 /// for further details.
 #[allow(clippy::large_enum_variant)]
-#[derive(Clone)]
 enum PublishConfig {
     Signing {
         keypair: Keypair,
         author: PeerId,
         inline_key: Option<Vec<u8>>,
-        last_seq_no: u64, // This starts from a random number and increases then overflows (if
-                          // required)
+        last_seq_no: SequenceNumber,
     },
     Author(PeerId),
     RandomAuthor,
     Anonymous,
 }
 
+/// A strictly linearly increasing sequence number.
+///
+/// We start from the current time as unix timestamp in milliseconds.
+#[derive(Debug)]
+struct SequenceNumber(u64);
+
+impl SequenceNumber {
+    fn new() -> Self {
+        let unix_timestamp = SystemTime::now()
+            .duration_since(SystemTime::UNIX_EPOCH)
+            .expect("time to be linear")
+            .as_nanos();
+
+        Self(unix_timestamp as u64)
+    }
+
+    fn next(&mut self) -> u64 {
+        self.0 = self
+            .0
+            .checked_add(1)
+            .expect("to not exhaust u64 space for sequence numbers");
+
+        self.0
+    }
+}
+
 impl PublishConfig {
     pub fn get_own_id(&self) -> Option<&PeerId> {
         match self {
@@ -192,7 +217,7 @@ impl From<MessageAuthenticity> for PublishConfig {
                     keypair,
                     author: public_key.to_peer_id(),
                     inline_key: key,
-                    last_seq_no: rand::random(),
+                    last_seq_no: SequenceNumber::new(),
                 }
             }
             MessageAuthenticity::Author(peer_id) => PublishConfig::Author(peer_id),
@@ -2757,12 +2782,9 @@ where
                 ref keypair,
                 author,
                 inline_key,
-                mut last_seq_no,
+                last_seq_no,
             } => {
-                // Increment the last sequence number
-                last_seq_no = last_seq_no.wrapping_add(1);
-
-                let sequence_number = last_seq_no;
+                let sequence_number = last_seq_no.next();
 
                 let signature = {
                     let message = proto::Message {