From 079377a4faa8c096a25123a055bba6a8a6cdfb1d Mon Sep 17 00:00:00 2001 From: stubbsta Date: Fri, 10 Jul 2026 14:21:34 +0200 Subject: [PATCH] Split checkAndGenerateRLNProof: extract attachRLNProof MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../waku/node/waku_node/lightpush.nim | 14 +++--- logos_delivery/waku/rln/proof.nim | 44 +++++++++---------- tests/node/test_wakunode_lightpush.nim | 8 ++-- 3 files changed, 31 insertions(+), 35 deletions(-) diff --git a/logos_delivery/waku/node/waku_node/lightpush.nim b/logos_delivery/waku/node/waku_node/lightpush.nim index 00e626e35..cfaade900 100644 --- a/logos_delivery/waku/node/waku_node/lightpush.nim +++ b/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) diff --git a/logos_delivery/waku/rln/proof.nim b/logos_delivery/waku/rln/proof.nim index 2f956e043..7c777d12a 100644 --- a/logos_delivery/waku/rln/proof.nim +++ b/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) diff --git a/tests/node/test_wakunode_lightpush.nim b/tests/node/test_wakunode_lightpush.nim index 6766d8eca..cc8ea255d 100644 --- a/tests/node/test_wakunode_lightpush.nim +++ b/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