From 8ada4e06f1943584c864f0464c53f39d96b830ed Mon Sep 17 00:00:00 2001
From: Aaryamann Challani <43716372+rymnc@users.noreply.github.com>
Date: Tue, 30 Apr 2024 18:52:47 +0530
Subject: [PATCH] fix(rln-relay): persist metadata every batch during initial
 sync (#2649)

* fix(rln-relay): persist metadata every batch during initial sync

* fix: test

* Apply suggestions from code review

Co-authored-by: Ivan FB <128452529+Ivansete-status@users.noreply.github.com>

* patch: isOkOr template

---------

Co-authored-by: Ivan FB <128452529+Ivansete-status@users.noreply.github.com>
---
 .../test_rln_group_manager_onchain.nim        | 10 +++---
 .../group_manager/on_chain/group_manager.nim  | 33 ++++++++++---------
 2 files changed, 22 insertions(+), 21 deletions(-)

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 3d95a4e40..bbebb556c 100644
--- a/tests/waku_rln_relay/test_rln_group_manager_onchain.nim
+++ b/tests/waku_rln_relay/test_rln_group_manager_onchain.nim
@@ -328,7 +328,10 @@ suite "Onchain group manager":
     let merkleRootAfter = manager.rlnInstance.getMerkleRoot().valueOr:
       raiseAssert $error
 
+    let metadataOpt = manager.rlnInstance.getMetadata().valueOr:
+      raiseAssert $error
     check:
+      metadataOpt.get().validRoots == manager.validRoots.toSeq()
       merkleRootBefore != merkleRootAfter
     await manager.stop()
 
@@ -481,13 +484,8 @@ suite "Onchain group manager":
     except Exception, CatchableError:
       assert false, "exception raised: " & getCurrentExceptionMsg()
 
-    await fut
+    check await fut.withTimeout(5.seconds)
 
-    let metadataOpt = manager.rlnInstance.getMetadata().valueOr:
-      raiseAssert $error
-    assert metadataOpt.isSome(), "metadata is not set"
-    check:
-      metadataOpt.get().validRoots == manager.validRoots.toSeq()
     await manager.stop()
 
   asyncTest "withdraw: should guard against uninitialized state":
diff --git a/waku/waku_rln_relay/group_manager/on_chain/group_manager.nim b/waku/waku_rln_relay/group_manager/on_chain/group_manager.nim
index dad3da928..458f5ec09 100644
--- a/waku/waku_rln_relay/group_manager/on_chain/group_manager.nim
+++ b/waku/waku_rln_relay/group_manager/on_chain/group_manager.nim
@@ -134,11 +134,18 @@ template retryWrapper(
   retryWrapper(res, RetryStrategy.new(), errStr, g.onFatalErrorAction):
     body
 
-proc setMetadata*(g: OnchainGroupManager): RlnRelayResult[void] =
+proc setMetadata*(
+    g: OnchainGroupManager, lastProcessedBlock = none(BlockNumber)
+): GroupManagerResult[void] =
+  let normalizedBlock =
+    if lastProcessedBlock.isSome():
+      lastProcessedBlock.get()
+    else:
+      g.latestProcessedBlock
   try:
     let metadataSetRes = g.rlnInstance.setMetadata(
       RlnMetadata(
-        lastProcessedBlock: g.latestProcessedBlock,
+        lastProcessedBlock: normalizedBlock,
         chainId: uint64(g.chainId.get()),
         contractAddress: g.ethContractAddress,
         validRoots: g.validRoots.toSeq(),
@@ -183,9 +190,6 @@ when defined(rln_v2):
       await g.registerCb.get()(membersSeq)
 
     g.validRootBuffer = g.slideRootQueue()
-    let setMetadataRes = g.setMetadata()
-    if setMetadataRes.isErr():
-      error "failed to persist rln metadata", error = setMetadataRes.error
 
 else:
   method atomicBatch*(
@@ -214,9 +218,6 @@ else:
       await g.registerCb.get()(membersSeq)
 
     g.validRootBuffer = g.slideRootQueue()
-    let setMetadataRes = g.setMetadata()
-    if setMetadataRes.isErr():
-      error "failed to persist rln metadata", error = setMetadataRes.error
 
 when defined(rln_v2):
   method register*(
@@ -550,13 +551,6 @@ proc getAndHandleEvents(
     raise newException(ValueError, "failed to handle events")
 
   g.latestProcessedBlock = toBlock
-  let metadataSetRes = g.setMetadata()
-  if metadataSetRes.isErr():
-    # this is not a fatal error, hence we don't raise an exception
-    warn "failed to persist rln metadata", error = metadataSetRes.error()
-  else:
-    trace "rln metadata persisted", blockNumber = g.latestProcessedBlock
-
   return true
 
 proc runInInterval(g: OnchainGroupManager, cb: proc, interval: Duration) =
@@ -591,6 +585,13 @@ proc getNewBlockCallback(g: OnchainGroupManager): proc =
     var handleBlockRes: bool
     g.retryWrapper(handleBlockRes, "Failed to handle new block"):
       await g.getAndHandleEvents(fromBlock, latestBlock)
+
+    # cannot use isOkOr here because results in a compile-time error that 
+    # shows the error is void for some reason
+    let setMetadataRes = g.setMetadata()
+    if setMetadataRes.isErr():
+      error "failed to persist rln metadata", error = setMetadataRes.error
+
     return handleBlockRes
 
   return wrappedCb
@@ -664,6 +665,8 @@ proc startOnchainSync(
       futs.add(g.getAndHandleEvents(fromBlock, toBlock))
       if futs.len >= maxFutures or toBlock == currentLatestBlock:
         await g.batchAwaitBlockHandlingFuture(futs)
+        g.setMetadata(lastProcessedBlock = some(toBlock)).isOkOr:
+          error "failed to persist rln metadata", error = $error
         futs = newSeq[Future[bool]]()
       fromBlock = toBlock + 1
   except CatchableError: