Drop nonce reuse on RLN retry; consume a fresh nonce instead

The retry path threaded a rejected attempt's message id back so
NonceManager.rollbackNonce could CAS the counter and reclaim the slot.
This bought at most one nonce per retry (a rare event) at the cost of
an Option[Nonce] param, a tuple return, and a concurrent-draw race the
CAS had to defend against.

Simpler to optimistically waste the nonce: checkAndGenerateRLNProof
returns Result[WakuMessage, string] and callers drop the tuple
destructuring. NonceManager.rollbackNonce and its tests, now dead,
are removed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

logos_delivery/waku/node/waku_node/lightpush.nim

@@ -30,8 +30,7 @@ import
   ../../waku_lightpush as lightpush_protocol,
   ../peer_manager,
   ../../common/rate_limit/setting,
-  ../../rln,
-  ../../rln/nonce_manager
+  ../../rln
 
 logScope:
   topics = "waku node lightpush api"
@@ -109,7 +108,6 @@ proc runRlnRefreshRetry(
     node: WakuNode,
     rln: Option[Rln],
     msgWithProof: WakuMessage,
-    drawnMessageId: Option[Nonce],
     pubsubForPublish: PubsubTopic,
     peer: RemotePeerInfo,
     fallback: legacy_lightpush_protocol.WakuLightPushResult[string],
@@ -124,10 +122,8 @@ proc runRlnRefreshRetry(
   proc runRetry(): Future[legacy_lightpush_protocol.WakuLightPushResult[string]] {.
       async, gcsafe
   .} =
-    let (retryMsg, _) = (
-      await checkAndGenerateRLNProof(
-        rln, msgWithProof, regenerate = true, reuseMessageId = drawnMessageId
-      )
+    let retryMsg = (
+      await checkAndGenerateRLNProof(rln, msgWithProof, regenerate = true)
     ).valueOr:
       return err("failed call checkAndGenerateRLNProof from lightpush retry: " & error)
     return await internalLegacyLightpushPublish(node, pubsubForPublish, retryMsg, peer)
@@ -162,7 +158,7 @@ proc legacyLightpushPublish*(
       none(Rln)
     else:
       some(node.rln)
-  let (msgWithProof, drawnMessageId) = (await checkAndGenerateRLNProof(rln, message)).valueOr:
+  let msgWithProof = (await checkAndGenerateRLNProof(rln, message)).valueOr:
     return err("failed call checkAndGenerateRLNProof from lightpush: " & error)
 
   try:
@@ -182,7 +178,7 @@ proc legacyLightpushPublish*(
       return firstResult
 
     return await runRlnRefreshRetry(
-      node, rln, msgWithProof, drawnMessageId, pubsubForPublish, peer, firstResult
+      node, rln, msgWithProof, pubsubForPublish, peer, firstResult
     )
   except CatchableError:
     return err(getCurrentExceptionMsg())
@@ -345,7 +341,7 @@ proc lightpushPublish*(
       none(Rln)
     else:
       some(node.rln)
-  let (msgWithProof, drawnMessageId) = (await checkAndGenerateRLNProof(rln, message)).valueOr:
+  let msgWithProof = (await checkAndGenerateRLNProof(rln, message)).valueOr:
     return lighpushErrorResult(LightPushErrorCode.OUT_OF_RLN_PROOF, error)
 
   let firstResult =
@@ -372,10 +368,8 @@ proc lightpushPublish*(
   rln.get().groupManager.invalidateMerkleProofCache()
 
   proc runRetry(): Future[lightpush_protocol.WakuLightPushResult] {.async, gcsafe.} =
-    let (retryMsg, _) = (
-      await checkAndGenerateRLNProof(
-        rln, msgWithProof, regenerate = true, reuseMessageId = drawnMessageId
-      )
+    let retryMsg = (
+      await checkAndGenerateRLNProof(rln, msgWithProof, regenerate = true)
     ).valueOr:
       return lighpushErrorResult(LightPushErrorCode.OUT_OF_RLN_PROOF, error)
     return

logos_delivery/waku/rln/nonce_manager.nim

@@ -57,13 +57,3 @@ proc getNonce*(n: NonceManager): NonceManagerResult[Nonce] =
     )
 
   return ok(retNonce)
-
-proc rollbackNonce*(n: NonceManager, nonce: Nonce): bool =
-  # CAS: only give back `nonce` when it is still the last id issued.
-  # If another draw advanced the counter between this attempt and the retry,
-  # leave the counter alone — the retry will draw a fresh id rather than
-  # risk a nullifier collision with the intervening message.
-  if n.nextNonce == nonce + 1:
-    n.nextNonce = nonce
-    return true
-  false

logos_delivery/waku/rln/proof.nim

@@ -74,7 +74,7 @@ proc generateRLNProofWithRootRefresh*(
   ## sees an empty cache and refetches from chain. Returns the proof bytes
   ## the caller can attach to the message.
   let proofBytes = (await rln.generateRLNProof(input, senderEpochTime)).valueOr:
-    return err(error)
+    return err("failed to generate RLN proof: " & $error)
 
   let rlnProof = RateLimitProof.init(proofBytes).valueOr:
     return err("could not decode proof for root check: " & $error)
@@ -87,35 +87,21 @@ proc generateRLNProofWithRootRefresh*(
   return await rln.generateRLNProof(input, senderEpochTime)
 
 proc checkAndGenerateRLNProof*(
-    rln: Option[Rln],
-    message: WakuMessage,
-    regenerate: bool = false,
-    reuseMessageId: Option[Nonce] = none(Nonce),
-): Future[Result[tuple[msg: WakuMessage, messageId: Option[Nonce]], string]] {.async.} =
-  ## Returns the message with an attached RLN proof and the message id drawn
-  ## from the nonce manager (`messageId = none` when no id was consumed —
-  ## message already had a valid proof, or RLN is not configured).
-  ## Pass `messageId` back as `reuseMessageId` on a retry so the CAS rollback
-  ## can reclaim it when no concurrent draw has advanced the counter.
+    rln: Option[Rln], message: WakuMessage, regenerate: bool = false
+): Future[Result[WakuMessage, string]] {.async.} =
+  ## Returns the message with an attached RLN proof.
   ## Set `regenerate = true` to bypass the "already has proof" short-circuit
   ## and always generate a fresh proof — used by the retry path after a stale
   ## proof was rejected; the caller should first `invalidateMerkleProofCache`
   ## so the fresh proof is generated against a refetched merkle path.
   if message.proof.len > 0 and not regenerate:
-    return ok((msg: message, messageId: none(Nonce)))
+    return ok(message)
 
   if rln.isNone():
     notice "Publishing message without RLN proof"
-    return ok((msg: message, messageId: none(Nonce)))
+    return ok(message)
 
   let r = rln.get()
-  if reuseMessageId.isSome():
-    if not r.nonceManager.rollbackNonce(reuseMessageId.get()):
-      # A concurrent draw advanced the counter past the rejected attempt's id;
-      # the retry will consume a fresh message id instead of reusing the old one.
-      debug "rln retry: concurrent draw prevented nonce reuse; consuming a fresh message id",
-        attempted = reuseMessageId.get(), nextNonce = r.nonceManager.nextNonce
-
   let
     time = getTime().toUnix()
     senderEpochTime = float64(time)
@@ -128,4 +114,4 @@ proc checkAndGenerateRLNProof*(
   ).valueOr:
     return err("error in checkAndGenerateRLNProof: " & $error)
   msgWithProof.proof = proof.encode().buffer
-  return ok((msg: msgWithProof, messageId: some(nonce)))
+  return ok(msgWithProof)

tests/node/test_wakunode_legacy_lightpush.nim

@@ -121,7 +121,10 @@ suite "RLN Proofs as a Lightpush Service":
     # mount rln-relay
     # match prod epoch window to reduce test flake
     wakuRlnConfig = getWakuRlnConfig(
-      manager = manager, index = MembershipIndex(1), epochSizeSec = 600
+      manager = manager,
+      userMessageLimit = 20,
+      index = MembershipIndex(1),
+      epochSizeSec = 600,
     )
 
     await allFutures(server.start(), client.start())
@@ -166,7 +169,7 @@ suite "RLN Proofs as a Lightpush Service":
       # Attach the RLN proof. In production the client mounts RLN and generates the
       # proof in legacyLightpushPublish; here we generate it using the server's RLN
       # instance since both ends share group state via the in-memory manager.
-      let (msgWithProof, _) =
+      let msgWithProof =
         (await checkAndGenerateRLNProof(some(server.rln), message)).get()
 
       # When the client publishes a message

tests/node/test_wakunode_lightpush.nim

@@ -117,7 +117,10 @@ suite "RLN Proofs as a Lightpush Service":
     # mount rln-relay
     # match prod epoch window to reduce test flake
     wakuRlnConfig = getWakuRlnConfig(
-      manager = manager, index = MembershipIndex(1), epochSizeSec = 600
+      manager = manager,
+      userMessageLimit = 20,
+      index = MembershipIndex(1),
+      epochSizeSec = 600,
     )
 
     await allFutures(server.start(), client.start())
@@ -162,7 +165,7 @@ suite "RLN Proofs as a Lightpush Service":
       # Attach the RLN proof. In production the client mounts RLN and generates the
       # proof in lightpushPublish; here we generate it using the server's RLN instance
       # since both ends share group state via the in-memory manager.
-      let (msgWithProof, _) =
+      let msgWithProof =
         (await checkAndGenerateRLNProof(some(server.rln), message)).get()
 
       # When the client publishes a message
@@ -183,8 +186,7 @@ suite "RLN Proofs as a Lightpush Service":
       # invalidateMerkleProofCache empties the cached path so the next
       # proof-gen refetches from chain, and `regenerate = true` on
       # checkAndGenerateRLNProof bypasses the "already has proof" short-circuit.
-      let (firstMsg, drawnNonce) =
-        (await checkAndGenerateRLNProof(some(server.rln), message)).get()
+      let firstMsg = (await checkAndGenerateRLNProof(some(server.rln), message)).get()
       check firstMsg.proof.len > 0
 
       # Corrupt the cache to model a stale/invalid witness — the same state a
@@ -195,20 +197,16 @@ suite "RLN Proofs as a Lightpush Service":
       check manager.merkleProofCache != goodCache
 
       # Retry path: invalidate the cache so the next proof-gen refetches from
-      # chain, then regenerate the proof. Pass drawnNonce so the CAS rollback
-      # reclaims it — mirroring the production retry path.
+      # chain, then regenerate the proof.
       manager.invalidateMerkleProofCache()
-      let (secondMsg, _) = (
-        await checkAndGenerateRLNProof(
-          some(server.rln), firstMsg, regenerate = true, reuseMessageId = drawnNonce
-        )
+      let secondMsg = (
+        await checkAndGenerateRLNProof(some(server.rln), firstMsg, regenerate = true)
       ).get()
 
       check:
         secondMsg.proof.len > 0
         # Regenerated, not passed through — Groth16 proofs carry random
-        # blinding, so a fresh call produces different bytes even though the
-        # rejected attempt's message id is intentionally reused on retry.
+        # blinding, so a fresh call produces different bytes.
         secondMsg.proof != firstMsg.proof
         # Cache was refetched from chain, overwriting the corruption.
         manager.merkleProofCache == goodCache

tests/waku_rln_relay/test_rln_nonce_manager.nim

@@ -41,34 +41,3 @@ suite "Nonce manager":
     check:
       nonce == 0.uint
       nonce2 == 0.uint
-
-  test "rollbackNonce reclaims the last-drawn id when counter has not advanced":
-    let nm = NonceManager.init(nonceLimit = 100.uint)
-    let first = nm.getNonce().valueOr:
-      raiseAssert $error
-    check nm.rollbackNonce(first)
-    let reissued = nm.getNonce().valueOr:
-      raiseAssert $error
-
-    check:
-      first == 0.uint
-      reissued == first
-      nm.nextNonce == 1.uint
-
-  test "rollbackNonce is a no-op when another draw has advanced the counter":
-    let nm = NonceManager.init(nonceLimit = 100.uint)
-    let first = nm.getNonce().valueOr:
-      raiseAssert $error
-    # A second draw slips in — simulates concurrent publish between attempt 1
-    # and the retry. The counter now points past first+1.
-    discard nm.getNonce().valueOr:
-      raiseAssert $error
-    check not nm.rollbackNonce(first)
-    check nm.nextNonce == 2.uint
-
-  test "rollbackNonce is a no-op when no nonce has been issued":
-    let nm = NonceManager.init(nonceLimit = 100.uint)
-    check not nm.rollbackNonce(Nonce(0))
-
-    check:
-      nm.nextNonce == 0.uint