-
Notifications
You must be signed in to change notification settings - Fork 57
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
chore(rln-relay-v2): wakunode testing + improvements #2501
Conversation
You can find the image built from this PR at
Built from 13227c2 |
32fdb4e
to
f010cac
Compare
2daca1d
to
89adeff
Compare
0988ea7
to
5d5c5ca
Compare
26fbdb9
to
7e1bfd8
Compare
7e1bfd8
to
c4604e7
Compare
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! 💯
Just added minor comments :)
.github/workflows/ci.yml
Outdated
@@ -55,11 +55,12 @@ jobs: | |||
if: ${{ needs.changes.outputs.v2 == 'true' || needs.changes.outputs.common == 'true' }} | |||
strategy: | |||
matrix: | |||
rln_v2 : [true, false] |
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.
Interesting. Does that imply that the rln_v2
param is false
for MacOS?
@@ -58,7 +58,7 @@ const | |||
# rln-v2: rate commitments are used for the Merkle tree construction, defaulting the UserMessageLimit to 20 | |||
# the root is created locally, using createMembershipList proc from waku_rln_relay_utils module, and the result is hardcoded in here | |||
when defined(rln_v2): | |||
const StaticGroupMerkleRoot* = "0a15ba7c5753ee78e8126603113028a343c1a01ffcb389565c76626be158e964" | |||
const StaticGroupMerkleRoot* = "2c149e48886b5ba3da2edf8db8d7a364ae7a25618489c04cf0c0380f7cdd4d6f" |
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.
Why this needs to be changed if the StaticGroupKeys
seems not to be changed?
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.
This is because the leaves in the tree are derived from StaticGroupKeys
in rln-v2, the leaf inserted is actually poseidon([identity_commitment, user_message_limit])
for the static group we default to 20 as the user_message_limit
:)
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, some minor comments.
Description
Follows suit with the changes that were made in #2482, #2484
Updates the wakunode tests for rln-v2, defaulting the UserMessageLimit to 1 for testing purposes.
Note
This PR wraps up the modifications to all the tests related to rln-v2, and does not add additional test vectors. We should be ready to test this in waku-simulator.
Changes
How to test
RLN_V2=true make -j16 test
Issue
closes #2345