Generalize the range proof test relation #80

cjpatton · 2025-09-08T18:28:13Z

Use the bit decomposition of from
https://github.com/SamuelSchlesinger/authenticated-pseudonyms to compute the bases in test_range().

src/tests/test_relations.rs

cathieyun · 2025-09-09T16:09:28Z

src/tests/test_relations.rs

-        (0..BITS)
-            .map(|i| var_Ds[i] *  bases[i])
-            .sum::<Sum<_>>(),
+        (0..bits).map(|i| var_Ds[i] * bases[i]).sum::<Sum<_>>(),


This might be outside the scope of this PR, but I'd like to double check that the underlying implementation does actually check this relation outside the proof.
(Also this is "fancier" than what the normal sigma spec actually specifies, so maybe we should just do this check explicitly / manually over the commitments instead of using this nice shortcut?)

This might be outside the scope of this PR, but I'd like to double check that the underlying implementation does actually check this relation outside the proof.

I'm not sure I'm totally clear. Do you mean plug in different values for the variable's we've allocated and make sure the relations actually hold when we expect them to?

If so, I agree that would be a good idea. I think though that we'd need to expand the test driver works (see test_relations). Thoughts on this @mmaker?

(Also this is "fancier" than what the normal sigma spec actually specifies, so maybe we should just do this check explicitly / manually over the commitments instead of using this nice shortcut?)

I believe you mean that append_equation() wants an Into<LinearCombination>, which I believe here is provided by std::iter::Sum<G> for Sum<G>. If you like we can construct the LinearCombination explicitly.

So this line of code is a little misleading, because it looks like it is appending a constraint to the proof statement... but in practice (because the equation is "trivial"), it actually marks this as a linear equation to check outside the proof. This is weird (and maybe not desirable) for several reasons:

I haven't actually checked that the "shortcut" which takes trivial equations, and enforces that their relations get checked outside the proof, actually does that correctly.

Even if that is done correctly, this is a "shortcut" in the sigma code only, but this shortcut doesn't (IIUC) exist in the sigma spec, so this doesn't actually reflect the right way to use the sigma spec. If you were using the API the spec provided, you would actually just do the thing the "shortcut" is doing under the hood, which is something like actually (explicitly) checking:
C == (0..bits).map(|i| D[i] * bases[i]).sum()

Therefore, to remove the confusion of using this shortcut, I think it's better to just do the explicit check. (From your comments, maybe you are thinking of this as a linear equation that's being enforced within the proof itself... which exactly illustrates why this shortcut is more confusing than helpful, IMO)

(ok but just to argue the other side, if we remove this "shortcut" here, then we need to do this check C == (0..bits).map(|i| D[i] * bases[i]).sum() at proof verification... and you have to forget not to do this, and it's a very easy thing to forget to check. So the nice part of this "shortcut" is that it encodes this as a check you need to perform at verification, if I understand the shortcut correctly...) So, given the tradeoffs, want to check if this makes sense to you / if you have opinions on the shortcut.

Thanks for clarifying, I initially misunderstood your comment.

I read this line as as appending a constraint. I don't like like the idea of the code having an implicit side-effect under the hood. I think we should make it explicit here, as it would be in the spec, but I'm not really clear on how. Would you mind suggesting a change?

I agree, let's make it explicit. The way the tests are set up right now do not facilitate adding "external-to-proof" checks on the commitments at verification time, so this would require restructuring the tests a little. I would suggest something that looks more like how we structured the bulletproofs tests (and this also introduces the "gadget" idea, which is the thing that adds constraints to your prover/verifier but is generic over prover/verifier - which I think is very nice, and tracks with what we discussed about "composing" multiple constraint sets):
https://github.com/zkcrypto/bulletproofs/blob/main/tests/r1cs.rs#L147

I can't take a crack at it this week, but hopefully this can help you get an idea of what I mean. Happy to work on this after EOW though.

I might try to find time on Friday, but will otherwise leave it to you :)

src/tests/test_relations.rs

Use the bit decomposition of from <https://github.com/SamuelSchlesinger/authenticated-pseudonyms> to compute the bases in `test_range()`.

cjpatton · 2025-09-10T00:53:49Z

Rebased to resolve conflicts.

Haven't tested it extensively yet! This is just a draft.

mmaker · 2025-09-11T10:47:47Z

Hi Cathie and Chris, thank you so much for your interest in this repo. I tried to simplify your algorithm and generalize it even more, but you should double-check it!

@cathieyun : CanonicalLinearRelation is the type you are trying to use and is conformant with the spec. We made it "type safe", so you can instantiate this object only if the relation passes the validation criteria we discussed. I'm not sure I understand the suggestion about structuring unit-tests, but if you could provide a PR it'd be really helpful to the project.

One important note: we are not generating test vectors here.
If you want to incorporate test vectors to the CFRG specification, and provide better examples, you must change this file:
https://github.com/mmaker/draft-irtf-cfrg-sigma-protocols/blob/main/poc/test_sigma_protocols.sage
and then edit the specification here:
https://github.com/mmaker/draft-irtf-cfrg-sigma-protocols/blob/main/draft-irtf-cfrg-sigma-protocols.md

I’m not sure I understand why you are focusing on a test vector in this repository and I'm sorry if there was confusion about this.
If you are trying to make a clear range proof example, I think your effort will be better invested there. This repository aims at being deployment-ready code that implements the IETF CFRG specification, with some utilities and sugar that we believe make the developer experience better. Some of the utilities built on top are needed by other projects and it won't be possible to alter them. The unit-test you're modifying is just an example relation in our test suite that contains many other relations, including BBS, CMZ wallet spending, ring signatures, and more.

The choices of the IETF will inform this repository but we have no commitment on how we build the API on top. With that in mind, I wanted to re-iterate that you're very welcome to continue the discussion here!

