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

Bp++: Rangeproof PR #207

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

sanket1729
Copy link
Member

@sanket1729 sanket1729 commented Dec 1, 2022

This second PR introduces Bulletproofs++ Rangeproof argument. Builds on top of #205 . The companion protocol is described here:
BP_PP_proofs.pdf
The companion rust code is available at https://github.com/sanket1729/rust-bulletproofs-pp

This includes the core protocol with some optimizations but does not include

  1. Complex multiplication.
  2. Early termination.
  3. Stack space efficient prover/verifier.
  4. An implementation using gej_add_gej
  5. Another trick in BP++ to reduce the size of c_vec. This is a non-trivial one to prove, but IMO the benefits are not worth the complexity.

Benchmarks show 0.95 ms for verification and ~4 ms prover time for 64 bit rangeproofs

#include "scalar.h"
#include "bulletproofs_util.h"

/* Initialize the transcript with a fix protocol string*/
Copy link
Contributor

Choose a reason for hiding this comment

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

In 83b18e2:

lol pls provide a comment explaining how to reproduce these values

Copy link
Member Author

Choose a reason for hiding this comment

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

Lol, yep. The commit history in this PR is messed up. I was not expecting such a quick review here. Updating the PR to have a cleaner history.

Copy link
Member Author

@sanket1729 sanket1729 Jan 20, 2023

Choose a reason for hiding this comment

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

You can double-check via https://github.com/sanket1729/rust-bitcoin/tree/bp_comm_reproduce cargo test -- --nocapture

@jonasnick
Copy link
Contributor

PR #216 contains two commits that fix compilation and tests.

@sanket1729 sanket1729 force-pushed the bpp_final branch 4 times, most recently from a14b562 to c0da897 Compare February 8, 2023 21:07
q_inv_pows = (secp256k1_scalar*)secp256k1_scratch_alloc(&ctx->error_callback, scratch, g_offset * sizeof(secp256k1_scalar));
g_vec_pub_deltas = (secp256k1_scalar*)secp256k1_scratch_alloc(&ctx->error_callback, scratch, g_offset * sizeof(secp256k1_scalar));

if ( q_pows == NULL || q_inv_pows == NULL ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if ( q_pows == NULL || q_inv_pows == NULL ) {
if ( q_pows == NULL || q_inv_pows == NULL || g_vec_pub_deltas == NULL) {

sanket1729 and others added 9 commits August 1, 2023 16:24
- The first two bytes can be anything.
- Also put the correct index for R_tmp
Liam had previsouly pointed that this should also commit to the
generators. Tim mentioned that generators should be exposed in API at
all. Porbably a good time to think through these issues
This commit also updates norm argument to use asset_gen instead of G for
storing the inner product. This is an artifact of how CTs are used in
elements/liquid today.
For compatibility with exisiting Pedersen commitments data structures,
it is necessary to have commitments be of the G_a*v + G*gamma where G_a
is asset gen and gamma is blinding factors.

However, in BP++ design, the blinding values are along H_vec. In order
make these compatible with BP++, we make h0 = G

Also updates a few test cases to start from |H| = 8.
Commit to digits and multiplicities
Commit to reciprocals of digits as 1/(e + d_i)
Commit to S. Compute l's adaptive to create a zero polynomial
Run the run norm proof argument on the computed C
Combines all four rounds
Copy link
Contributor

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

See #220 for discussion on the generators API.

* Args: ctx: pointer to a context object
* scratch: pointer to a scratch space
* gens: pointer to the generator set to use, which must have exactly
* `n = max(num_digits, base) + 7` generators, where num_digits is the number.
Copy link
Contributor

Choose a reason for hiding this comment

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

"num_digits is the number." -> "num_digits is floor(log_base(value)) + 1?

@jonasnick
Copy link
Contributor

I think you already did these changes, but, for reference, I made a checklist for rebasing #266.

/** Verifies an Bulletproofs++ rangeproof. Returns 1 on success, 0 on failure.
* Args: ctx: pointer to a context object
* scratch: pointer to a scratch space
* gens: pointer to the generator set to use, which must have at least 2*n_bits generators

Choose a reason for hiding this comment

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

bench_bppp uses 24 generators for both proving/verification; it looks like this should also be updated to reflect floor(log_base(value)) + 1.

@dr-orlovsky
Copy link

Is there anything which has to be done to get this merged?

I mean I am trying to estimate when we may have BPP rangeproofs in secp256k-zkp

@jonasnick
Copy link
Contributor

jonasnick commented Feb 19, 2024

Is there anything which has to be done to get this merged?

@dr-orlovsky Yes, this needs a lot more review and polishing. As far as I know, no one is working on this right now.

@dr-orlovsky
Copy link

Thank you for providing the update! So I assume it sounds more like ~year(s) and certainly not weeks/month

g_len = num_digits > base ? num_digits : base;
h_len = 8;
n_rounds = secp256k1_bppp_log2(g_len > h_len ? g_len : h_len);
return 33 * 4 + 65*n_rounds + 64;
Copy link

Choose a reason for hiding this comment

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

The Bulletproofs++ paper claims on page 4, in Table 1, that a 64-bit range proof consists of 10 group elements and 3 scalars. The calculation here counts 12 group elements and 2 scalars (since n_rounds is 4 with base 16). What is the explanation for this discrepancy?

Copy link
Member Author

Choose a reason for hiding this comment

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

@tevador, this does not implement all the optimizations with early terminations.

Copy link

Choose a reason for hiding this comment

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

Searching for "early termination" in the paper gives nothing. Can you point me to the relevant section of the paper?

Copy link

Choose a reason for hiding this comment

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

OK, I think I figured it out from the code. If you do just 3 rounds instead of 4, you'll end up with n_vec of length 2 and l_vec of length 1, which is actually more compact.

Copy link
Collaborator

@real-or-random real-or-random Jun 10, 2024

Choose a reason for hiding this comment

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

Indeed. In the paper, we stop the recursion already when len(l_vec) + len(n_vec) <= 6, see page 13.

Copy link

Choose a reason for hiding this comment

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

I think it should be len(l_vec) + len(n_vec) < 6. After 2 rounds, we get len(l_vec) = 2 and len(n_vec) = 4, but the proof can still be reduced by 31 or 32 bytes by performing the 3rd round.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants