[zk-token-sdk] Add aggregate range proof instructions #31793
Conversation
samkim-crypto
added
work in progress
Codecov Report
@@ Coverage Diff @@
## master #31793 +/- ##
=========================================
- Coverage 81.9% 81.9% -0.1%
=========================================
Files 740 745 +5
Lines 206372 206700 +328
=========================================
+ Hits 169096 169347 +251
- Misses 37276 37353 +77 |
| ProofInstruction::VerifyAggregateRangeProof64, | ||
| ProofInstruction::VerifyAggregateRangeProof128, | ||
| ProofInstruction::VerifyAggregateRangeProof256, |
There was a problem hiding this comment.
Similar comment to the other review, I wish these instruction names were named in a less general way and where closer to the actual math they're doing. In this case I don't have a good suggestion for what those names could be though
There was a problem hiding this comment.
Yes, so naming here could be a bit complicated. These instructions are basically verifying multiple range proofs in a single instruction. Doing so is more efficient (and proofs more compact) than verifying individual range proofs separately.
A suitable name could be VerifyMultiplePedersenCommitmentsHoldSpecifiedRangeValues, but this is a long name and there are also three variations of them depending on the sum of the bit lengths... so it would be VerifyMultiplePedersenCommitmentsHoldSpecifiedRangeValuesWithTotalBitLength64... I think it might be better to change the name to VerifyBatchedRangeProofsBitLength64. Wdyt?
There was a problem hiding this comment.
@CriesofCarrots - wdyt? Would you mind weighing in here, a 3rd pov would probably be useful
There was a problem hiding this comment.
I'm leaning toward something like VerifyBatchedRangeProofsU64
There was a problem hiding this comment.
Okay, I'll update to VerifyBatchedRangeProofsU64 and then add proof description on the zk_token_proof_instruction.rs.
zk-token-sdk/src/instruction/aggregate_range_proof/aggregate_range_proof_256.rs
Outdated
Show resolved
Hide resolved
…ange_proof_256.rs Co-authored-by: mvines <[email protected]>
… be pre-allocated
Problem
The zk-token-proof program does not yet have an instruction that can verify an aggregate range proof on a multiple Pedersen commitment.
Summary of Changes
Add
VerifyAggregateRangeProof64,VerifyAggregateRangeProof128andVerifyAggregateRangeProof256instructions. These instructions have separate instruction data, but share the same context. The proof data types are organized in the submoduleinstruction::aggregate_range_proof.The verification for these instructions were benched in the devserver and CU units were computed assuming that 1 CU should take roughly 33ns (as per #25464 (comment)).
Fixes #