Thank you so much for contributing to sigma-rs :)

cjpatton · 2025-09-11T21:22:31Z

src/tests/test_relations.rs

-    let bases = [1, 2, 4, 8, 16, 32, 64, 128, 256, 313, 512].map(G::Scalar::from);
-    const BITS: usize = 11;
+    let delta = range.end - range.start;
+    let whole_bits = (delta - 1).ilog2() as usize;


@mmaker I don't like the name bits for this because it sounds like the number of bits we want to decompose the input into (i.e., bases.len()). I rename to whole_bits to denote the fact that it's the number of "non-remainder" bits.

makes sense, thanks

cjpatton · 2025-09-11T21:23:51Z

src/tests/test_relations.rs

+        // XXX Make this constant time
+        let mut rest = input - range.start;
+        let mut b = vec![G::Scalar::ZERO; bases.len()];
+        assert!(rest < delta);
+        for (i, &base) in bases.iter().enumerate().rev() {
+            if rest >= base {
+                b[i] = G::Scalar::ONE;
+                rest -= base;
+            }
+        }
+
+        b


@mmaker I think this could should be constant time (i.e., avoid branching on rest >= base) so that it's safer to copy-paste.

Sure, but it was't before either haha. You can submit a different pull request with this code if you think it's not too complicated for a reader that just wants to know how things can work. I personally think it's too complex but trust your judgment here.

assert!(rest < delta); for (i, &base) in bases.iter().enumerate().rev() { let choice = !base.ct_lt(&rest); let (new_rest, _) = rest.overflowing_sub(base); ConditionallySelectable::conditional_assign(&mut b[i], &G::Scalar::ONE, choice); ConditionallySelectable::conditional_assign(&mut rest, &new_rest, choice); }

cjpatton · 2025-09-11T21:41:08Z

Hi Cathie and Chris, thank you so much for your interest in this repo. I tried to simplify your algorithm and generalize it even more, but you should double-check it!

I pushed a few changes and left some comments for you :)

One important note: we are not generating test vectors here. If you want to incorporate test vectors to the CFRG specification, and provide better examples, you must change this file: https://github.com/mmaker/draft-irtf-cfrg-sigma-protocols/blob/main/poc/test_sigma_protocols.sage and then edit the specification here: https://github.com/mmaker/draft-irtf-cfrg-sigma-protocols/blob/main/draft-irtf-cfrg-sigma-protocols.md

I’m not sure I understand why you are focusing on a test vector in this repository and I'm sorry if there was confusion about this. If you are trying to make a clear range proof example, I think your effort will be better invested there. This repository aims at being deployment-ready code that implements the IETF CFRG specification, with some utilities and sugar that we believe make the developer experience better. Some of the utilities built on top are needed by other projects and it won't be possible to alter them. The unit-test you're modifying is just an example relation in our test suite that contains many other relations, including BBS, CMZ wallet spending, ring signatures, and more.

Generating test vectors is not my goal. My goal for this PR was to flesh out the range proof in full detail so that we all have a common understanding of how it works. It was helpful to do so with actual code. I wasn't aware there was a sage implementation.

You and Cathie are clear that you don't want the range proof spec in draft-irtf-cfrg-sigma-protocols; are you alright with having it in the reference implementation? For what it's worth, working with sage is a bit of a nightmare at this scale, especially if you have dependencies from other repos. The only way I've figured out how to do this with is with submodules, which is hacky and not fun. It seems like we're not doing any fancy math, in which case I wonder if we could just convert to pure Python.

The choices of the IETF will inform this repository but we have no commitment on how we build the API on top. With that in mind, I wanted to re-iterate that you're very welcome to continue the discussion here!

To clarify: do you plan to be interoperable with the sigma draft? I.e., if/when test vectors are produced from the sage code, will you consume them here?

mmaker · 2025-09-12T16:47:09Z

Chris,

I'm glad there's no misunderstanding then! And yes, we do consume the test vectors already inside sigma_proofs::tests::spec!

To be clear: I'm happy to put a range proof test inside the spec if that helps you and @cathieyun, I'm not particularly opinionated :)

mmaker · 2025-09-13T09:58:51Z

src/tests/test_relations.rs

+        // XXX Make this constant time
+        let mut rest = input - range.start;
+        let mut b = vec![G::Scalar::ZERO; bases.len()];
+        assert!(rest < delta);
+        for (i, &base) in bases.iter().enumerate().rev() {
+            if rest >= base {
+                b[i] = G::Scalar::ONE;
+                rest -= base;
+            }
+        }
+
+        b


Sure, but it was't before either haha. You can submit a different pull request with this code if you think it's not too complicated for a reader that just wants to know how things can work. I personally think it's too complex but trust your judgment here.

assert!(rest < delta); for (i, &base) in bases.iter().enumerate().rev() { let choice = !base.ct_lt(&rest); let (new_rest, _) = rest.overflowing_sub(base); ConditionallySelectable::conditional_assign(&mut b[i], &G::Scalar::ONE, choice); ConditionallySelectable::conditional_assign(&mut rest, &new_rest, choice); }

mmaker · 2025-09-13T09:59:07Z

src/tests/test_relations.rs

-    let bases = [1, 2, 4, 8, 16, 32, 64, 128, 256, 313, 512].map(G::Scalar::from);
-    const BITS: usize = 11;
+    let delta = range.end - range.start;
+    let whole_bits = (delta - 1).ilog2() as usize;


makes sense, thanks

Signed-off-by: Abdullah Talayhan <[email protected]>

…omposedResponse (sigma-rs#83) Signed-off-by: Michele Orrù <[email protected]>

Thanks to Ian for spotting the missing trivial optimization.

cjpatton commented Sep 8, 2025

View reviewed changes