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-relay): confirm that the provided credential is correct using onchain query #1980

Merged
merged 1 commit into from
Sep 4, 2023

Conversation

rymnc
Copy link
Contributor

@rymnc rymnc commented Sep 1, 2023

Description

Updates tests to use zero-based indexing in the contract deployed at 0x59af93Bb30ED234418E6fC2324597e3FD72FB90e on sepolia
Also checks that the decrypted credential's commitment does in fact belong to the tree by calling memberExists() on the rln storage contract

Changes

  • Adds comments about contract fns
  • checks that the decrypted credential's commitment belongs to the tree
  • updated tests

@github-actions
Copy link

github-actions bot commented Sep 1, 2023

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:1980

@rymnc rymnc force-pushed the member-exists-check branch from bb0e2b7 to 131934c Compare September 1, 2023 09:03
@rymnc rymnc marked this pull request as ready for review September 1, 2023 09:33
@rymnc rymnc force-pushed the member-exists-check branch from 131934c to b0c984a Compare September 1, 2023 09:37
Copy link
Contributor

@alrevuelta alrevuelta left a comment

Choose a reason for hiding this comment

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

ah just realized that memberExists should actually be memberExistsOrExisted (forget the naming). I mean, since the contract is dummy and just emit events, it can't really know if the member exists. It just can know if it exists or existed in the past. Because at some point someone can "unregister" the membership and it won't exist anymore.

Perhaps right now it doesn't matter as we dont "revoke" credentials, but in the future I guess we can't realy on this?

@rymnc
Copy link
Contributor Author

rymnc commented Sep 1, 2023

ah just realized that memberExists should actually be memberExistsOrExisted (forget the naming). I mean, since the contract is dummy and just emit events, it can't really know if the member exists. It just can know if it exists or existed in the past. Because at some point someone can "unregister" the membership and it won't exist anymore.

Perhaps right now it doesn't matter as we dont "revoke" credentials, but in the future I guess we can't realy on this?

Err, the contract does store commitments onchain so it is possible to know if the member exists

@alrevuelta
Copy link
Contributor

Err, the contract does store commitments onchain so it is possible to know if the member exists

ah in that case we're good. Thought it was pure event based with no state kept in the contract. noice.

Copy link
Contributor

@alrevuelta alrevuelta left a comment

Choose a reason for hiding this comment

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

lgtm

@rymnc rymnc merged commit be48891 into master Sep 4, 2023
13 checks passed
@rymnc rymnc deleted the member-exists-check branch September 4, 2023 05:52
@rymnc rymnc self-assigned this Sep 4, 2023
@richard-ramos
Copy link
Member

Is this is the PR that introduced these contract changes?
waku-org/waku-rlnv1-contract#8

@rymnc
Copy link
Contributor Author

rymnc commented Sep 4, 2023

Is this is the PR that introduced these contract changes? waku-org/waku-rln-contract#8

yessir

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants