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

possible bug: [RLN] Mebership Index don't need to match #2742

Open
AlejandroCabeza opened this issue May 29, 2024 · 3 comments
Open

possible bug: [RLN] Mebership Index don't need to match #2742

AlejandroCabeza opened this issue May 29, 2024 · 3 comments
Assignees
Labels
bug Something isn't working effort/hours Estimated to be completed in a few hours track:rln RLN Track (Secure Messaging/Applied ZK), e.g. relay and applications

Comments

@AlejandroCabeza
Copy link
Contributor

AlejandroCabeza commented May 29, 2024

Problem

When mounting OnChainGroupManager, the MembershipIndex specified in WakuRlnConfig doesn't need to match the MembershipIndex used on the addMembershipToKeystore.
Also, as for the WakuRlnConfig indices, for some reason they must begin at 0 in incremental order. Otherwise it timeouts.

To reproduce

It's easier to understand by having a look at the test case tests/node/test_wakunode_relay_rln:482, named Valid contract. This test represents the happy path for sending an OnChain RLN message.
In the line 516 two WakuRlnConfig instances are defined, one with MembershipIndex 0, and the other with MembershipIndex 1. In line 536, the indices are retrieved using onChainGroupManager.membershipIndex.get(); but this is irrelevant, as any pair of numbers will do.

Expected behavior

For a node to use a specific set of RLN credentials, their config should specify the same MembershipIndex as the one used to add those credentials to the keystore.

nwaku version/commit hash

@AlejandroCabeza AlejandroCabeza added the bug Something isn't working label May 29, 2024
@AlejandroCabeza
Copy link
Contributor Author

Okay, with today's fresh mind I gathered a bit more information on these.

Matching indices

They're not required because after the node's RLN intialisation we're monkeypatching the node.groupManager.idCredentials. Reading the section below gives a better understanding on this.

Indices in Incremental Order

The credential indices assigned to each node's RLN config are not the tree's indices for those credentials, but the registration order of the patched credentials. Elaborating with an example:

  1. Let's say there are 3 available credentials, c0, c1, c2. You have instantiated them and registered them in that order.
  2. There are two configs that require one index each: configA(indexA) and configB(indexB). And let's arbitrarily say indexA is 0 and indexB is 2.
  3. After initialising two nodes, nodeA and nodeB (each with their corresponding config), you need to monkeypatch each node's groupManager's idCredentials to workaround an initialisation issue (happens during testing). To monkeypatch the credentials, you MUST use the credentials that correspond to the index specified in the respective config, but regarding the registration order of those credentials (not the index those credentials were saved in in the keystore).
  • Given c0 was registered first (akin to index 0), c1 second (akin to index 1), and c2 third (akin to index2). Then for configA, who we said had index=0, you'd need to use c0; and for configB, who we said had index=2, you need to use c2. That is:
    • nodeA.groupManager.idCredentials = c0
    • nodeB.groupManager.idCredentials = c2
      If you don't make these match, the RLN message will be rejected.

I'm still not sure as to why this works this way. As part of the test setup, when you add the credentials to the keystore (calling addMembershipCredentialsToKeystore) you need to specify the index you want to save them in.
Even if you're monkeypatching the credentials later, if you save the credentials in the keystore with indices 100 and 101; I'd expect the nodes to fail booting if you pass them any other indices, as that'd mean it failed fetching the credentials from the keystore.

@AlejandroCabeza
Copy link
Contributor Author

A node that didn't register credentials (this test case works this way, it uses a different manager to register credentials beforehand), should be able to fetch the corresponding credentials with the keystore path, keystore password and membership index.
In this case, after the nodes boot, the idCredentials are none where they should be populated (as the parameters are seemingly okay). This would explain the behaviour described above.

@rymnc rymnc added the track:rln RLN Track (Secure Messaging/Applied ZK), e.g. relay and applications label Jun 3, 2024
@rymnc
Copy link
Contributor

rymnc commented Jun 3, 2024

will investigate this week

@rymnc rymnc self-assigned this Jun 3, 2024
@gabrielmer gabrielmer added the effort/hours Estimated to be completed in a few hours label Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working effort/hours Estimated to be completed in a few hours track:rln RLN Track (Secure Messaging/Applied ZK), e.g. relay and applications
Projects
Status: To Do
Development

No branches or pull requests

3 participants