Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

41/WAKU2-RLN-CONTRACT: Raw spec #522

Closed
wants to merge 20 commits into from
Closed

41/WAKU2-RLN-CONTRACT: Raw spec #522

Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
bee51cd
First RLN RFC Draft
kgrgpg Aug 1, 2022
e026c9b
Merge branch 'master' into RLN-Contract-RFC
kgrgpg Aug 1, 2022
98df664
Update content/docs/rfcs/38/README.md
kgrgpg Aug 2, 2022
bd5ddec
Update content/docs/rfcs/38/README.md
kgrgpg Aug 2, 2022
35a97b8
Line Breaks. Slug number.
kgrgpg Aug 2, 2022
b3d454f
Line Breaks
kgrgpg Aug 2, 2022
c9edd11
Merge branch 'master' into RLN-Contract-RFC
kgrgpg Aug 2, 2022
6cfa7a0
Contract implementation source
kgrgpg Aug 14, 2022
e384a14
Semantic line breaks
kgrgpg Aug 14, 2022
cdaa21a
Name Change
kgrgpg Aug 14, 2022
dd53832
Reference of Poseidon Hasher
kgrgpg Aug 14, 2022
1155d3c
Abstract refers to RLN and RLN-Relay
kgrgpg Aug 14, 2022
977f0d0
Background RLN secret and pk
kgrgpg Aug 14, 2022
01e3705
Category
kgrgpg Sep 18, 2022
158edb6
Tags
kgrgpg Sep 18, 2022
4a17144
Tags section removed
kgrgpg Sep 18, 2022
bd53546
Wording
kgrgpg Sep 18, 2022
601ddc6
Wire Format to Contract Specification
kgrgpg Sep 18, 2022
6cb579c
Implementation Suggestion to Example
kgrgpg Sep 18, 2022
66347f1
References
kgrgpg Sep 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions content/docs/rfcs/41/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
---
slug: 41
title: 41/WAKU2-RLN-CONTRACT
name: Rate Limiting Nullifier Membership Contract
status: raw
category: Standards Track
tags: RLN
editor: Keshav Gupta <keshav@status.im>
contributors:
---

# Abstract
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Abstract
# Abstract

