-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(rln): update APIs after circuit update #84
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the quick PR :)
This PR fixes #82 |
@richard-ramos you ok to handle the query from @s1fr0 ? |
Does this imply any spec changes? If so this needs a corresponding issue in RFC repo. We need to have the RFC up to date and alive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above
Tracking here vacp2p/rfc#560 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not break any API but will require the following actions
In JS-Land:
- Edit the Cargo.toml of rln-wasm to increase the version number
- Publish a new version of @waku/zerokit-rln-wasm, with
cargo make publish
- Publish a new version of js-rln that uses the version of the package published in last step
In GO-Land:
- Update go-zerokit-rln so it uses latest zerokit version, and update all static libraries
- Update go-waku to use latest go-zerokit-rln
@richard-ramos, please can you check if the nightly wasm artifact here is suitable till we complete the steps you mentioned? Having a new release is preferred since we are breaking APIs. (for slashing/withdrawing) |
The corresponding RFC 32/RLN has been updated in vacp2p/rfc#561 to reflect the changes implemented by this PR. |
Please let me know if we're ok to merge @oskarth @rymnc @richard-ramos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving assuming we are tracking relevant changes in implementations too:
- Spec PR is done so that's good
- refactor(rln): update APIs after circuit update #84 (review) this should have issues in nwaku/go-waku/js-waku so we make sure to update all implementations, or at least capture that this has to be done - it'd be good if we created these issues before merging (easy to forget otherwise)
- who will update nwaku?
We're tracking the nwaku update here - waku-org/nwaku#1451. I can take it up after we're done with waku-org/nwaku#1457 |
This PR updates some zerokit RLN APIs in light of the security design changes discussed in vacp2p/research#152, Rate-Limiting-Nullifier/rln-circuits#1 and implemented in Rate-Limiting-Nullifier/rln-circuits#2.
In particular external_nullifier is now computed as the Poseidon hash of epoch and RLN identifier, while in generating shares the nullifier is computed only by hashing the
a1
coefficient (see above issues for more details).Note that all circuit resources had to be re-generated and that new circuits are not compatible with previous ones.
@richard-ramos @fryorcraken could you please ensure that these changes doesn't break anything in rln-wasm and js-rln ? (should not, but better we double check)
FYI: @curryrasul @AtHeartEngineer