waku-org / nwaku

Waku node and protocol.

Other

202 stars 53 forks source link

Enable Zerokit by default #1061

Closed oskarth closed 2 years ago

oskarth commented 2 years ago

Problem

We want to use Zerokit by default to have one code base to worry about and pave the way to cross-client testnet.

Notes

Before this can happen, we need the following:

[x] https://github.com/status-im/nwaku/pull/1030 or eqv merged. EDIT: merged https://github.com/status-im/nwaku/pull/1060
[ ] Testnet1 complete https://github.com/vacp2p/research/issues/126
[x] Implementation tested to ensure no major regressions found
[x] https://github.com/status-im/nwaku/issues/1227

Acceptance criteria

Zerokit enabled by default

We can either remove flag in PR that addresses this, or do this at a later stage (depends on confidence around perf etc)

cc @s1fr0 @staheri14 just to capture this future work and its dependencies.

s1fr0 commented 2 years ago

Integration will probably require revising/updating round constants of current smart contract Poseidon Hasher implementation so that result aligned with Circom zkSNARK Poseidon circuit.

@kgrgpg We might start working on a new rln-contract branch where the Circom constants are loaded in place of Kilic's ones and then check if the returned hashes correspond to the ones given by Circom implementation.

staheri14 commented 2 years ago

@s1fr0 it would be very helpful to have an issue in https://github.com/vacp2p/rln-contract covering updates that are needed to be done on the contract, and with pointers to the relevant repos and files e.g., where the Circom constants reside, etc. Thanks!

staheri14 commented 2 years ago

What is the state of this issue? I guess it needs some updates @s1fr0

staheri14 commented 2 years ago

If zero-kit gets enabled by default, it will help resolving this issue as well https://github.com/status-im/nwaku/issues/1213

s1fr0 commented 2 years ago

Zerokit is enabled by default since release v.0.12. Kilic's library is still supported and can be enabled by passing KILICRLN=true instead of RLN=true (which now enables, instead, zerokit RLN).

Not sure what the second item refers to, since https://github.com/vacp2p/research/issues/126 clearly states that zerokit integration is out of scope. Maybe a new testnet for nwaku with zerokit enabled? I would say that due to preparation of the live demo, all CC extensively tested (and bug-fixed) zerokit not only in nwaku but also in go-waku and js-waku so a preliminary testnet1 can be considered complete.

This issue was replaced by https://github.com/status-im/nwaku/issues/1227 and hence can be closed.