From ef6e277df7422a37b1bbd92ac97f933d63d4cd97 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Fri, 16 Jul 2021 15:02:22 -0700
Subject: [PATCH] fix: make timestamps strictly increasing (#201)

* fix: make timestamps strictly increasing

On Linux, this is almost always the case. Windows, however, doesn't have
nanosecond accuracy.

We make the timestamp sequence numbers strictly increasing by returning
the last timestamp + 1 where necessary.

* apply code review

Co-authored-by: Marten Seemann <martenseemann@gmail.com>

* use a lock

Co-authored-by: Marten Seemann <martenseemann@gmail.com>
---
 peer/record.go      | 17 ++++++++++++++++-
 peer/record_test.go | 13 +++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/peer/record.go b/peer/record.go
index 3968fc22..212cea72 100644
--- a/peer/record.go
+++ b/peer/record.go
@@ -2,6 +2,7 @@ package peer
 
 import (
 	"fmt"
+	"sync"
 	"time"
 
 	pb "github.com/libp2p/go-libp2p-core/peer/pb"
@@ -125,9 +126,23 @@ func PeerRecordFromProtobuf(msg *pb.PeerRecord) (*PeerRecord, error) {
 	return record, nil
 }
 
+var (
+	lastTimestampMu sync.Mutex
+	lastTimestamp   uint64
+)
+
 // TimestampSeq is a helper to generate a timestamp-based sequence number for a PeerRecord.
 func TimestampSeq() uint64 {
-	return uint64(time.Now().UnixNano())
+	now := uint64(time.Now().UnixNano())
+	lastTimestampMu.Lock()
+	defer lastTimestampMu.Unlock()
+	// Not all clocks are strictly increasing, but we need these sequence numbers to be strictly
+	// increasing.
+	if now <= lastTimestamp {
+		now = lastTimestamp + 1
+	}
+	lastTimestamp = now
+	return now
 }
 
 // Domain is used when signing and validating PeerRecords contained in Envelopes.
diff --git a/peer/record_test.go b/peer/record_test.go
index adbe03ae..4022d929 100644
--- a/peer/record_test.go
+++ b/peer/record_test.go
@@ -52,3 +52,16 @@ func TestSignedPeerRecordFromEnvelope(t *testing.T) {
 		}
 	})
 }
+
+// This is pretty much guaranteed to pass on Linux no matter how we implement it, but Windows has
+// low clock precision. This makes sure we never get a duplicate.
+func TestTimestampSeq(t *testing.T) {
+	var last uint64
+	for i := 0; i < 1000; i++ {
+		next := TimestampSeq()
+		if next <= last {
+			t.Errorf("non-increasing timestamp found: %d <= %d", next, last)
+		}
+		last = next
+	}
+}