Refinements of RLN on mainnet spec #34
Conversation
standards/core/rln-contract.md
Outdated
| ### Choosing Which _Expired_ Memberships to Overwrite | ||
|
|
||
| When registering a new membership, the smart contract may need to decide which _Expired_ memberships to overwrite. | ||
| The criteria for this selection can vary depending on the implementation. | ||
|
|
||
| Key considerations include: | ||
| - To minimize gas costs, it's better to overwrite a single high-rate membership rather than multiple low-rate ones. | ||
| - To encourage timely withdrawals, it's better to overwrite memberships that have been _Expired_ for a long time. |
There was a problem hiding this comment.
Do we agree @richard-ramos that for now, we don't include this feature in the smart contract?
There was a problem hiding this comment.
The way it works right now is that you can:
- Specify the memberships you want to expire
- If you don't specify:
-
- If there's enough space in the tree / enough rate limit: create the membership
-
- Otherwise, attempt to free it.
Since for any option ideally the client should call estimateGas to determine which option is cheaper, and the smart contract's been already modified to support this scenario, we can keep it. Unless we want to remove the tracking of the membership age ordering in the smart contract completely? (i.e. we will not know in the smart contract what's the oldest membership to expire). If so, then, cool. It means less gas for the user, and nwaku can track the age of the memberships then, and choose which memberships to expire.
There was a problem hiding this comment.
nwaku can track the age of the membership
Are you saying that nwaku would need to listen to emit events from the smart contract? I think we definitely do not want that as the whole point of onchain tree was to not have to load all events.
However, can the solution be:
- membership and some age information are stored in contract
- BUT they are not ordered
Meaning that it's cheaper to insert or expire (no ordering at insertion). But it's possible to get all data from the contract API (no tracing of emitted events).
The ordering can then be done by the applicaiton (nwaku, jswaku) etc
There was a problem hiding this comment.
Sure! no need to use events. There's already a function that would let you iterate over all the memberships registered. Also perhaps in the future we might want to integrate with the graph and obtain this information from there too.
I'll propose the changes on a separate branch from waku-org/waku-rlnv2-contract#13 so it's possible to compare both options. I do think not having to track the ordering in the contract will be very beneficial due to reduced costs
standards/core/rln-contract.md
Outdated
| - If `r > R_{free}`: | ||
| - if `r > R_{free} + R_{expired}`, registration MUST fail; | ||
| - if `r <= R_{free} + R_{expired}`, the new membership MUST be registered by overwriting some _Expired_ memberships. | ||
| - The sender of the registration transaction MAY specify a list of _Expired_ memberships to be overwritten. If the list is not provided, the contract MAY use any criteria to select _Expired_ memberships to overwrite (see Implementation Suggestions). |
There was a problem hiding this comment.
sembr please.
If the list is not provided, the contract MAY use any criteria to select Expired memberships to overwrite (see Implementation Suggestions).
I would not bother implementing that @richard-ramos
standards/core/rln-contract.md
Outdated
| - To minimize gas costs, it's better to overwrite a single high-rate membership rather than multiple low-rate ones. | ||
| - To encourage timely deposit withdrawals, it's better to overwrite memberships that have been _Expired_ for a long time. |
There was a problem hiding this comment.
I think this is unneeded complexity in the smart contract. I did push in favour of lazy override initially. But I was convinced otherwise by higher gas cost and fluctuation of price.
IMO it would make more sense for the app to select the membership to override.
Meaning the contract would just revert if the total rate limit is exceed and no memberships are being overridden (no gas lost, right @richard-ramos ?)
It's then up to the application to be clever on how to select memberships to override.
s-tikhomirov
force-pushed
the
feat-rln-on-mainnet-spec-edits
branch
from
92cd45f
to
cc1a917
Compare
|
This PR is ready for review. It corresponds to my other suggested PR to the contract code: waku-org/waku-rlnv2-contract#17 |
This allows the contract to transfer ownership to address 0x000...000 as defined in waku-org/specs#34: `At some point, the _Owner_ SHOULD renounce their privileges, and the contract MUST become immutable`. The problem with `Ownable2StepUpgradeable` is that it requires accepting the ownership transfer, which is not possible with address 0x0
This allows the contract to transfer ownership to address 0x000...000 as defined in waku-org/specs#34: `At some point, the _Owner_ SHOULD renounce their privileges, and the contract MUST become immutable`. The problem with `Ownable2StepUpgradeable` is that it requires accepting the ownership transfer, which is not possible with address 0x0
This allows the contract to transfer ownership to address 0x000...000 as defined in waku-org/specs#34: `At some point, the _Owner_ SHOULD renounce their privileges, and the contract MUST become immutable`. The problem with `Ownable2StepUpgradeable` is that it requires accepting the ownership transfer, which is not possible with address 0x0
This allows the contract to transfer ownership to address 0x000...000 as defined in waku-org/specs#34: `At some point, the _Owner_ SHOULD renounce their privileges, and the contract MUST become immutable`. The problem with `Ownable2StepUpgradeable` is that it requires accepting the ownership transfer, which is not possible with address 0x0
This PR contains relateively minor edits to the RLN contract spec that resulted from discussion during its implementation.