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!: replace lazy_static with once_cell #69

Merged
merged 4 commits into from Sep 20, 2023

Conversation

AaronFeickert
Copy link
Contributor

Extended commitment generators are currently produced using the lazy_static macro. Unfortunately, lazy_static is no longer actively maintained. Other repositories in the ecosystem are moving to once_cell, which is actively maintained.

This PR migrates from lazy_static to once_cell. In the process, it cleans up the construction. Specifically, it moves from using word-based numbering (e.g. ONE, TWO) in hash inputs to numerals, which simplifies the design. It also adds a useful test that asserts the number of generators matches the size of ExtensionDegree to avoid panics.

Closes #67.

BREAKING CHANGE: Modifies the construction of commitment generators.

Copy link
Contributor

@hansieodendaal hansieodendaal left a comment

Choose a reason for hiding this comment

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

Nice and concise, a definite improvement. Just a couple of small comments below.

src/generators/pedersen_gens.rs Outdated Show resolved Hide resolved
src/generators/pedersen_gens.rs Outdated Show resolved Hide resolved
@AaronFeickert
Copy link
Contributor Author

AaronFeickert commented Sep 4, 2023

There are still some assumptions that this code makes about ExtensionDegree that I don't particularly like:

  • The enum will always contain only incrementing numerical values; this holds now due to the auto-incrementing functionality, but could be inadvertently changed later
  • The MINIMUM and MAXIMUM constants will always refer to the lowest and highest valid enum values, respectively
  • The TOTAL constant (defined in terms of the other constants) will always represent the total number of valid enum values

However, these assumptions are carefully documented, should be easy to check by inspection, are reasonably tested, and I don't see a good way to remove them without overhauling the ExtensionDegree design.

Copy link
Contributor

@hansieodendaal hansieodendaal left a comment

Choose a reason for hiding this comment

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

ACK

src/generators/pedersen_gens.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@hansieodendaal hansieodendaal left a comment

Choose a reason for hiding this comment

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

ACK

Copy link
Contributor

@hansieodendaal hansieodendaal left a comment

Choose a reason for hiding this comment

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

ACK

@SWvheerden SWvheerden merged commit e01c380 into tari-project:main Sep 20, 2023
5 checks passed
@AaronFeickert AaronFeickert deleted the once-cell branch September 20, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace lazy_static = "1.4" with once_cell = "1"
4 participants