41/WAKU2-RLN-CONTRACT: Raw spec #522
Conversation
content/docs/rfcs/38/README.md
Outdated
| ### 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. |
There was a problem hiding this comment.
What are the thoughts around offering a smart contract that accept ERC-20 tokens as deposit?
content/docs/rfcs/38/README.md
Outdated
| * `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. This function MUST remove the associated member from the list of members. This function MUST fire the `MemberWithdrawn` event. |
There was a problem hiding this comment.
-
I would advise to *not have the receiver parameter but instead, withdraw the ETH to the contract caller to protect against MEV attacks (where ones learns the secret by monitoring the mempool and squeeze a transaction faster to own the funds).
-
We discussed about considering the RLN as a membership that was not refundable. So that a spammer gets slashed but the funds remain in the pool. Are we still considering this design?
There was a problem hiding this comment.
I agree with your suggestion. I am notified of this now. The contract needs design changes and the RFC will reflect so.
content/docs/rfcs/38/README.md
Outdated
|
|
||
|
|
||
| * 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` |
There was a problem hiding this comment.
Same an above, receiver should not be included to avoid MEV attacks.
content/docs/rfcs/38/README.md
Outdated
| @@ -0,0 +1,76 @@ | |||
| --- | |||
| slug: 38 | |||
There was a problem hiding this comment.
38 is taken already: #512
The next free one is 41, afaik.
|
@fryorcraken Thanks for your comments. Note that this RFC has been written with https://github.com/kgrgpg/rln/blob/main/contracts/Rln.sol in mind. This will evolve as RLN contract implementation changes with time. The support for ERC-20 tokens has been created as a new issue. This can be found at waku-org/nwaku#1058 . Please contribute to the discussion there. |
Co-authored-by: fryorcraken.eth <110212804+fryorcraken@users.noreply.github.com>
Co-authored-by: fryorcraken.eth <110212804+fryorcraken@users.noreply.github.com>
Has anyone reviewed the solidity contract? I don't see any PR on https://github.com/kgrgpg/rln/ that could have given the opportunity to review the solidity code. It's a bit of a deadlock here if feedback on the RFC is handled as "that's how the contract API is" but there are no opportunity to review the contract API. |
|
@fryorcraken seems to be a revision of https://github.com/kilic/rlnapp/blob/master/packages/contracts/contracts/RLN.sol but no PR/issue/review though. |
From my understanding, there was a RLN contract that already exists that Blagoj/Taylor were working on. I guess this is https://github.com/kilic/rlnapp/blob/master/packages/contracts/contracts/RLN.sol or something similar (can't find it under PSE GH, but presumably somewhere there? zk-kit, rln, rlnapp, what does zk-chat use?...) Then there was a piece of work to deploy that. There may have been changes required to add e.g. ERC20 support, but not clear to me beyond that.
|
|
Also, how is this contract Waku 2 specific? As opposed to being generally useful for RLN. |
|
@oskarth#7074 @fryorcraken @s1fr0 I was introduced to https://github.com/kilic/rlnapp/blob/master/packages/contracts/contracts/RLN.sol at the start of my employment. I had wondered the same about it not being under the Status/Vac organisation. Since I needed more control of the repo, I created a public clone on my personal repo. In any case, the contract is now pushed to https://github.com/vacp2p/rln-contract . |
|
I was introduced last week to https://github.com/Rate-Limiting-Nullifier/consensus-layer-rln-registration/blob/main/contracts/RLN_Registry.sol by Taylor. This seemed rather a much trimmed down version for RLN membership contract to me when compared to https://github.com/kilic/rlnapp/blob/master/packages/contracts/contracts/RLN.sol. Taylor said to me that he didn't write the latter and will take a look when he is back from his vacation. |
For sure, will take a look and add my comments in the next couple of days. I am sorry for the delay, but busy with other things. |
|
@kgrgpg what is the state of this PR? would be great if we could get the comments addressed and get it merged. |
Things look good to me. |
|
Re-requesting reviews. |
kgrgpg
requested review from
richard-ramos,
kaiserd,
s1fr0,
staheri14 and
fryorcraken
content/docs/rfcs/41/README.md
Outdated
| title: 41/WAKU2-RLN-CONTRACT | ||
| name: Rate Limiting Nullifier Membership Contract | ||
| status: raw | ||
| category: RLN |
There was a problem hiding this comment.
Category should be one of (Standards Track|Informational|Best Current Practice).
In this case, most likely Standards Track.
content/docs/rfcs/41/README.md
Outdated
| name: Rate Limiting Nullifier Membership Contract | ||
| status: raw | ||
| category: RLN | ||
| tags: 17/WAKU-RLN-RELAY |
There was a problem hiding this comment.
For now, the only well defined tags are waku-application and waku-core-protocol.
We could introduce a new tag here, but I'd rather simply name it RLN, or RLN-core.
Any opinions?
content/docs/rfcs/41/README.md
Outdated
| contributors: | ||
| --- | ||
|
|
||
| ## Tags |
There was a problem hiding this comment.
This section is just informational in the template. See template:
This section (including its subsections) MUST be removed.
content/docs/rfcs/41/README.md
Outdated
|
|
||
|
|
||
| # Abstract | ||
| The following standard allows for the implementation of a standard API for Rate Limiting Nullifier Contract that manages membership of the participants in a peer-to-peer messaging group. |
There was a problem hiding this comment.
| The following standard allows for the implementation of a standard API for Rate Limiting Nullifier Contract that manages membership of the participants in a peer-to-peer messaging group. | |
| 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. |
content/docs/rfcs/41/README.md
Outdated
| 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 message is considered spam. |
There was a problem hiding this comment.
| are considered spammers and their message is considered spam. | |
| are considered spammers and their messages are considered spam. |
| 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.
In a future draft (or at least for the stable function), I'd expect a more in depth explanation here.
content/docs/rfcs/41/README.md
Outdated
| 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. | ||
|
|
||
| # 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.
While this section should exist and contain an implementable spec, in this case, I'd rather call it
Contract Specification
(This spec does not specify a protocol with wire format).
| * 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. | ||
| 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.
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.
|
|
||
|
|
||
| # Implementation Suggestions | ||
|
|
There was a problem hiding this comment.
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.
| # Copyright | ||
|
|
||
| Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/). | ||
|
|
There was a problem hiding this comment.
References Section listing the references in this spec. (See other specs)
|
@kgrgpg what's the state of this PR? |
|
@kaiserd Thanks for your comments. I have addressed your comments. |
|
Closing as not planned and active development on the rln-contract has been moved to https://github.com/rate-limiting-nullifier/rln-contract |
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. This pull request is for the issue explained in #511.