41/WAKU2-RLN-CONTRACT: Raw spec #522
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,85 @@ | ||||||||
| --- | ||||||||
| slug: 41 | ||||||||
| title: 41/WAKU2-RLN-CONTRACT | ||||||||
| name: Rate Limiting Nullifier Contract | ||||||||
| status: raw | ||||||||
| category: RLN | ||||||||
|
There was a problem hiding this comment. Category should be one of (Standards Track|Informational|Best Current Practice). |
||||||||
| tags: 17/WAKU-RLN-RELAY | ||||||||
|
There was a problem hiding this comment. For now, the only well defined tags are |
||||||||
| editor: Keshav Gupta <keshav@status.im> | ||||||||
| contributors: | ||||||||
| --- | ||||||||
|
|
||||||||
| ## Tags | ||||||||
|
There was a problem hiding this comment. This section is just informational in the template. See template:
|
||||||||
|
|
||||||||
| Currently identified tags comprise | ||||||||
|
|
||||||||
| * `17/WAKU-RLN-RELAY` protocol is an extension of `11/WAKU-RELAY` which additionally provides spam protection using Rate Limiting Nullifiers (RLN). | ||||||||
|
|
||||||||
|
|
||||||||
| # Abstract | ||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||
| The following standard allows for the implementation of a standard API for Rate Limiting Nullifier Contract that manages membership of the participants in a group. | ||||||||
|
There was a problem hiding this comment. If this is the scope, then it's independent from RLN. I think here should be mentioned how RLN disincentivizes spam (i.e., economically) by attaching proofs which reveal shares of the private key to sent messages. There was a problem hiding this comment. As it is, it does not indicate how this API relates to the RLN-Relay and the RLN RFC. More elaboration is needed. |
||||||||
|
|
||||||||
|
|
||||||||
| # Background / Rationale / Motivation | ||||||||
|
There was a problem hiding this comment. In here, we need more details on how each membership is represented, the relation between user sk and pk. |
||||||||
|
|
||||||||
| This contract serves as the basis for the implementation of RLN membership. New members can be registered by depositing a stake of valuable funds. | ||||||||
|
There was a problem hiding this comment. I think here the relevant RFCs should be reported and briefly explained, stressing their role with respect to this https://rfc.vac.dev/spec/17/, https://rfc.vac.dev/spec/32/, https://rfc.vac.dev/spec/11/ |
||||||||
| They can also be slashed if they misbehave like spamming in a peer to peer pseudo-anonymous messaging network. | ||||||||
|
|
||||||||
| # 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. 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. | ||||||||
|
There was a problem hiding this comment. Please use sematic line break as mentioned in https://rfc.vac.dev/spec/1/#conventions. There was a problem hiding this comment. I think this sentence had to be slightly rephrased. The role of the deposit is not mentioned at this point, but later is in the contract APIs. merkle tree -> Merkle tree. |
||||||||
|
|
||||||||
|
There was a problem hiding this comment. In a future draft (or at least for the stable function), I'd expect a more in depth explanation here. |
||||||||
| # Wire Format Specification / Syntax | ||||||||
|
There was a problem hiding this comment. The section names in the template may be adjusted if it makes sense. Their content is what MUST be there.
(This spec does not specify a protocol with wire format). |
||||||||
|
|
||||||||
| ### 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. | ||||||||
|
There was a problem hiding this comment. The Merkle tree has not been introduced before in this RFC. It needs to be explained in the "background" section. |
||||||||
| * `_poseidonHasher` is the address of the poseidon hasher contract used in the `hash` function. | ||||||||
|
There was a problem hiding this comment. Probably it's a good idea to include a reference to the poseidon hasher contract in the implementations section |
||||||||
|
|
||||||||
| ### 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. | ||||||||
|
There was a problem hiding this comment. Should the condition There was a problem hiding this comment. 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. | ||||||||
|
There was a problem hiding this comment. 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 There was a problem hiding this comment. Right, this is a well-known issue re RLN slashing model and the followings are the identified countermeasures:
There was a problem hiding this comment. 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` | ||||||||
|
There was a problem hiding this comment. this is more a contract issue, but I find confusing using There was a problem hiding this comment. I agree. I'd suggest changing this even for the raw spec. |
||||||||
|
|
||||||||
|
|
||||||||
| * 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)` | ||||||||
|
There was a problem hiding this comment. Again, this is a contract issue: why emitting the pubkey? The index should be enough to correctly identify the slashed key and save gas. There was a problem hiding this comment.
The pk may be needed for cases that the event listeners do not have the whole Merkle tree stored. There was a problem hiding this comment. 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) There was a problem hiding this comment. 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. |
||||||||
|
|
||||||||
|
|
||||||||
| # Implementation Suggestions | ||||||||
|
|
||||||||
|
There was a problem hiding this comment. 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. There was a problem hiding this comment. 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. There was a problem hiding this comment. I'd not put an implementation here, as such specs should be implementation detail agnostic. 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. |
||||||||
| Complete implementation of this contract can be found at https://github.com/kilic/rlnapp/blob/master/packages/contracts/contracts/RLN.sol | ||||||||
|
There was a problem hiding this comment. Should this be updated to https://github.com/vacp2p/rln-contract ? |
||||||||
|
|
||||||||
| # Copyright | ||||||||
|
|
||||||||
| Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/). | ||||||||
|
|
||||||||
|
There was a problem hiding this comment.
|
||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cdaa21a