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

Feat/membership notrack refactor #17

Merged

Conversation

s-tikhomirov
Copy link
Contributor

@s-tikhomirov s-tikhomirov commented Sep 27, 2024

Description

This PR introduces relatively minor changes, renaming, and refatoring implemented during my initial review of the RLNv2 contract inplementation.

Checklist

Ensure you completed all of the steps below before submitting your pull request:

  • Added natspec comments?
  • Ran pnpm adorno?

@s-tikhomirov s-tikhomirov changed the base branch from main to feat/membership-notrack September 27, 2024 18:25
@s-tikhomirov s-tikhomirov marked this pull request as ready for review October 3, 2024 10:58
@s-tikhomirov
Copy link
Contributor Author

This PR is ready for review. It corresponds to my other suggested PR to the spec: waku-org/specs#34

src/WakuRlnV2.sol Outdated Show resolved Hide resolved
/// this fucntion decreases Merkle tree size and spends more gas (if eraseFromMembershipSet == true).
/// @param idCommitments The list of idCommitments of the memberships to erase
/// @param eraseFromMembershipSet Indicates whether to erase membership data from the membership set
function eraseMemberships(uint256[] calldata idCommitments, bool eraseFromMembershipSet) external {
Copy link
Member

Choose a reason for hiding this comment

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

I guess setting eraseFromMembershipSet to true could be taken into account in the future to bring additional rewards to whoever call the function if we decide to choose that route, right? otherwise there's no incentive to pass true since it will cost extra gas with no benefit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My idea here is that we don't want to burden users with extra gas cost for cleaning up the tree, but "we" (as contract administrators) may call eraseMemberships with eraseFromMembershipSet = true when necessary to clean up the tree at our own expense. I agree that we could plug in a reward mechanism for third parties to do this clean-up if we deem it necessary.

Co-authored-by: richΛrd <[email protected]>
@s-tikhomirov s-tikhomirov merged commit 79fd7ec into feat/membership-notrack Oct 7, 2024
4 checks passed
@s-tikhomirov s-tikhomirov deleted the feat/membership-notrack-refactor branch October 7, 2024 13:02
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.

2 participants