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-proof] include VerifyBatchRangeProofU256 in the enable_zk_transfer_with_fee feature gate #34747

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

samkim-crypto
Copy link
Contributor

@samkim-crypto samkim-crypto commented Jan 11, 2024

Problem

The VerifyBatchRangeProofU256 instruction allows users to create a "batched" range proof on a sequence of commitments. The individual bit-lengths of the encoded values in these commitments must some up to 256.

Technically this allows a user to create a range proof of a single 256-bit committed value. This is theoretically fine, but since elliptic curve group is defined over a 256-bit prime number, it could lead to some unexpected consequences/security issues in the future. It would be good to restrict the individual bit-lengths in the batched range proof to be at most 128.

Summary of Changes

Since VerifyBatchRangeProofU256 is technically only required for transfer with fee, include the instruction as part of the transfer with fee feature gate.

Fixes #

@samkim-crypto samkim-crypto added the work in progress This isn't quite right yet label Jan 11, 2024
Copy link

codecov bot commented Jan 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d3690dd) 81.8% compared to head (0e4fd49) 81.8%.
Report is 37 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master   #34747     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         824      824             
  Lines      222692   222694      +2     
=========================================
- Hits       182314   182264     -50     
- Misses      40378    40430     +52     

@samkim-crypto samkim-crypto removed the work in progress This isn't quite right yet label Jan 12, 2024
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks good to me! Does this need to get backported to 1.17?

@samkim-crypto
Copy link
Contributor Author

Ah yes, it will need to be backported to 1.17. I'll add the tag.

@samkim-crypto samkim-crypto added the v1.17 PRs that should be backported to v1.17 label Jan 12, 2024
@samkim-crypto samkim-crypto merged commit bc13642 into solana-labs:master Jan 12, 2024
38 checks passed
Copy link
Contributor

mergify bot commented Jan 12, 2024

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

mergify bot pushed a commit that referenced this pull request Jan 12, 2024
…k_transfer_with_fee` feature gate (#34747)

include `VerifyBatchRangeProofU256` in the `enable_zk_transfer_with_fee` feature

(cherry picked from commit bc13642)
samkim-crypto added a commit that referenced this pull request Jan 12, 2024
…nable_zk_transfer_with_fee` feature gate (backport of #34747) (#34765)

[zk-token-proof] include `VerifyBatchRangeProofU256` in the `enable_zk_transfer_with_fee` feature gate (#34747)

include `VerifyBatchRangeProofU256` in the `enable_zk_transfer_with_fee` feature

(cherry picked from commit bc13642)

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.17 PRs that should be backported to v1.17
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants