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

chore: improve terminology consistency #16

Merged

Conversation

s-tikhomirov
Copy link
Contributor

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

Description

Rationale

This PR aims to introduce consistent usage of terms throughout the code to make sure that:

  • variable and function names reflect their essence;
  • the same thing is called the same name;
  • different things are called different names;
  • naming is consistent with the spec.

Making terminology consistent will hopefully make it easier to review substantial changes to the contract both in the context of the currentlu open membership-related PRs (#13, #14) as well as for future modifications.

Terminology

To remain consistent with the spec and between different pieces of code, the following term usage is suggested. This PR implements (but is not limited to) these guidelines:

Recommended Not recommended Comments
Membership Member, User By using "animate" notions like "member" or "user", we open up the discussion on who the user is, what distinguishes one users from multiple users, whether one user can register multiple memberships, etc. In the scope of the smart contract, the basic unit we account for is a membership.
Membership set Set, Tree (exceptions apply) The membership set is the central concept of the RLN protocol. The fact that it is implemented as a Merkle tree, in particular a LazyIMT, is an implementation detail. The suggestion is to use the phrase "membership set" whenever possible, unless dealing with tree-specific functions like getting a Merkle proof. (Although even that can be called "inclusion proof" or something.) When using the word "tree" or its derivatives, clarify in the comments that this tree stores the membership set (or, even more precisely, the rate limits of memberships in the membership set).
Rate limit Limit, message limit The term "rate limit" corresponds to the spec and also is consistent with the term "rate commitment".
Rate commitment, Id commitment Commitment There are two types of commitments in the code: rate commitments and ID commitments. Using just the word "commitment" can be confusing, as it's not immediately clear which commitment we're talking about. The suggestion is to always specify the type of commitment, including in derivative names of variables and functions, like getRateCommitment.

Checklist

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

  • Added natspec comments?
  • Ran pnpm adorno?

@s-tikhomirov s-tikhomirov marked this pull request as ready for review September 27, 2024 13:08
@s-tikhomirov s-tikhomirov force-pushed the feat/membership-notrack-term-consistency branch 2 times, most recently from c77fbeb to c494c4a Compare September 27, 2024 16:07
@s-tikhomirov s-tikhomirov force-pushed the feat/membership-notrack-term-consistency branch from c494c4a to 3ede4c3 Compare September 27, 2024 16:15
@s-tikhomirov s-tikhomirov merged commit ce52272 into feat/membership-notrack Sep 27, 2024
3 of 4 checks passed
@s-tikhomirov s-tikhomirov deleted the feat/membership-notrack-term-consistency branch September 27, 2024 16:19
richard-ramos added a commit that referenced this pull request Oct 7, 2024
* unify terminology for mdetails
* more DRY in LinearPriceCalculator
* improve terminology consistency (#16)
* refactor, minor fixes
* do not reset ongoing grace period on extension (cf. spec change)
* minor renaming, comments
* refactor, group functions by funcitonality
* extract membership expiration calculation to internal function
* save active duration per membership (must carry over after extension)
* optional lazy erasure from membership set
* minor fixes in tests
* fix: off-by-one: end index can't be equal to next free index
* minor refactoring, comments
* define period boundaries: start inclusive, end exclusive
* separate eraseMembership functions to user-focused (lazy) and admin-focused (tree cleanup)
* minor fix to maintain line lengths
* unify membership-related events
* add test for  zero grace period
* fix typo in comment

Co-authored-by: richΛrd <[email protected]>

---------

Co-authored-by: richΛrd <[email protected]>
richard-ramos added a commit that referenced this pull request Oct 7, 2024
* unify terminology for mdetails
* more DRY in LinearPriceCalculator
* improve terminology consistency (#16)
* refactor, minor fixes
* do not reset ongoing grace period on extension (cf. spec change)
* minor renaming, comments
* refactor, group functions by funcitonality
* extract membership expiration calculation to internal function
* save active duration per membership (must carry over after extension)
* optional lazy erasure from membership set
* minor fixes in tests
* fix: off-by-one: end index can't be equal to next free index
* minor refactoring, comments
* define period boundaries: start inclusive, end exclusive
* separate eraseMembership functions to user-focused (lazy) and admin-focused (tree cleanup)
* minor fix to maintain line lengths
* unify membership-related events
* add test for  zero grace period
* fix typo in comment

Co-authored-by: richΛrd <[email protected]>

---------

Co-authored-by: richΛrd <[email protected]>
richard-ramos added a commit that referenced this pull request Oct 8, 2024
* refactor: do not keep track of membership registration order
* unify terminology for mdetails
* more DRY in LinearPriceCalculator
* improve terminology consistency (#16)
* refactor, minor fixes
* do not reset ongoing grace period on extension (cf. spec change)
* minor renaming, comments
* refactor, group functions by funcitonality
* extract membership expiration calculation to internal function
* save active duration per membership (must carry over after extension)
* optional lazy erasure from membership set
* minor fixes in tests
* fix: off-by-one: end index can't be equal to next free index
* minor refactoring, comments
* define period boundaries: start inclusive, end exclusive
* separate eraseMembership functions to user-focused (lazy) and admin-focused (tree cleanup)
* minor fix to maintain line lengths
* unify membership-related events
* add test for  zero grace period
* fix typo in comment

Co-authored-by: richΛrd <[email protected]>

---------

Co-authored-by: richΛrd <[email protected]>

* code review

* refactor: unique register function

* fix: split errors of `_eraseMembershipLazily`

* chore: minor naming clarifications

---------

Co-authored-by: Sergei Tikhomirov <[email protected]>
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.

1 participant