-
Notifications
You must be signed in to change notification settings - Fork 53
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): added tests for static rln-relay-v2 #2484
Conversation
You can find the image built from this PR at
Built from fb8bf41 |
0db67f7
to
e445c37
Compare
e445c37
to
5f31f48
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! 💯
raise newException(ValueError, "Failed to remove member from the merkle tree") | ||
|
||
if g.withdrawCb.isSome(): | ||
await g.withdrawCb.get()(@[Membership(rateCommitment: rateCommitment, index: index)]) |
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.
Not super critical but shall we separate this line into two statements so that it is clear the item that withdrawCb
is returning?
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.
addressed in 249c50c
Co-authored-by: Ivan FB <128452529+Ivansete-status@users.noreply.github.com>
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
registrations[0].idCommitment == idCommitment | ||
registrations[0].index == 10 | ||
when defined(rln_v2): | ||
require: |
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.
nitpick. registrations.len == 1
and registrations[0].index == 10
is the same in v1 and v2. extract it and remove the duplicate?
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.
addressed in 320661d
withdrawals[0].index == 0 | ||
when defined(rln_v2): | ||
require: | ||
withdrawals.len == 1 |
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.
same, duplicated?
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.
addressed in 588160f
Description
Follows suit with the changes that were made in #2482
Updates the static group manager + tests for rln-v2, defaulting the UserMessageLimit to 20 for testing purposes.
Note
The rln-v2 test suite is not feature complete yet, i.e it has to be integrated in other test suites like
test_wakunode_rln_relay.nim
andtest_waku_rln_relay.nim
, those will be addressed in follow up PRsChanges
How to test
source env.sh
nim c --out:build/static_rln_test -r -d:chronicles_log_level=DEBUG -d:rln_v2:true -d:os=Darwin --verbosity:0 --hints:off -d:chronicles_log_level=TRACE -d:git_version="v0.24.0-rc.0-35-g50308e" -d:release --passL:librln_v0.4.1.a --passL:-lm tests/waku_rln_relay/test_rln_group_manager_static.nim