The following standard specifies a standard API for Rate Limiting Nullifier Contract that manages membership of the participants in a peer-to-peer messaging group.
This API is intended to be used in conjunction with [17/WAKU-RLN-RELAY](https://rfc.vac.dev/spec/17/).
Peers that violate the messaging rate in a relay network,
like explained in [11/WAKU2-RELAY](https://rfc.vac.dev/spec/11/),
are considered spammers and their messages are considered spam.
Spammers are financially punished and removed from the system.
Rate limiting nullifier (RLN), like explained in [32/RLN](https://rfc.vac.dev/spec/32/),
a construct based on zero-knowledge proofs that provides an anonymous rate-limited signaling/messaging framework suitable for decentralized (and centralized) environments.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
a construct based on zero-knowledge proofs that provides an anonymous rate-limited signaling/messaging framework suitable for decentralized (and centralized) environments.
is a construct based on zero-knowledge proofs that provides an anonymous rate-limited signaling/messaging framework suitable for decentralized (and centralized) environments.



# Background / Rationale / Motivation
Copy link
Contributor

Choose a reason for hiding this comment

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

In here, we need more details on how each membership is represented, the relation between user sk and pk.

Copy link
Author

Choose a reason for hiding this comment

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

Refer 977f0d0


This contract serves as the basis for the implementation of RLN membership.
Copy link
Contributor

Choose a reason for hiding this comment

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

The spec should not depends on its abstract.
I'd make explicit what "this contract" is.
I'd at least say: "The RLN contract".
Better yet, add a description of what RLN is in one or two sentences.

Copy link
Contributor

Choose a reason for hiding this comment

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

Reading it again, I'd add at least one introductory sentence at the beginning of this section.

New members can be registered by depositing a stake of valuable funds.
They can also be slashed if they misbehave like spamming in a peer-to-peer pseudo-anonymous messaging network.
Peers MUST be registered to the RLN group to be able to publish messages.
Each peer has an RLN key pair denoted by `secret` and `pubkey`.
The state of the membership contract contains the list of registered members' public identity keys i.e., `pubkeys`.
For the registration, a peer creates a transaction that invokes the registration function of the contract via which registers its `pubkey` in the group.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For the registration, a peer creates a transaction that invokes the registration function of the contract via which registers its `pubkey` in the group.
For the registration, a peer creates a transaction that invokes the registration function of the contract.
This function registers the respective peer's `pubkey` with the group.

The peer who has the secret key `secret` associated with a registered `pubkey` would be able to withdraw a portion the staked fund by providing valid proof.


Copy link
Contributor

Choose a reason for hiding this comment

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

Coming back to my first comment on this section, I'd reorganize this section.

Starting with

This contract serves as the basis for the implementation of RLN membership.

is a bit confusing.


# Theory / Semantics

As explained in [17/WAKU-RLN-RELAY](https://rfc.vac.dev/spec/17/),
all peers who want to publish the message in the spam protected peer-to-peer network MUST register for membership to the RLN group.
This is done via the `register` function of the RLN membership contract.
If a registered member of the RLN group spams messages in the network then its secret key get exposed which can be used to slash its membership from the group.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If a registered member of the RLN group spams messages in the network then its secret key get exposed which can be used to slash its membership from the group.
If a registered member of the RLN group spams messages in the network then its secret key gets exposed which can be used to slash its membership from the group.

This is done by calling the `withdraw` function of the RLN membership contract, which removes the member from the Merkle tree thereby revoking its rights to send any messages.

Copy link
Contributor

Choose a reason for hiding this comment

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

In a future draft (or at least for the stable function), I'd expect a more in depth explanation here.

# Contract Specification

### Constructor

* `constructor(uint256 membershipDeposit, uint256 depth, address _poseidonHasher) public`
* `membershipDeposit` sets the amount of ETH that must be deposited to `register` to the RLN group.
* `depth` is the depth of the Merkle tree that determines the maximum number of members to the RLN group.
* `_poseidonHasher` is the address of the poseidon hasher contract used in the `hash` function.
Copy link
Member

Choose a reason for hiding this comment

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

Probably it's a good idea to include a reference to the poseidon hasher contract in the implementations section

Copy link
Author

@kgrgpg kgrgpg Aug 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Refer dd53832


### Methods

* register: This function registers the `pubkey` to the list of members.
For successful registration, ETH equal to the amount specified in `membershipDeposit` MUST be paid while calling this function.
Copy link
Member

Choose a reason for hiding this comment

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

Should the condition pubkeyIndex < SET_SIZE be described as one of the conditions required for a succesful register?

Copy link
Author

Choose a reason for hiding this comment

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

This can be better addressed as an issue in the contract implementation rather than the RFC?

This function MUST fire the `MemberRegistered` event.
* `function register(uint256 pubkey) external payable`



* registerBatch: This function registers multiple `pubkeys` to the list of members.
For succesful registration, ETH equal to the amount specified in `membershipDeposit * pubkeys.length` MUST be paid while calling this function.
This function MUST fire the `MemberRegistered` event for each registered member.
* `function registerBatch(uint256[] calldata pubkeys) external payable`


* withdraw: This function accepts the `secret` of the public key at index`_pubkeyIndex` to withdraw the ETH equal to the`membershipDeposit` to the `receiver` address.
Copy link
Contributor

Choose a reason for hiding this comment

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

From what is written above and here, it seems like a spammer could prepare lots of spam messages (that would eventually slash him) while preparing a transaction to withdraw the deposit to another address in his possession. In fact, by knowing in advance that he will be slashed, he can front-run relayers to get the deposit.

AFAIR, we discussed that the amount withdrawn should be less than the membershipDeposit so that we disincentivize such behavior (or as @fryorcraken said, we not allow any deposit to be withdraw).

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, this is a well-known issue re RLN slashing model and the followings are the identified countermeasures:

  • X1 portion of the fund can be charged as a fee (for protocol revenue as indicated in Incorporating fee option in the rln contract #485)
  • X2 portion of the fund can optionally be burned
  • Only a X3 portion is refundable to the account holder where X1 + X2 + X3 = 1
    X1, X2 and X3 all should be configurable options in the constructor.
    I think the second and third items must be easily integratabtle in the current RFC. @kgrgpg Please update the write up to reflect the last 2. For the first item, there is a separate issue Incorporating fee option in the rln contract #485

Copy link
Author

Choose a reason for hiding this comment

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

A new PR of this RFC will be created once #485 issue is solved and vacp2p/rln-contract#5 PR on the RLN contract is approved

This function MUST remove the associated member from the list of members.
This function MUST fire the `MemberWithdrawn` event.
* `function withdraw(uint256 secret, uint256 _pubkeyIndex, address payable receiver) external`
Copy link
Contributor

Choose a reason for hiding this comment

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

this is more a contract issue, but I find confusing using _pubkeyIndex to denote the withdrawn public key index, while the global variable containing the next available Merkle tree index in is named pubkeyIndex

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. I'd suggest changing this even for the raw spec.
The spec should be a clear guideline to implementers and not be confusing.



* withdrawBatch: This function accepts multiple `secrets` of the public keys at the indices `pubkeyIndexes` to withdraw the ETH equal to the `membershipDeposit * secrets.length` to the `receiver` address.
This function MUST remove the associated members from the list of members.
This function MUST fire the `MemberWithdrawn` event for each member slashed from the group.
* `function withdrawBatch(uint256[] calldata secrets, uint256[] calldata pubkeyIndexes, address payable[] calldata receivers) external`


### Events

* MemberRegistered: MUST trigger when a member is registered to the RLN group.
* `event MemberRegistered(uint256 indexed pubkey, uint256 indexed index)`


* MemberWithdrawn: MUST trigger when a member is slashed from the RLN group.
* `event MemberWithdrawn(uint256 indexed pubkey, uint256 indexed index)`
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, this is a contract issue: why emitting the pubkey? The index should be enough to correctly identify the slashed key and save gas.

Copy link
Contributor

Choose a reason for hiding this comment

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

Again, this is a contract issue: why emitting the pubkey? The index should be enough to correctly identify the slashed key and save gas.

The pk may be needed for cases that the event listeners do not have the whole Merkle tree stored.

Copy link
Contributor

@s1fr0 s1fr0 Aug 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

And then why it should listen to such event? I think the main goal is to update the local copy of the Merkle tree, but for this you just need to delete the previous leaf at the slashed index (but the leaf has to correctly be there, unless is last leaf added, in order to correctly recompute the root)

Copy link
Contributor

Choose a reason for hiding this comment

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

It is true that in the current implementation, we keep the Merkle tree and emitting the pk may not be necessary, but consider the case that instead of replacing the leaf, a blacklist of Pks is maintained. Emitting the pk gives the flexibility for such solutions.



# Example Implementation

Complete implementation of this contract can be found at https://github.com/vacp2p/rln-contract

The contract for Poseidon Hasher can be found at https://github.com/vacp2p/rln-contract/blob/main/contracts/PoseidonHasher.sol

## References

Copy link
Member

@richard-ramos richard-ramos Aug 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Instead of referencing the implementation of the contract via a github link, what do you think of adding the solidity source code just like it is done in the ethereum ERCs? otherwise, it might be a good idea to reference an specific commit / branch, so this spec matches the contract at an specific point in time.

Copy link
Author

Choose a reason for hiding this comment

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

Will this make the RFC too long or clunky? But I do agree that specific changes to the RFC can be mapped to the specific commits/branch of the contract implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd not put an implementation here, as such specs should be implementation detail agnostic.
The purpose of this section is more to give guidance, e.g., pointing specific implementation choices that significantly improve performance.

If you want to link the implementation. I'd add a new subsection "Example Implementation".

You could also add an appendix that contains the whole source code.

1. [17/WAKU-RLN-RELAY](https://rfc.vac.dev/spec/17/)
2. [11/WAKU2-RELAY](https://rfc.vac.dev/spec/11/)
3. [32/RLN](https://rfc.vac.dev/spec/32/)

# Copyright

Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/).

Copy link
Contributor

Choose a reason for hiding this comment

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

References Section listing the references in this spec. (See other specs)