chore(rln-relay): Requirements to consider RLN ready (non experimental)

[rymnc]

Background

As discussed with @alrevuelta, following are the discussion points for rln-relay

Details

• 1. run rln-relay on all configured pubsub topics and content topics (passed in w/ --pubsub-topic) -
  • chore(rln): run rln in all relay pubsubtopics + remove cli flags #1917
• 2. separate out bandwidth constraint validator
  • feat(relay): ordered validator execution #1966
• chore(relay): ordered validator execution #1967
  1. WakuMessage decoding
  2. WakuRelay
  3. WakuRlnRelay
  • feat(relay): ordered validator execution #1966
• 4. trace for bandwidth info log
  • 1 The log
  • 2 Remove DEBUG from here and leave just TRACE (all 3 places)
  • chore(rln-relay): apt log levels for certain logs #1986 (comment)
• 5. freeze nwaku startup till rln-relay is ready (otherwise it would reject valid proofs)
  • fix(rln-relay): waitFor startup, otherwise valid proofs will be marked invalid #1920
• 6. review metrics (few too many one-time metrics, like startup time etc)
  • chore(rln): remove old and add new rln metric #1926
• 7. clear up the nullifier table (clear the table once in 10 epochs or so) (reported by @richard-ramos)
  • chore(rln-relay): clean up nullifier table every MaxEpochGap #1994
• 8. health check api (/health should denote if the nwaku node is ready for use)
• 9. Sync from the deployment block instead of from genesis. Related to discussions in fix(rln-relay): sync from a recent block number, not 0 #1859
  • fix(rln-relay): sync from deployed block number #1955
• 10. The rln feature should be available in the non-experimental context
• 11. Add metric to show the amout of registered memberships in the tree.
• 12. rename rlnkeystore application to waku-rln-relay, thanks @richard-ramos
  • chore(rln-relay): rename keystore application to waku-rln-relay #1924
• 13. remove registration capability from nwaku
  • fix(rln-relay): remove registration capability #1916
• 14. bulk registration script to generate keystores (maybe in go?)
  • feat: rln subcommands go-waku#654
  • feat(rln): init rln_keystore_generator #1925
  • chore(rln_keystore_generator): generate and persist credentials #1928
  • feat(rln_keystore_generator): wired to onchain group manager #1931
• 15. Add metric with the total registered memberships in the contract
  • chore(rln): remove old and add new rln metric #1926
• 16. "simulate" a message in the "publish message" rpc endpoint to detect if the message would be considered as spam. if so, return and error without publishing the message. like this we avoid unnecesary spam, and we error notifying that the message will never arrive due to rln rate limitation.
  • chore(api): validate rln message before sending (rest + rpc) #1968
• 17. s/flush_interval/flush_every_ms misconfiguration on rln config
  • fix(rln-relay): flush_interval incorrectly set #1933
• 18. bug(rln-relay): RLN DB should be aware of chain and contract address
  • bug(rln-relay): RLN DB should be aware of chain and contract address #1921
  • fix(rln-relay): RLN DB should be aware of chain and contract address #1932
• 19. bug: rln db disappears #1944
  • fix(rln-relay): bump zerokit to v0.3.2 #1951
  • fix(rln): use config.open instead of sled::open vacp2p/zerokit#209
• 20. bug(rln): "provided root does not belong to acceptable window of roots" #1952
  • fix(rln-relay): window of acceptable roots synced to rln metadata #1953
• 21. bug(rln-relay): non deterministic sorting applied to credentials, and contract  #1922
  • fix(rln-relay): modify keystore credentials logic #1956
• 22. add comments about rln contract functions
  • chore(rln-relay): confirm that the provided credential is correct using onchain query #1980
• 23. "If your keystore has just 1 key, dont require the rln-relay-membership-index", just load the existing one. Only require that flag when having a keystore with multiple keys"
  • chore(rln-relay): use the only key from keystore if only 1 exists #1984
• 24. check if the credential fetched from the keystore exists in the onchain membership set
  • chore(rln-relay): confirm that the provided credential is correct using onchain query #1980
• 25. Add a check isRlnInSync() function (or similar) that returns true if synced to the latest head or false if rln tree is not in sync with latest head. + use it in apis (so that we error if rln is not in sync)
  • chore(rln-relay): add isReady check #1989
