fix(rln-relay): invalid start index being set results in invalid proofs #1915
Conversation
|
cc: @alrevuelta @richard-ramos thanks for the report! |
|
You can find the image built from this PR at |
| @@ -89,7 +89,7 @@ proc createRLNInstanceLocal(d = MerkleTreeDepth, | |||
| cache_capacity: 15_000, | |||
| mode: "high_throughput", | |||
| compression: false, | |||
| flush_interval: 12_000, | |||
| flush_interval: 500, | |||
There was a problem hiding this comment.
What is the flush_interval attribute used for?
There was a problem hiding this comment.
This is describes how often we flush pmtree cache to disk. just experimenting with waku-simulator here
|
Can confirm that after testing this PR with |
| @@ -501,6 +504,8 @@ method init*(g: OnchainGroupManager): Future[void] {.async.} = | |||
| except CatchableError: | |||
| error "failed to restart group sync", error = getCurrentExceptionMsg() | |||
|
|
|||
| # Contract starts from the first index | |||
There was a problem hiding this comment.
Why are we calculating the startIndex manually (setting g.latestIndex to be 1 here, and then using that value in atomicBatch, instead of passing the index retrieved from the event logs?
I'm probably wrong, so please correct me if that's the case, but I think this might be problematic in case of a partial sync or if a lot of time has passed since the last sync, since in getAndHandleEvents the function setMetadata is called for each block chunk, and if for some reason we don't sync the full chain, the next time we start nwaku, it will load the metadata to retrieve the latest block that was synced, and will call atomicOperation passing the index 1, instead of the index from the logs that were emitted in the blocks that are being processed, thus the leaves from the merkle tree will be overwritten instead of inserting new elements.
There was a problem hiding this comment.
Hi, thanks for raising this issue - fixed in 048cdc2
Description
This pr introduces a couple small changes for logging and fixes the startIndex starting from 0 instead of 1 like the contract
Changes