From 5417ba399d644d41d2531edcb2a4a10c2022603b Mon Sep 17 00:00:00 2001
From: pucedoteth <119044801+pucedoteth@users.noreply.github.com>
Date: Sat, 29 Aug 2026 01:14:53 +0200
Subject: [PATCH] fix: hand out unique nonces when the counter catches up

#108 added a catch-up branch to next_nonce so an idle client whose
counter has fallen behind the wall clock does not keep signing with stale
nonces. Its goal was that next_nonce always returns a unique value, but
the branch returns now_ms to every caller that reaches it:

    if nonce + 300000 < now_ms {
        CUR_NONCE.fetch_max(now_ms + 1, Ordering::Relaxed);
        return now_ms;
    }

The counter is advanced past now_ms, but the value handed back is not
claimed from it. Concurrent callers that read a stale nonce before the
first fetch_max lands all take this branch and all return the same
now_ms, and the exchange rejects the duplicates.

The window is small but it opens exactly when a client is most likely to
send a burst: after an idle period, which is what puts the counter more
than 300 seconds behind in the first place.

Catch the counter up and then claim a slot from it, so each caller in the
branch gets its own value. The first caller still returns now_ms, so
single-threaded behaviour is unchanged.

Adds a regression test that fails reliably on the current code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/helpers.rs | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/src/helpers.rs b/src/helpers.rs
index c642af7e..e4dcf82d 100644
--- a/src/helpers.rs
+++ b/src/helpers.rs
@@ -20,8 +20,11 @@ pub(crate) fn next_nonce() -> u64 {
     }
     // more than 300 seconds behind
     if nonce + 300000 < now_ms {
-        CUR_NONCE.fetch_max(now_ms + 1, Ordering::Relaxed);
-        return now_ms;
+        // Catch the counter up to the wall clock, then claim a slot from it. Returning
+        // `now_ms` directly hands the same nonce to every caller that races into this
+        // branch, and the exchange rejects the duplicates.
+        CUR_NONCE.fetch_max(now_ms, Ordering::Relaxed);
+        return CUR_NONCE.fetch_add(1, Ordering::Relaxed);
     }
     nonce
 }
@@ -95,6 +98,37 @@ lazy_static! {
 mod tests {
     use super::*;
 
+    #[test]
+    fn next_nonce_is_unique_after_catch_up() {
+        for _ in 0..50 {
+            // Park the counter far enough behind the wall clock that every caller takes the
+            // catch-up branch, which is what an idle client hits before its next burst.
+            let now = now_timestamp_ms();
+            CUR_NONCE.store(now - 400_000, Ordering::Relaxed);
+
+            let barrier = std::sync::Arc::new(std::sync::Barrier::new(8));
+            let handles: Vec<_> = (0..8)
+                .map(|_| {
+                    let barrier = barrier.clone();
+                    std::thread::spawn(move || {
+                        barrier.wait();
+                        next_nonce()
+                    })
+                })
+                .collect();
+            let nonces: Vec<u64> = handles.into_iter().map(|h| h.join().unwrap()).collect();
+
+            let mut sorted = nonces.clone();
+            sorted.sort_unstable();
+            sorted.dedup();
+            assert_eq!(
+                sorted.len(),
+                nonces.len(),
+                "duplicate nonces handed out: {nonces:?}"
+            );
+        }
+    }
+
     #[test]
     fn float_to_string_for_hashing_test() {
         assert_eq!(float_to_string_for_hashing(0.), "0".to_string());