• 26. rln tree db inspector - feat(rln_db_inspector): create rln_db_inspector tool #1999
• 27. bug(rln): problems syncing the rln tree, different roots #2002
• 28. bug(rln): Wrong rln registered memberships metric #2008
  • fix(rln-relay): waku_rln_number_registered_memberships metrics appropriately handled #2018

Acceptance criteria

• Discussion about the above points, will be split into appropriate issues as required.

[alrevuelta]

Adding also to 4. leaving this in only trace. Otherwise it is too verbose for debug and can really affect performance. Imagine 100 msg/second. There would be a line per message in DEBUG.

Adding:
4.2: Remove DEBUG from here and leave just TRACE.

[alrevuelta]

Adding: 9. Sync from the deployment block instead of from genesis. Related to discussions in here

[alrevuelta]

Adding 11. Add metric to show the amout of registered memberships in the tree.

[rymnc]

added 13 and 14 to cut scope in nwaku

[alrevuelta]

Added 15. Add metric with the total registered memberships in the contract

[alrevuelta]

Added 16. "simulate" a message in the "publish message" rpc endpoint to detect if the message would be considered as spam. if so, return and error without publishing the message. like this we avoid unnecesary spam, and we error notifying that the message will never arrive due to rln rate limitation.

[rymnc]

adding 17. flush_interval and 18. rln metadata upgrade

[rymnc]

adding 19. rln tree persistence bug and 20. window of roots persistence

[alrevuelta]

Renamed this issue to "Requirements to consider RLN ready (non experimental)". Note that this has becone an umbrela issue to track whats left to consider RLN ready and not experimental anymore.

[rymnc]

added 22 to add comments to contract declarations

[rymnc]

@alrevuelta re: 16, the publishing node does perform the validation before relaying it onto other peers - and if it is spam, it is dropped at the source
does that work?

[alrevuelta]

the publishing node does perform the validation before relaying it onto other peers - and if it is spam, it is dropped at the source
does that work?

@rymnc Not sure what you mean. I meant something like this #1968

[alrevuelta]

Added 23. "If your keystore has just 1 key, dont require the rln-relay-membership-index", just load the existing one. Only require that flag when having a keystore with multiple keys"

[rymnc]

Please use the following contract from this point onwards - 0x0A988fd9CA5BAebDf098b8A73621b2AaDa6492E8

[alrevuelta]

Added "25. Add a check isRlnInSync() function (or similar) that returns true if synced to the latest head or false if rln tree is not in sync with latest head. + use it in apis (so that we error if rln is not in sync)"

Requirement for 8.

[rymnc]

may I suggest that we move 2 => to another issue since it is not directly related to the feature set that rln-relay provides? it would more likely be a part of the refactor that @Ivansete-status mentioned in #1966 (review)

[alrevuelta]

may I suggest that we move 2 => to another issue since it is not directly related to the feature set that rln-relay provides? it would more likely be a part of the refactor that @Ivansete-status mentioned in #1966 (review)

yep, lets track 2 separately. the important thing was ordered validators, and thats already covered.

[rymnc]

moved to #1992

[alrevuelta]

Added 27 and 28
#2002
#2008

[oskarth]

Hey guys, awesome work on RLN, really cool to see the all progress being made :)

I don't know exactly what litmus test you are using to distinguish production-ready/experimental etc, but just wanted to point out that Zerokit depends on ark-circom which depends on ark-groth16 https://github.com/arkworks-rs/groth16#ark-groth16, with the current status:

WARNING: This is an academic proof-of-concept prototype, and in particular has not received careful code review. This implementation is NOT ready for production use.

Probably not relevant at this stage for RLN, but good to keep an eye out from a security POV when it comes to real funds. A lot of people are relying on above library afaik (e.g. Worldcoin went to production with it, maybe others too). Also for your interest I believe PSE is investing in arkworks and would also be open to a security audit of a subset of arkworks libraries.

(This comment probably belong upstream in Zerokit repo but saw this linked from Twitter so I'll just drop it in here as a FYI).

[alrevuelta]

Weekly Update

• achieved: waku rln is not an experimental feature anymore, and is part of nwaku code base. from now on experimental features are hidden behind a flag and not in different build