chore(rln-relay): clean up nullifier table every MaxEpochGap #1994
Conversation
|
You can find the image built from this PR at |
|
You can find the experimental image built from this PR at |
rymnc
force-pushed
the
clear-nullifier-table
branch
from
e678c5c
to
8aea4fe
Compare
| @@ -1,23 +1,18 @@ | |||
| {.used.} | |||
|
|
|||
| import | |||
| std/[sequtils, tempfiles], | |||
| std/[tempfiles, sequtils], | |||
There was a problem hiding this comment.
isn't s before t in the alphabet?:D
There was a problem hiding this comment.
err, my bad - that change shouldn't be in the diff
rymnc
force-pushed
the
clear-nullifier-table
branch
from
8aea4fe
to
3beaf37
Compare
There was a problem hiding this comment.
lgtm thanks! left a comment in case you find it relevant.
side questions:
- so as it was before, we should have a memory leak? if the table was not being emptied, that should grow to infinity? just asking because i didnt saw the leak in the simulations.
- what would be the napkin math on the worst case size of this table? I assume
seq[ProofMetadata]can be as long as the max amount of memberships, since we would only store 1 proof per epoch per membership (if it sent a message). And then if we allow a diff between clocks of 20 seconds and 1 message every 1 second MaxEpochGap=20/1. Having 10k memberhips, is it fair to assume that we can have 10.000 * 20 entries at worse?
| @@ -300,13 +299,26 @@ proc appendRLNProof*(rlnPeer: WakuRLNRelay, | |||
| msg.proof = proofGenRes.get().encode().buffer | |||
| return true | |||
|
|
|||
| proc clearNullifierLog(rlnPeer: WakuRlnRelay) = | |||
There was a problem hiding this comment.
just to understand it. what we really want to keep is the seq[ProofMetadata] of the epochs that are no older than MaxEpochGap right?
I mean, in your case you keep MaxEpochGap at max, no matter if they meet this condition. But we would be interested in:
for epoch in rlnPeer.nullifierLog.keys():
if (epoch + MaxEpochGap) < now_epoch:
rlnPeer.nullufierLog.del(epoch)this doesn't require an OrderedTable (unsure if ordered tables impact performance).
not a blocker
There was a problem hiding this comment.
Reading the RFC for the external nullifiers which are stored in the nullifierLog I wonder if it's the case that they're a hash of an epoch, and not an epoch per se, so maybe that's why the approach from this PR was chosen instead, and the comparison against the current epoch is not possible
depends on how long were your simulations - but yes, it would've grown to infinity |
Description
This PR clears the nullifier table every time there are more than
MaxEpochGapepochs stored in itChanges
Issue
Addresses a part of #1906