feat: add range proof batch verification to validators #4260
Conversation
Added batch verification of range proofs where ever more than one range proof needed to be verified.
| } | ||
| Ok(()) | ||
| } | ||
|
|
There was a problem hiding this comment.
This is a pub function. Does this need to be public?
Either way, this function can benefit from docs. To understand what this function does, you need to read the comment on the function batch_verify_range_proofs and the tests for this function, this is not ideal.
There was a problem hiding this comment.
Good point, fixed
hansieodendaal
force-pushed
the
ho_batch_verification
branch
from
a8531a3 to
4057498
Compare
| pub fn batch_verify_range_proofs( | ||
| prover: &RangeProofService, | ||
| outputs: &[&TransactionOutput], | ||
| ) -> Result<(), RangeProofError> { | ||
| // We need optimized power of two chunks, for example if we have 15 outputs, then we need chunks of 8, 4, 2, 1. | ||
| let power_of_two_vec = power_of_two_chunk_sizes(outputs.len(), 8); | ||
| debug!( | ||
| target: LOG_TARGET, | ||
| "Queueing range proof batch verify output(s): {:?}", &power_of_two_vec | ||
| ); | ||
| let mut index = 0; | ||
| for power_of_two in power_of_two_vec { | ||
| let mut statements = Vec::with_capacity(power_of_two); | ||
| let mut proofs = Vec::with_capacity(power_of_two); | ||
| for output in outputs.iter().skip(index).take(power_of_two) { | ||
| statements.push(RistrettoAggregatedPublicStatement { | ||
| statements: vec![Statement { | ||
| commitment: output.commitment.clone(), | ||
| minimum_value_promise: 0, | ||
| }], | ||
| }); | ||
| proofs.push(output.proof.to_vec().clone()); | ||
| } | ||
| index += power_of_two; | ||
| prover.verify_batch(proofs.iter().collect(), statements.iter().collect())?; | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
This function should be in the bulletproofs+ crate. Think about it - Verifying batches of non power-of-two sets is something you'd be writing over and over if its not there.
There was a problem hiding this comment.
Yes it could move there and be totally transparent.
There was a problem hiding this comment.
I created an issue for it
| @@ -70,7 +70,7 @@ mod transaction_input; | |||
| mod transaction_input_version; | |||
| mod transaction_kernel; | |||
| mod transaction_kernel_version; | |||
| mod transaction_output; | |||
| pub mod transaction_output; | |||
There was a problem hiding this comment.
rather use pub use to expose what you want to, otherwise all of these will eventually be pub
Description
Added batch verification of range proofs where ever more than one range proof needed to be verified.
Motivation and Context
Batch verification range proofs is much faster than running linear proofs
How Has This Been Tested?
Unit tests
Integration tests
System-level tests pending