diff --git a/logos_delivery/waku/node/waku_node/lightpush.nim b/logos_delivery/waku/node/waku_node/lightpush.nim index 878777f8e..56d39aae3 100644 --- a/logos_delivery/waku/node/waku_node/lightpush.nim +++ b/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() diff --git a/logos_delivery/waku/rln/constants.nim b/logos_delivery/waku/rln/constants.nim index eabc7da54..9f93d5fe0 100644 --- a/logos_delivery/waku/rln/constants.nim +++ b/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 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 c113e06ef..5f91cf19f 100644 --- a/logos_delivery/waku/rln/group_manager/group_manager_base.nim +++ b/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.} = 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 b28f894eb..0af1b8d25 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 @@ -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.} = diff --git a/logos_delivery/waku/rln/proof.nim b/logos_delivery/waku/rln/proof.nim index 7c777d12a..8a29eb67b 100644 --- a/logos_delivery/waku/rln/proof.nim +++ b/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)