From e73cf4729a7c4c3f3cb1733bc773b3ba6752340c Mon Sep 17 00:00:00 2001 From: stubbsta Date: Sun, 12 Jul 2026 17:35:57 +0200 Subject: [PATCH] Shield the shared merkle path refetch from caller cancellation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ensureFreshMerkleProofPath coalesces concurrent callers onto a single in-flight refetch future and awaited it directly. Chronos propagates cancellation into the awaited future, so one caller being cancelled (e.g. a publish retry whose RlnRefreshRetryTimeout fired mid-refetch) cancelled the shared future under every coalesced caller — and threw away an eth_call that may have been about to complete. Await the in-flight future via join() instead: cancelling a caller only detaches its callback. The refetch always runs to completion, coalesced callers keep waiting, and even when every caller times out the cache still gets populated in the background so the next publish fast-paths instead of paying the RPC round-trip again. Detaching is safe because doRefresh cannot fail with an unhandled exception: the eth_call wrappers catch CatchableError and return Result, and failures surface to callers through fetchOk / an empty cache exactly as before. Adds a test cancelling the initiating caller mid-refetch and asserting the shared future survives and the coalesced caller completes with a populated cache. Co-Authored-By: Claude Fable 5 --- .../group_manager/on_chain/group_manager.nim | 8 +++-- .../test_rln_group_manager_onchain.nim | 35 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) 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 9b9f147e4..a746068f2 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 @@ -268,6 +268,10 @@ proc ensureFreshMerkleProofPath*( ## `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. if g.membershipIndex.isNone(): return err("membership index is not set") @@ -276,7 +280,7 @@ proc ensureFreshMerkleProofPath*( if not g.proofPathRefreshInFlightFut.isNil() and not g.proofPathRefreshInFlightFut.finished(): - await g.proofPathRefreshInFlightFut + await g.proofPathRefreshInFlightFut.join() if g.merkleProofCache.len > 0: return ok() return err("merkle proof path refresh failed") @@ -292,7 +296,7 @@ proc ensureFreshMerkleProofPath*( discard await g.updateMemberCount() g.proofPathRefreshInFlightFut = doRefresh() - await g.proofPathRefreshInFlightFut + await g.proofPathRefreshInFlightFut.join() if not fetchOk: return err("merkle proof path refresh failed") 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 b5a6684e2..c3e38262b 100644 --- a/tests/waku_rln_relay/test_rln_group_manager_onchain.nim +++ b/tests/waku_rln_relay/test_rln_group_manager_onchain.nim @@ -583,6 +583,41 @@ suite "Onchain group manager": manager.proofPathRefreshInFlightFut == inFlight manager.merkleProofCache.len > 0 + test "ensureFreshMerkleProofPath: cancelling one caller does not cancel the shared refetch": + (waitFor manager.init()).isOkOr: + raiseAssert $error + + let credentials = generateCredentials() + (waitFor manager.register(credentials, UserMessageLimit(20))).isOkOr: + assert false, "register failed: " & error + + manager.merkleProofCache = @[] + manager.proofPathRefreshInFlightFut = nil + + let f1 = manager.ensureFreshMerkleProofPath() + let inFlight = manager.proofPathRefreshInFlightFut + let f2 = manager.ensureFreshMerkleProofPath() + + check: + inFlight != nil + not inFlight.finished() + + # Cancel the initiating caller — models a publish retry whose refresh + # timeout fired while the refetch was still in flight. + waitFor f1.cancelAndWait() + + check: + f1.cancelled() + # The shared refetch survives the caller's cancellation. + not inFlight.cancelled() + + # The coalesced caller still completes and the cache gets populated. + let r2 = waitFor f2 + check: + r2.isOk() + manager.merkleProofCache.len > 0 + manager.proofPathRefreshInFlightFut == inFlight + test "verifyProof: should verify valid proof": let credentials = generateCredentials() (waitFor manager.init()).isOkOr: