Tighten RLN proof-refresh docstrings and comments

Condense the verbose comments introduced with the reactive merkle
proof-refresh work. No code changes.

- group_manager_base: base methods carry the contract; the
  OnchainGroupManager overrides keep only their concrete behavior
  instead of repeating the rationale.
- proof.nim, lightpush.nim, constants.nim: trim multi-line docstrings
  and inline comments to their load-bearing facts.

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

logos_delivery/waku/node/waku_node/lightpush.nim

@@ -92,10 +92,9 @@ proc internalLegacyLightpushPublish(
 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. 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.
+  ## Returns the explicit pubsub topic, else derives it from `contentTopic`
+  ## via autosharding. The legacy wire format requires a pubsub topic and the
+  ## server never derives it, so the client must resolve it here.
   if pubsubTopic.isSome():
     return ok(pubsubTopic.get())
   if node.wakuAutoSharding.isNone():
@@ -114,11 +113,9 @@ proc runRlnRefreshRetry(
     peer: RemotePeerInfo,
     fallback: legacy_lightpush_protocol.WakuLightPushResult[string],
 ): Future[legacy_lightpush_protocol.WakuLightPushResult[string]] {.async, gcsafe.} =
-  ## Force-refreshes the RLN merkle proof path and retries the publish once.
-  ## Only the refresh (on-chain refetch + proof regeneration) is bounded by
-  ## RlnMerkleProofRefreshTimeout — a hanging RPC cannot stall the caller
-  ## indefinitely and `fallback` (the original rejection) is returned instead.
-  ## The retried publish itself runs unbounded, matching the first attempt.
+  ## Refreshes the RLN merkle proof path and retries the publish once. Only the
+  ## refresh is bounded by RlnMerkleProofRefreshTimeout (returning `fallback` on
+  ## timeout); the retried publish runs unbounded, matching the first attempt.
   info "legacy lightpush send rejected as RLN-invalid; " &
     "refreshing merkle proof and retrying once"
   rln.get().groupManager.invalidateMerkleProofCache()
@@ -146,9 +143,8 @@ proc legacyLightpushPublish*(
     error "failed to publish message as legacy lightpush not available"
     return err("Waku lightpush not available")
 
-  # toRLNSignal includes the timestamp in the proof input, so it must be fixed
-  # before proof generation. The downstream ensureTimestampSet in the client
-  # publish becomes an idempotent no-op safety net.
+  # toRLNSignal hashes the timestamp into the proof, so fix it before proof gen;
+  # the downstream ensureTimestampSet then becomes a no-op.
   let message = ensureTimestampSet(message)
 
   let rln =
@@ -168,9 +164,8 @@ proc legacyLightpushPublish*(
     let firstResult =
       await internalLegacyLightpushPublish(node, pubsubForPublish, msgWithProof, peer)
 
-    # A publish error mentioning RLN can indicate a stale merkle proof path;
-    # refresh it and retry the publish once. Legacy lightpush has no status
-    # codes, so we string-match the RLN error for backward compatibility.
+    # Legacy has no status codes, so string-match the RLN error to detect a
+    # stale merkle proof path, then refresh and retry once.
     if firstResult.isOk() or rln.isNone() or
         not firstResult.error.contains(RlnValidatorErrorMsg):
       return firstResult
@@ -329,9 +324,8 @@ proc lightpushPublish*(
       error "lightpush publish error", error = msg
       return lighpushErrorResult(LightPushErrorCode.INTERNAL_SERVER_ERROR, msg)
 
-  # toRLNSignal includes the timestamp in the proof input, so the timestamp
-  # must be fixed before proof generation. The downstream ensureTimestampSet
-  # in the client publish becomes an idempotent no-op safety net.
+  # toRLNSignal hashes the timestamp into the proof, so fix it before proof gen;
+  # the downstream ensureTimestampSet then becomes a no-op.
   let message = ensureTimestampSet(message)
 
   let rln =
@@ -345,11 +339,9 @@ proc lightpushPublish*(
   let firstResult =
     await lightpushPublishHandler(node, pubsubForPublish, msgWithProof, toPeer, mixify)
 
-  # A publish rejection can indicate a stale Merkle proof path. Gate only on
-  # unambiguously RLN-related failures: 504 (OUT_OF_RLN_PROOF) is always
-  # RLN-specific; 420 (INVALID_MESSAGE) is also returned for non-RLN
-  # rejections (e.g. oversized messages), so require the error description to
-  # contain RlnValidatorErrorMsg.
+  # Gate the refresh on unambiguously RLN-related failures: 504
+  # (OUT_OF_RLN_PROOF) is always RLN; 420 (INVALID_MESSAGE) also covers non-RLN
+  # rejections (e.g. oversized), so additionally require RlnValidatorErrorMsg.
   if firstResult.isOk() or rln.isNone():
     return firstResult
   let isRlnRelatedFailure =
@@ -360,11 +352,9 @@ proc lightpushPublish*(
   if not isRlnRelatedFailure:
     return firstResult
 
-  # Schedule a merkle proof refresh and surface the rejection immediately,
-  # normalized to 504 (OUT_OF_RLN_PROOF) with RlnProofRefreshScheduledMsg so
-  # callers can tell "stale proof, retry the publish" from a permanent
-  # rejection. A retried publish regenerates its proof against the refreshed
-  # cache, or coalesces onto the still-running refetch.
+  # Schedule the refresh and return immediately, normalized to 504 with
+  # RlnProofRefreshScheduledMsg so callers can tell "stale proof, retry" from a
+  # permanent rejection. A retry regenerates against the refreshed cache.
   info "lightpush send rejected as RLN-invalid; scheduling merkle proof refresh",
     statusCode = $firstResult.error.code
   rln.get().groupManager.scheduleMerkleProofRefresh()

logos_delivery/waku/rln/constants.nim

@@ -20,18 +20,14 @@ const RlnCredentialsFilename* = "rlnCredentials.txt"
 # RLN Validator message rejection Error string, is used to trigger proof refresh and publish retry in the lightpush client
 const RlnValidatorErrorMsg* = "RLN validation failed"
 
-# Returned as the OUT_OF_RLN_PROOF error description when a stale merkle proof
-# is suspected and a background refresh has been scheduled. Signals to callers
-# that retrying the publish is worthwhile: the retry regenerates the proof
-# against the refreshed cache.
+# OUT_OF_RLN_PROOF description marker telling callers a background refresh was
+# scheduled and retrying the publish is worthwhile.
 const RlnProofRefreshScheduledMsg* =
   "stale RLN proof suspected; refresh scheduled, retry the publish"
 
-# Upper bound on a reactive merkle proof refresh after a stale-RLN
-# rejection: fetching a fresh merkle path from the RLN contract (eth_call
-# round-trip) plus proof regeneration. Without a bound a hanging RPC endpoint
-# could stall the caller indefinitely. Only the refresh is covered — a
-# subsequent publish retry runs unbounded, matching the first attempt.
+# Bounds the legacy lightpush merkle proof refresh (eth_call refetch + proof
+# regen) so a hanging RPC cannot stall the caller. The retried publish is not
+# bounded.
 const RlnMerkleProofRefreshTimeout* = 5.seconds
 
 # inputs of the membership contract constructor

logos_delivery/waku/rln/group_manager/group_manager_base.nim

@@ -142,18 +142,13 @@ method generateProof*(
   return err("generateProof is not implemented")
 
 method invalidateMerkleProofCache*(g: GroupManager) {.base, gcsafe, raises: [].} =
-  ## Drops the cached merkle proof path. Callers invoke this when a publish is
-  ## rejected on a stale membership cache (e.g. a lightpush 420 INVALID_MESSAGE
-  ## or 504 OUT_OF_RLN_PROOF reply); the next proof-gen then sees an empty
-  ## cache and refetches from chain. Base implementation is a no-op.
+  ## Drops the cached merkle proof path so the next proof-gen refetches from
+  ## chain. Called after a publish is rejected on a stale cache. No-op base.
   discard
 
 method scheduleMerkleProofRefresh*(g: GroupManager) {.base, gcsafe, raises: [].} =
-  ## Drops the cached merkle proof path and starts a detached background
-  ## refetch. Callers invoke this when a publish is rejected on a stale
-  ## membership cache but must not wait for the refetch themselves: the next
-  ## proof-gen either finds the refreshed cache or coalesces onto the
-  ## in-flight refetch. Base implementation is a no-op.
+  ## Like `invalidateMerkleProofCache`, but starts the refetch in the
+  ## background so the caller need not wait for it. No-op base.
   discard
 
 method isReady*(g: GroupManager): Future[bool] {.base, async.} =

logos_delivery/waku/rln/group_manager/on_chain/group_manager.nim

@@ -262,16 +262,9 @@ method validateRoot*(g: OnchainGroupManager, root: MerkleNode): Future[bool] {.a
 proc ensureFreshMerkleProofPath*(
     g: OnchainGroupManager
 ): Future[Result[void, string]] {.async.} =
-  ## Keeps `merkleProofCache` fresh. Refetches only when the cache is empty.
-  ## Callers that suspect the cached path is stale (e.g. after a lightpush
-  ## 420 INVALID_MESSAGE or 504 OUT_OF_RLN_PROOF rejection) must first invoke
-  ## `invalidateMerkleProofCache`, so the next call here sees an empty cache
-  ## and refetches from chain. Guards against a missing membership index
-  ## because `fetchMerkleProofElements` unwraps it.
-  ## The in-flight refetch is awaited via `join` so that cancelling one
-  ## caller (e.g. a timed-out publish retry) never cancels the shared
-  ## refetch: concurrent callers coalesced onto it keep waiting, and the
-  ## refetch runs to completion so the cache is populated for the next call.
+  ## Refetches `merkleProofCache` only when empty; suspected-stale callers
+  ## invalidate first. Concurrent callers coalesce onto one refetch, awaited
+  ## via `join` so cancelling one caller never cancels the shared refetch.
   if g.membershipIndex.isNone():
     return err("membership index is not set")
 
@@ -303,17 +296,12 @@ proc ensureFreshMerkleProofPath*(
   return ok()
 
 method invalidateMerkleProofCache*(g: OnchainGroupManager) {.gcsafe, raises: [].} =
-  ## Drops the cached merkle proof path. The next `ensureFreshMerkleProofPath`
-  ## then sees an empty cache and refetches from chain. Callers invoke this
-  ## when a publish is rejected on a stale membership cache (e.g. a lightpush
-  ## 420 INVALID_MESSAGE or 504 OUT_OF_RLN_PROOF reply).
+  ## Empties the cache so the next `ensureFreshMerkleProofPath` refetches.
   g.merkleProofCache = @[]
 
 method scheduleMerkleProofRefresh*(g: OnchainGroupManager) {.gcsafe, raises: [].} =
-  ## Drops the cached merkle proof path and starts a detached refetch so the
-  ## cache is warm again by the time a caller retries the publish. A refetch
-  ## failure is logged and left for the next `ensureFreshMerkleProofPath` call
-  ## to repair.
+  ## Empties the cache and spawns a detached refetch; a failed refetch is
+  ## logged and repaired by the next `ensureFreshMerkleProofPath`.
   g.merkleProofCache = @[]
 
   proc refresh() {.async.} =

logos_delivery/waku/rln/proof.nim

@@ -67,12 +67,9 @@ proc generateRLNProof*(
 proc generateRLNProofWithRootRefresh*(
     rln: Rln, input: seq[byte], senderEpochTime: float64
 ): Future[Result[seq[byte], string]] {.async.} =
-  ## Generates an RLN proof and self-validates its merkle root against the
-  ## acceptable-root window. If the cached path has slid out of that window
-  ## (typical after a long inactivity or on-chain group churn), invalidates
-  ## the local cache and regenerates the proof once — the next proof-gen
-  ## sees an empty cache and refetches from chain. Returns the proof bytes
-  ## the caller can attach to the message.
+  ## Generates an RLN proof and checks its merkle root against the
+  ## acceptable-root window. If the root is stale, invalidates the cache and
+  ## regenerates once against a refetched path. Returns the proof bytes.
   let proofBytes = (await rln.generateRLNProof(input, senderEpochTime)).valueOr:
     return err("failed to generate RLN proof: " & $error)
 
@@ -89,11 +86,9 @@ proc generateRLNProofWithRootRefresh*(
 proc attachRLNProof*(
     r: Rln, message: WakuMessage
 ): Future[Result[WakuMessage, string]] {.async.} =
-  ## 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.
+  ## Returns the message with a freshly generated RLN proof, replacing any
+  ## existing one and drawing a new message id. Retry paths suspecting a stale
+  ## path should call `invalidateMerkleProofCache` first.
   var msgWithProof = message
   msgWithProof.proof = (
     await r.generateRLNProof(message.toRLNSignal(), float64(getTime().toUnix()))
@@ -104,9 +99,8 @@ proc attachRLNProof*(
 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.
+  ## Returns the message with an attached RLN proof, or unchanged when it
+  ## already carries a proof or RLN is not configured.
   if message.proof.len > 0:
     return ok(message)