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

17/WAKU2-RLN-RELAY: Update #32

Merged
merged 8 commits into from
May 29, 2024
Merged

17/WAKU2-RLN-RELAY: Update #32

merged 8 commits into from
May 29, 2024

Conversation

jimstir
Copy link
Collaborator

@jimstir jimstir commented Apr 19, 2024

Move 17/WAKU2-RLN-RELAY to stable open discussion.
Implementation :

@jimstir jimstir marked this pull request as draft April 19, 2024 04:50
Spammers are also financially punished and removed from the system.


<!-- **Protocol identifier***: `/vac/waku/waku-rln-relay/2.0.0-alpha1` -->
**Protocol identifier***: `/waku/waku-rln-relay/2.0.0-alpha1`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is this needed for a node implementing rln-relay node. I am assuming the implementer would only use /waku/relay/2.0.0

Copy link
Contributor

Choose a reason for hiding this comment

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

agree, this should be removed

Comment on lines 112 to 113
The proof generation relies on the knowledge of two pieces of private information i.e., `sk` and `authPath`.
The `authPath` is a subset of Merkle tree nodes by which a peer can prove the inclusion of its `pk` in the group. <!-- TODO refer to RLN RFC for authPath def -->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should authPath be replaced with path_elements and indices?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, it should

This amount is denoted by `staked_fund` and is a system parameter.
The peer who has the secret key `sk` associated with a registered `pk` would be able to withdraw a portion `reward_portion` of the staked fund by providing valid proof. <!-- a secure way to prove the possession of a pk is yet under discussion, maybe via commit and reveal -->
The peer who has the secret key `sk` associated with a registered `pk` would be able to withdraw a portion `reward_portion` of the staked fund by providing valid proof.
<!-- a secure way to prove the possession of a pk is yet under discussion, maybe via commit and reveal -->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is this still under discussion?

Copy link
Contributor

Choose a reason for hiding this comment

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

waku does not plan on using slashing so it may be removed
cc: @jm-clius for confirmation

Copy link
Contributor

Choose a reason for hiding this comment

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

Slashing can be removed from RLN-Relay, yes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jm-clius Slashing is not implemented in the rln-relay contract, correct? Does this mean off-chain slashing does not occur with spam detection?

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed. When spam is detected, the gossipsub score of the spam-forwarding peer gets lowered. This eventually means that nodes would disconnect from peers that do not drop spam before relaying it to the network.

@jimstir jimstir marked this pull request as ready for review April 19, 2024 11:49
@rymnc
Copy link
Contributor

rymnc commented Apr 19, 2024

hey, thanks for this PR, not sure if this should go to stable yet? as rln-v2 is planned to be shipped for waku's production (and therefore stable) network?

Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

Approving the changes, but would not update status to stable (should remain in draft until we've hardened RLNv2)

This amount is denoted by `staked_fund` and is a system parameter.
The peer who has the secret key `sk` associated with a registered `pk` would be able to withdraw a portion `reward_portion` of the staked fund by providing valid proof. <!-- a secure way to prove the possession of a pk is yet under discussion, maybe via commit and reveal -->
The peer who has the secret key `sk` associated with a registered `pk` would be able to withdraw a portion `reward_portion` of the staked fund by providing valid proof.
<!-- a secure way to prove the possession of a pk is yet under discussion, maybe via commit and reveal -->
Copy link
Contributor

Choose a reason for hiding this comment

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

Slashing can be removed from RLN-Relay, yes

@@ -2,7 +2,7 @@
slug: 17
title: 17/WAKU2-RLN-RELAY
name: Waku v2 RLN Relay
status: draft
status: stable
Copy link
Contributor

Choose a reason for hiding this comment

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

The changes to the draft spec below are useful. However, I wouldn't move this to stable yet. As @rymnc mentions, we are currently transitioning to RLNv2 and will stabilize on that version. I think we also need to explain the role of gossipsub scoring in our protocol before it would be a complete description.

@jimstir jimstir changed the title 17/WAKU2-RLN-RELAY: Move to Stable 17/WAKU2-RLN-RELAY: Update Apr 30, 2024
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 after the change to draft :)

@jimstir jimstir merged commit 7b443c1 into main May 29, 2024
2 checks passed
@jimstir jimstir deleted the rln-relay-update branch May 29, 2024 02:27
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.

None yet

3 participants