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

[zk-token-sdk] replace hard-coded constants with constant variables #32274

Merged
merged 9 commits into from
Jun 28, 2023

Conversation

samkim-crypto
Copy link
Contributor

@samkim-crypto samkim-crypto commented Jun 26, 2023

Problem

Throughout zk-token-sdk, there are many hard-coded constants, which make the code harder to read and maintain.

Summary of Changes

Use constant variables to replace hard-coded constants.

In the process, I updated/simplified the way serialization is done in the sigma module. Previously, direct indexing was used to update slices. Now, chunks iterator is used to update the slices.

The hard-coded constants in the range_proof module will be updated in a separate PR.

Fixes #

@samkim-crypto samkim-crypto added the work in progress This isn't quite right yet label Jun 26, 2023
@samkim-crypto samkim-crypto force-pushed the zk-token-sdk/constant-variables branch from 8b83ce1 to a967163 Compare June 26, 2023 02:43
@codecov
Copy link

codecov bot commented Jun 26, 2023

Codecov Report

Merging #32274 (c2b0bff) into master (4cfdb37) will increase coverage by 0.0%.
The diff coverage is 89.4%.

@@           Coverage Diff           @@
##           master   #32274   +/-   ##
=======================================
  Coverage    82.0%    82.0%           
=======================================
  Files         773      773           
  Lines      209616   209603   -13     
=======================================
+ Hits       172038   172083   +45     
+ Misses      37578    37520   -58     

@samkim-crypto samkim-crypto force-pushed the zk-token-sdk/constant-variables branch 3 times, most recently from 6af03dc to e1a63d0 Compare June 27, 2023 06:55
@samkim-crypto samkim-crypto added v1.16 PRs that should be backported to v1.16 and removed work in progress This isn't quite right yet labels Jun 27, 2023
Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

Generally in favor :)
Just a couple suggestions, plus a few comments that need updating.

zk-token-sdk/src/sigma_proofs/mod.rs Outdated Show resolved Hide resolved
zk-token-sdk/src/sigma_proofs/fee_proof.rs Outdated Show resolved Hide resolved
zk-token-sdk/src/sigma_proofs/pubkey_proof.rs Outdated Show resolved Hide resolved
zk-token-sdk/src/sigma_proofs/zero_balance_proof.rs Outdated Show resolved Hide resolved
zk-token-sdk/src/zk_token_elgamal/pod/range_proof.rs Outdated Show resolved Hide resolved
@samkim-crypto samkim-crypto force-pushed the zk-token-sdk/constant-variables branch from 9488552 to 29bc565 Compare June 28, 2023 01:39
zk-token-sdk/src/sigma_proofs/mod.rs Outdated Show resolved Hide resolved
zk-token-sdk/src/sigma_proofs/mod.rs Outdated Show resolved Hide resolved
zk-token-sdk/src/sigma_proofs/mod.rs Outdated Show resolved Hide resolved
@samkim-crypto samkim-crypto force-pushed the zk-token-sdk/constant-variables branch from c65cb3d to c2b0bff Compare June 28, 2023 06:52
@samkim-crypto samkim-crypto merged commit 91186d3 into master Jun 28, 2023
@samkim-crypto samkim-crypto deleted the zk-token-sdk/constant-variables branch June 28, 2023 22:03
mergify bot pushed a commit that referenced this pull request Jun 28, 2023
…32274)

* add ristretto and scalar byte length constants

* add serialization and deserialization helper functions

* remove hard-coded constants in the `sigma` module

* remove hard-coded constants in the `encryption` module

* remove hard-coded constants in the `zk-token-elgamal` module

* Apply suggestions from code review

Co-authored-by: Tyera <[email protected]>

* fix docs for range proof constants

* Apply suggestions from code review

Co-authored-by: Tyera <[email protected]>

* clippy

---------

Co-authored-by: Tyera <[email protected]>
(cherry picked from commit 91186d3)
samkim-crypto added a commit that referenced this pull request Jun 29, 2023
…ables (backport of #32274) (#32322)

[zk-token-sdk] replace hard-coded constants with constant variables (#32274)

* add ristretto and scalar byte length constants

* add serialization and deserialization helper functions

* remove hard-coded constants in the `sigma` module

* remove hard-coded constants in the `encryption` module

* remove hard-coded constants in the `zk-token-elgamal` module

* Apply suggestions from code review

Co-authored-by: Tyera <[email protected]>

* fix docs for range proof constants

* Apply suggestions from code review

Co-authored-by: Tyera <[email protected]>

* clippy

---------

Co-authored-by: Tyera <[email protected]>
(cherry picked from commit 91186d3)

Co-authored-by: samkim-crypto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.16 PRs that should be backported to v1.16
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants