Split checkAndGenerateRLNProof: extract attachRLNProof

The `regenerate: bool` param was control coupling: every caller passing
`regenerate = true` was a retry path that had already established
`rln.isSome()` (it just called `invalidateMerkleProofCache` on the
group manager) and wanted an unconditional fresh proof, not a "check
and maybe generate".

Extract that into `attachRLNProof(r: Rln, message)`, which always draws
a fresh nonce and rebuilds the proof, replacing any existing one.
`checkAndGenerateRLNProof(rln: Option[Rln], message)` keeps only its
"check" behaviour — short-circuit on an existing proof, pass through
when RLN is not configured — and delegates to `attachRLNProof`.

Taking a concrete `Rln` deletes the previously unreachable
`regenerate = true` + `rln.isNone()` branch (which would have
republished the stale rejected proof) at the type level, and removes
the duplicated nonce-draw/generateProof/encode body that
checkAndGenerateRLNProof carried alongside generateRLNProof.

The two lightpush retry sites and the invalidate+regenerate test now
call attachRLNProof directly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

logos_delivery/waku/node/waku_node/lightpush.nim

@@ -93,7 +93,9 @@ proc resolveLegacyPubsubTopic(
     node: WakuNode, pubsubTopic: Option[PubsubTopic], contentTopic: ContentTopic
 ): Result[PubsubTopic, string] =
   ## Returns the explicit pubsub topic if provided, otherwise derives it from
-  ## `contentTopic` via autosharding.
+  ## `contentTopic` via autosharding. Unlike v3 lightpush, the legacy wire
+  ## format carries a mandatory pubsub topic and the server never derives it,
+  ## so the client must resolve the topic before building the request.
   if pubsubTopic.isSome():
     return ok(pubsubTopic.get())
   if node.wakuAutoSharding.isNone():
@@ -122,10 +124,8 @@ proc runRlnRefreshRetry(
   proc runRetry(): Future[legacy_lightpush_protocol.WakuLightPushResult[string]] {.
       async, gcsafe
   .} =
-    let retryMsg = (
-      await checkAndGenerateRLNProof(rln, msgWithProof, regenerate = true)
-    ).valueOr:
-      return err("failed call checkAndGenerateRLNProof from lightpush retry: " & error)
+    let retryMsg = (await attachRLNProof(rln.get(), msgWithProof)).valueOr:
+      return err("failed call attachRLNProof from lightpush retry: " & error)
     return await internalLegacyLightpushPublish(node, pubsubForPublish, retryMsg, peer)
 
   let retryFut = runRetry()
@@ -368,9 +368,7 @@ proc lightpushPublish*(
   rln.get().groupManager.invalidateMerkleProofCache()
 
   proc runRetry(): Future[lightpush_protocol.WakuLightPushResult] {.async, gcsafe.} =
-    let retryMsg = (
-      await checkAndGenerateRLNProof(rln, msgWithProof, regenerate = true)
-    ).valueOr:
+    let retryMsg = (await attachRLNProof(rln.get(), msgWithProof)).valueOr:
       return lighpushErrorResult(LightPushErrorCode.OUT_OF_RLN_PROOF, error)
     return
       await lightpushPublishHandler(node, pubsubForPublish, retryMsg, toPeer, mixify)

logos_delivery/waku/rln/proof.nim

@@ -86,32 +86,32 @@ proc generateRLNProofWithRootRefresh*(
   rln.groupManager.invalidateMerkleProofCache()
   return await rln.generateRLNProof(input, senderEpochTime)
 
-proc checkAndGenerateRLNProof*(
-    rln: Option[Rln], message: WakuMessage, regenerate: bool = false
+proc attachRLNProof*(
+    r: Rln, message: WakuMessage
 ): 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:
+  ## Returns the message with a freshly generated RLN proof attached,
+  ## replacing any existing proof. Always draws a new message id from the
+  ## nonce manager. Retry paths that suspect a stale merkle path should call
+  ## `invalidateMerkleProofCache` first so the proof is generated against a
+  ## refetched path.
+  var msgWithProof = message
+  msgWithProof.proof = (
+    await r.generateRLNProof(message.toRLNSignal(), float64(getTime().toUnix()))
+  ).valueOr:
+    return err("error in attachRLNProof: " & error)
+  return ok(msgWithProof)
+
+proc checkAndGenerateRLNProof*(
+    rln: Option[Rln], message: WakuMessage
+): Future[Result[WakuMessage, string]] {.async.} =
+  ## Returns the message with an attached RLN proof. Passes the message
+  ## through unchanged when it already carries a proof or when RLN is not
+  ## configured.
+  if message.proof.len > 0:
     return ok(message)
 
   if rln.isNone():
     notice "Publishing message without RLN proof"
     return ok(message)
 
-  let r = rln.get()
-  let
-    time = getTime().toUnix()
-    senderEpochTime = float64(time)
-    epoch = r.calcEpoch(senderEpochTime)
-  let nonce = r.nonceManager.getNonce().valueOr:
-    return err("could not get new message id to generate an rln proof: " & $error)
-  var msgWithProof = message
-  let proof = (
-    await r.groupManager.generateProof(msgWithProof.toRLNSignal(), epoch, nonce)
-  ).valueOr:
-    return err("error in checkAndGenerateRLNProof: " & $error)
-  msgWithProof.proof = proof.encode().buffer
-  return ok(msgWithProof)
+  return await attachRLNProof(rln.get(), message)

tests/node/test_wakunode_lightpush.nim

@@ -184,8 +184,8 @@ suite "RLN Proofs as a Lightpush Service":
       # Exercises the primitive pair that lightpushPublish leans on after a
       # 420 (INVALID_MESSAGE) or 504 (OUT_OF_RLN_PROOF) rejection: calling
       # 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.
+      # proof-gen refetches from chain, and attachRLNProof rebuilds the proof
+      # even though the message already carries one.
       let firstMsg = (await checkAndGenerateRLNProof(some(server.rln), message)).get()
       check firstMsg.proof.len > 0
 
@@ -199,9 +199,7 @@ suite "RLN Proofs as a Lightpush Service":
       # Retry path: invalidate the cache so the next proof-gen refetches from
       # chain, then regenerate the proof.
       manager.invalidateMerkleProofCache()
-      let secondMsg = (
-        await checkAndGenerateRLNProof(some(server.rln), firstMsg, regenerate = true)
-      ).get()
+      let secondMsg = (await attachRLNProof(server.rln, firstMsg)).get()
 
       check:
         secondMsg.proof.len > 0