waku-org / nwaku

Waku node and protocol.
Other
199 stars 52 forks source link

bug: RLN flags issues #2763

Open AlejandroCabeza opened 4 months ago

AlejandroCabeza commented 4 months ago

Background

According to the "RLN Relay Configuration and Parameters" block in RLN's Test Plan, which are based on the RLN specs, there are some flags that can't be tested.

Details

Here goes a snippet of the aforementioned block that includes a brief description of the issue:

2. **RLN Relay Configuration and Parameters:**
    3. **RLN Relay Membership Index (`rln-relay-membership-index`)**: Confirm the correct usage of the on-chain commitment index.
        - Setting `rlnRelayCredIndex` in `WakuRlnConfig` doesn't fetch the keys in the keystore. Although this might be a symptom, and not the actual issue.
    5. **RLN Relay Identity Key Configuration (`rln-relay-id-key`)**: Validate the correct application of the RLN relay identity secret key from the provided Hex string.
        - `rlnRelayIdKey` flag is not used
    6. **RLN Relay Identity Commitment Key Configuration (`rln-relay-id-commitment-key`)**: Ensure the correct usage of the RLN relay identity commitment key from the Hex string.
        - `rlnRelayIdCommitmentKey` flag is not used
    9. **RLN Relay Ethereum Private Key (`rln-relay-eth-private-key`)**: Validate the use of the private key for broadcasting transactions on the Ethereum network.
        - `rlnRelayIdCommitmentKey` is not passed to `WakuRlnConf`
    10. **RLN Relay Credential Password (`rln-relay-cred-password`)**: Check the encryption of RLN credentials using the specified password.
        - `WakuRlnRelay.init` fails if used. Might be a similar situation to 3: not cause, but "symptom".
    11. **RLN Relay Merkle Tree Path (`rln-relay-tree-path`)**: Verify the correct configuration and usage of the path to the RLN Merkle tree database.
        - `WakuRlnRelay.init` fails if used. Might be a similar situation to 3: not cause, but "symptom".

Acceptance criteria

What's expected out of each of those flags?

rymnc commented 4 months ago

2,5,6 are deprecated. as i remember, they were to be removed after 2 releases after deprecation

rymnc commented 4 months ago

9 is used in the keystore generator subcommand for wakunode2

rymnc commented 4 months ago

10 and 11 work as intended, can you share a trace of the error you get if you use it?

rymnc commented 4 months ago

3 is interesting, investigating

rymnc commented 4 months ago

rlnRelayCredIndex

https://github.com/waku-org/nwaku/blob/6298267ab55ae5d4ebe1b5d22eb77d60f6e37d7d/waku/waku_rln_relay/group_manager/on_chain/group_manager.nim#L572-L573

called from

https://github.com/waku-org/nwaku/blob/66eba029efc9d97d7ba224807e414d346cdace6e/waku/waku_rln_relay/rln_relay.nim#L423-L432

called from

https://github.com/waku-org/nwaku/blob/66eba029efc9d97d7ba224807e414d346cdace6e/waku/factory/node_factory.nim#L198-L209

AlejandroCabeza commented 3 months ago

9 is used in the keystore generator subcommand for wakunode2

Well, now I found it. The day of posting I wasn't able to.

10 and 11 work as intended, can you share a trace of the error you get if you use it?

Error: failed to mount WakuRlnRelay: could not initialize the group manager: the commitment does not have a membership You can reproduce it in branch tests/rln, file tests/node/test_wakunode_relay_rln.nim. Uncomment the last two lines of proc getWakuRlnConfigOnChain, lines 94 and 95, so the WakuRlnConfig contains rlnRelayCredPath and rlnRelayCredPassword.

https://github.com/waku-org/nwaku/issues/2763#issuecomment-2149060562

What do you mean to point out with this?