From 95d0a8bdd86bee46fa27c9449f6a401d552e5844 Mon Sep 17 00:00:00 2001 From: stubbsta Date: Thu, 9 Jul 2026 12:51:37 +0200 Subject: [PATCH] Replace forceMerkleProofRefresh flag with invalidateMerkleProofCache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The forceMerkleProofRefresh bool was threaded through five procs (checkAndGenerateRLNProof → generateRLNProof → generateProof base + on-chain → ensureFreshMerkleProofPath) purely so a caller could ask the merkle path to be refetched from chain on a stale-RLN retry. Reading any one of those signatures required a mental stack trace back up to the retry caller to know what "force" meant. Flip the contract: expose invalidateMerkleProofCache on the group manager; callers invoke it on the retry path to empty the cache, and the normal proof-gen flow then sees an empty cache and refetches on its own. No flag crosses procs. Removed forceMerkleProofRefresh from: - group_manager_base.generateProof (+ on_chain override) - ensureFreshMerkleProofPath (its `force` param) - generateRLNProof Renamed on checkAndGenerateRLNProof: forceMerkleProofRefresh → regenerate. It now controls one thing — bypassing the "message already has a proof" short-circuit — and never propagates downstream. Callers pair it with invalidateMerkleProofCache when they need the refetch semantics too. Retry callers updated in lockstep: generateRLNProofWithRootRefresh, REST attachRlnProofValidateWithRetry, and both legacy + modern lightpush retry paths now call groupManager.invalidateMerkleProofCache() before regenerating. The primitive test in test_wakunode_lightpush.nim now exercises the pair directly, and the onchain ensureFreshMerkleProofPath test is rewritten to exercise invalidate → ensureFresh as one flow (retiring its `force = true` variant). Pure refactor; behavior unchanged. Verified against: tests/waku_rln_relay/test_rln_group_manager_onchain.nim (29/29) tests/wakunode_rest/test_rest_relay.nim (13/13) tests/waku_rln_relay/test_wakunode_rln_relay.nim (5/5, 1 skip) tests/node/test_wakunode_lightpush.nim (11/11) tests/node/test_wakunode_legacy_lightpush.nim (9/9) Co-Authored-By: Claude Opus 4.7 --- .../waku/node/waku_node/lightpush.nim | 24 ++++-------- .../waku/rest_api/endpoint/relay/handlers.nim | 5 +-- .../rln/group_manager/group_manager_base.nim | 8 +++- .../group_manager/on_chain/group_manager.nim | 26 ++++++++----- logos_delivery/waku/rln/proof.nim | 37 ++++++++----------- tests/node/test_wakunode_lightpush.nim | 24 ++++++------ .../test_rln_group_manager_onchain.nim | 16 +++++--- 7 files changed, 69 insertions(+), 71 deletions(-) diff --git a/logos_delivery/waku/node/waku_node/lightpush.nim b/logos_delivery/waku/node/waku_node/lightpush.nim index c5d16500d..1e2159c62 100644 --- a/logos_delivery/waku/node/waku_node/lightpush.nim +++ b/logos_delivery/waku/node/waku_node/lightpush.nim @@ -70,10 +70,7 @@ proc mountLegacyLightPushClient*(node: WakuNode) = WakuLegacyLightPushClient.new(node.peerManager, node.rng) proc internalLegacyLightpushPublish( - node: WakuNode, - pubsubTopic: PubsubTopic, - message: WakuMessage, - peer: RemotePeerInfo, + node: WakuNode, pubsubTopic: PubsubTopic, message: WakuMessage, peer: RemotePeerInfo ): Future[legacy_lightpush_protocol.WakuLightPushResult[string]] {.async, gcsafe.} = ## Dispatches to the legacy lightpush client if mounted, otherwise to the ## self-hosted server. Callers guarantee at least one is mounted. @@ -105,7 +102,7 @@ proc resolveLegacyPubsubTopic( let parsedTopic = NsContentTopic.parse(contentTopic).valueOr: return err("Invalid content-topic: " & $error) let shard = node.wakuAutoSharding.get().getShard(parsedTopic).valueOr: - return err("Autosharding error: " & error) + return err("Autosharding error: " & error) return ok($shard) proc runRlnRefreshRetry( @@ -122,16 +119,14 @@ proc runRlnRefreshRetry( ## hanging RPC/libp2p round-trip cannot stall the caller indefinitely. info "legacy lightpush send rejected as RLN-invalid; " & "refreshing merkle proof and retrying once" + rln.get().groupManager.invalidateMerkleProofCache() proc runRetry(): Future[legacy_lightpush_protocol.WakuLightPushResult[string]] {. async, gcsafe .} = let (retryMsg, _) = ( await checkAndGenerateRLNProof( - rln, - msgWithProof, - forceMerkleProofRefresh = true, - reuseMessageId = drawnMessageId, + rln, msgWithProof, regenerate = true, reuseMessageId = drawnMessageId ) ).valueOr: return err("failed call checkAndGenerateRLNProof from lightpush retry: " & error) @@ -176,9 +171,8 @@ proc legacyLightpushPublish*( ).valueOr: return err(error) - let firstResult = await internalLegacyLightpushPublish( - node, pubsubForPublish, msgWithProof, peer - ) + 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 @@ -375,14 +369,12 @@ proc lightpushPublish*( info "lightpush send rejected; refreshing merkle proof and retrying once", statusCode = $firstResult.error.code + rln.get().groupManager.invalidateMerkleProofCache() proc runRetry(): Future[lightpush_protocol.WakuLightPushResult] {.async, gcsafe.} = let (retryMsg, _) = ( await checkAndGenerateRLNProof( - rln, - msgWithProof, - forceMerkleProofRefresh = true, - reuseMessageId = drawnMessageId, + rln, msgWithProof, regenerate = true, reuseMessageId = drawnMessageId ) ).valueOr: return lighpushErrorResult(LightPushErrorCode.OUT_OF_RLN_PROOF, error) diff --git a/logos_delivery/waku/rest_api/endpoint/relay/handlers.nim b/logos_delivery/waku/rest_api/endpoint/relay/handlers.nim index 2270e7f12..6832e8697 100644 --- a/logos_delivery/waku/rest_api/endpoint/relay/handlers.nim +++ b/logos_delivery/waku/rest_api/endpoint/relay/handlers.nim @@ -85,10 +85,9 @@ proc attachRlnProofValidateWithRetry( err(RlnPublishError(kind: ValidationRejected, desc: firstValidateResult.error)) info "relay publish rejected as RLN-invalid; refreshing merkle proof and retrying once" + rln.groupManager.invalidateMerkleProofCache() msg.proof = ( - await rln.generateRLNProof( - msg.toRLNSignal(), float64(getTime().toUnix()), forceMerkleProofRefresh = true - ) + await rln.generateRLNProof(msg.toRLNSignal(), float64(getTime().toUnix())) ).valueOr: return err( RlnPublishError( diff --git a/logos_delivery/waku/rln/group_manager/group_manager_base.nim b/logos_delivery/waku/rln/group_manager/group_manager_base.nim index 708e3252c..d52e8e2ef 100644 --- a/logos_delivery/waku/rln/group_manager/group_manager_base.nim +++ b/logos_delivery/waku/rln/group_manager/group_manager_base.nim @@ -137,10 +137,16 @@ method generateProof*( epoch: Epoch, messageId: MessageId, rlnIdentifier = DefaultRlnIdentifier, - forceMerkleProofRefresh: bool = false, ): Future[GroupManagerResult[RateLimitProof]] {.base, async.} = ## Dummy implementation for 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. + discard + method isReady*(g: GroupManager): Future[bool] {.base, async.} = return true diff --git a/logos_delivery/waku/rln/group_manager/on_chain/group_manager.nim b/logos_delivery/waku/rln/group_manager/on_chain/group_manager.nim index 3290e9b1a..80915f6d8 100644 --- a/logos_delivery/waku/rln/group_manager/on_chain/group_manager.nim +++ b/logos_delivery/waku/rln/group_manager/on_chain/group_manager.nim @@ -260,18 +260,18 @@ method validateRoot*(g: OnchainGroupManager, root: MerkleNode): Future[bool] {.a return g.indexOfRoot(root) >= 0 proc ensureFreshMerkleProofPath*( - g: OnchainGroupManager, force: bool = false + g: OnchainGroupManager ): Future[Result[void, string]] {.async.} = - ## Keeps `merkleProofCache` fresh. Refetches only when the cache is empty - ## or when the caller explicitly forces a refresh — typically in response - ## to a lightpush 420 (INVALID_MESSAGE) or 504 (OUT_OF_RLN_PROOF) reply - ## that suggests the cached path is stale. Otherwise trusts the cache. - ## Guards against a missing membership index because `fetchMerkleProofElements` - ## unwraps it. + ## 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. if g.membershipIndex.isNone(): return err("membership index is not set") - if g.merkleProofCache.len > 0 and not force: + if g.merkleProofCache.len > 0: return ok() if not g.proofPathRefreshInFlightFut.isNil() and @@ -298,6 +298,13 @@ proc ensureFreshMerkleProofPath*( return err("merkle proof path refresh failed") 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). + g.merkleProofCache = @[] + method register*( g: OnchainGroupManager, rateCommitment: RateCommitment ): Future[Result[void, string]] {.async.} = @@ -472,7 +479,6 @@ method generateProof*( epoch: Epoch, messageId: MessageId, rlnIdentifier = DefaultRlnIdentifier, - forceMerkleProofRefresh: bool = false, ): Future[GroupManagerResult[RateLimitProof]] {.async.} = ## Generates an RLN proof using the cached Merkle proof and custom witness # Ensure identity credentials and membership index are set @@ -484,7 +490,7 @@ method generateProof*( return err("user message limit is not set") debug "Generating RLN proof" - ?(await g.ensureFreshMerkleProofPath(force = forceMerkleProofRefresh)) + ?(await g.ensureFreshMerkleProofPath()) if (g.merkleProofCache.len mod 32) != 0: return err("Invalid merkle proof cache length") diff --git a/logos_delivery/waku/rln/proof.nim b/logos_delivery/waku/rln/proof.nim index 10a5fff6d..8a79a1d2d 100644 --- a/logos_delivery/waku/rln/proof.nim +++ b/logos_delivery/waku/rln/proof.nim @@ -55,19 +55,12 @@ proc toRLNSignal*(wakumessage: WakuMessage): seq[byte] = return output proc generateRLNProof*( - rln: Rln, - input: seq[byte], - senderEpochTime: float64, - forceMerkleProofRefresh: bool = false, + rln: Rln, input: seq[byte], senderEpochTime: float64 ): Future[Result[seq[byte], string]] {.async.} = let epoch = rln.calcEpoch(senderEpochTime) let nonce = rln.nonceManager.getNonce().valueOr: return err("could not get new message id to generate an rln proof: " & $error) - let proof = ( - await rln.groupManager.generateProof( - input, epoch, nonce, forceMerkleProofRefresh = forceMerkleProofRefresh - ) - ).valueOr: + let proof = (await rln.groupManager.generateProof(input, epoch, nonce)).valueOr: return err("could not generate rln-v2 proof: " & $error) return ok(proof.encode().buffer) @@ -76,8 +69,9 @@ proc generateRLNProofWithRootRefresh*( ): 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), force-refreshes - ## the merkle path and regenerates the proof once. Returns the proof bytes + ## (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. let proofBytes = (await rln.generateRLNProof(input, senderEpochTime)).valueOr: return err(error) @@ -88,14 +82,14 @@ proc generateRLNProofWithRootRefresh*( if await rln.groupManager.validateRoot(rlnProof.merkleRoot): return ok(proofBytes) - info "RLN: stale merkle root detected; force-refreshing merkle path" - return - await rln.generateRLNProof(input, senderEpochTime, forceMerkleProofRefresh = true) + info "RLN: stale merkle root detected; refreshing merkle path and regenerating proof" + rln.groupManager.invalidateMerkleProofCache() + return await rln.generateRLNProof(input, senderEpochTime) proc checkAndGenerateRLNProof*( rln: Option[Rln], message: WakuMessage, - forceMerkleProofRefresh: bool = false, + 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 @@ -103,7 +97,11 @@ proc checkAndGenerateRLNProof*( ## 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. - if message.proof.len > 0 and not forceMerkleProofRefresh: + ## 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))) if rln.isNone(): @@ -126,12 +124,7 @@ proc checkAndGenerateRLNProof*( 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, - forceMerkleProofRefresh = forceMerkleProofRefresh, - ) + await r.groupManager.generateProof(msgWithProof.toRLNSignal(), epoch, nonce) ).valueOr: return err("error in checkAndGenerateRLNProof: " & $error) msgWithProof.proof = proof.encode().buffer diff --git a/tests/node/test_wakunode_lightpush.nim b/tests/node/test_wakunode_lightpush.nim index fa42be6ac..57228f9fa 100644 --- a/tests/node/test_wakunode_lightpush.nim +++ b/tests/node/test_wakunode_lightpush.nim @@ -177,12 +177,12 @@ suite "RLN Proofs as a Lightpush Service": assert publishResponse.isErr(), "We expect an error response" check publishResponse.error.code == LightPushErrorCode.NO_PEERS_TO_RELAY - asyncTest "force refresh regenerates proof and refetches merkle path": - # Exercises the primitive that lightpushPublish leans on after a 420 - # (INVALID_MESSAGE) or 504 (OUT_OF_RLN_PROOF) rejection: an already - # attached proof must NOT short-circuit checkAndGenerateRLNProof when - # forceMerkleProofRefresh=true, and the cache must be refetched from - # chain instead of trusted. + asyncTest "invalidate + regenerate refetches merkle path and rebuilds proof": + # 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. let (firstMsg, drawnNonce) = (await checkAndGenerateRLNProof(some(server.rln), message)).get() check firstMsg.proof.len > 0 @@ -194,15 +194,13 @@ suite "RLN Proofs as a Lightpush Service": manager.merkleProofCache = newSeq[byte](goodCache.len) check manager.merkleProofCache != goodCache - # Force-regenerate. The existing proof must be discarded, the cache - # refetched from chain, and a fresh proof produced. Pass drawnNonce so - # the CAS rollback reclaims it — mirroring the production retry path. + # 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. + manager.invalidateMerkleProofCache() let (secondMsg, _) = ( await checkAndGenerateRLNProof( - some(server.rln), - firstMsg, - forceMerkleProofRefresh = true, - reuseMessageId = drawnNonce, + some(server.rln), firstMsg, regenerate = true, reuseMessageId = drawnNonce ) ).get() diff --git a/tests/waku_rln_relay/test_rln_group_manager_onchain.nim b/tests/waku_rln_relay/test_rln_group_manager_onchain.nim index a51efa353..b5a6684e2 100644 --- a/tests/waku_rln_relay/test_rln_group_manager_onchain.nim +++ b/tests/waku_rln_relay/test_rln_group_manager_onchain.nim @@ -440,8 +440,9 @@ suite "Onchain group manager": (waitFor manager.register(credentials, UserMessageLimit(20))).isOkOr: assert false, "register failed: " & error - # Prime cache. With forceMerkleProofRefresh=false (the default), a non-empty - # cache short-circuits ensureFreshMerkleProofPath. + # Prime cache. A non-empty cache short-circuits ensureFreshMerkleProofPath; + # the retry path invalidates the cache via invalidateMerkleProofCache before + # calling proof-gen again. manager.merkleProofCache = (waitFor manager.fetchMerkleProofElements()).valueOr: raiseAssert "failed to fetch initial path: " & error manager.proofPathRefreshInFlightFut = nil @@ -494,7 +495,7 @@ suite "Onchain group manager": res.isErr() res.error == "membership index is not set" - test "ensureFreshMerkleProofPath: refetches when caller forces a refresh": + test "invalidateMerkleProofCache + ensureFreshMerkleProofPath refetches": (waitFor manager.init()).isOkOr: raiseAssert $error @@ -502,13 +503,16 @@ suite "Onchain group manager": (waitFor manager.register(credentials, UserMessageLimit(20))).isOkOr: assert false, "register failed: " & error - # Prime cache with a non-empty value. force=true must bypass the - # "cache is populated" fast-path and exercise the refetch branch. + # Prime cache with a non-empty value, then take the retry path: + # invalidate empties the cache so ensureFreshMerkleProofPath refetches. manager.merkleProofCache = (waitFor manager.fetchMerkleProofElements()).valueOr: raiseAssert "failed to prime path: " & error manager.proofPathRefreshInFlightFut = nil - let res = waitFor manager.ensureFreshMerkleProofPath(force = true) + manager.invalidateMerkleProofCache() + check manager.merkleProofCache.len == 0 + + let res = waitFor manager.ensureFreshMerkleProofPath() check: res.isOk()