Skip to content
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): add more hardcoded memberhips to static group #2108

Merged
merged 2 commits into from
Oct 5, 2023

Conversation

alrevuelta
Copy link
Contributor

Description:

  • We currently had a hardcoded tree with 100 memberships for the static version of rln.
  • This PR adds more harcoded memberships, a total of 10000.
  • Useful for simulations and tests.
  • New file is 2.7Mb which should be okay.
works
./build/wakunode2 --rln-relay-dynamic=false --rln-relay=true --rln-relay-membership-index=9999

does not work (we just have 10000)
./build/wakunode2 --rln-relay-dynamic=false --rln-relay=true --rln-relay-membership-index=10001

@alrevuelta alrevuelta requested a review from rymnc October 4, 2023 15:15
@github-actions
Copy link

github-actions bot commented Oct 4, 2023

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2108

Built from 09bf749

Copy link
Collaborator

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for that!

I approve because now we need two approvers. However, my approval shouldn't have lot of weight :P

A couple of comments:
Isn't the next constant redundant? Maybe it can be obtained from the .len of the array?
StaticGroupSize* = 10000

The next comment should be updated to reflect 10000:
# StaticGroupKeys is a static list of 100 membership keys in the form of (identity key, identity commitment)

In case we are interested in having only private constants, maybe it worth to just include that "constants" module instead of importing it.

Copy link
Contributor

@rymnc rymnc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

@alrevuelta
Copy link
Contributor Author

@Ivansete-status indeed, fixed 5a8e020

Isn't the next constant redundant? Maybe it can be obtained from the .len of the array?

A bit yeap, but it serves as a way to know how many static memberships there are. There are checks ensuring that == .len so imho its nice to have.

@alrevuelta alrevuelta merged commit 1042cac into master Oct 5, 2023
10 checks passed
@alrevuelta alrevuelta deleted the add-static-rln branch October 5, 2023 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants