Ideas for better usability

[vck3000]

Hi guys,

Extremely cool project, thanks for putting in so much effort into open sourcing this for us.

I've been developing some circuits for some university research and had some ideas on possible features for usability that I thought might have been nice to have. If you'd like me to make an issue/discussion for some or all of the below, let me know!

1. Option to configure the logger.
   I'm wrapping a circuit up in a web framework and it would be nice if we could configure the logger by means of an API call or by passing a logger object in to control the verbosity of messages. Sometimes I would also want to print multiple things on the same line for easier viewing so maybe a api.Print() method may also be nice to have?

2. Solving for output value in backend.Prove() along with proof.
   Prove() currently requires a complete witness to work. I believe some other frameworks allow us to define "output" wires, and given inputs, can solve for the output. I understand that computing the result of a circuit in normal programming paradigms is fairly trivial in most cases, but if the circuit has to compute it itself anyway, I thought it might be nice for the circuit itself to be able to calculate and output the result rather than computing the result twice.

3. Implementing assert.ProverSucceededSingleCurve() (or modifying assert.ProverSucceeded()).
   The current assert.ProverSucceeded() runs slowly due to it testing through all the implemented backends and curves, making my unit tests time consuming to sit through. I think to the end user, circuit correctness is more important, and I reckon we assume that any implemented backend/curve should still work. If "for a given circuit that works for one backend/curve, it will work for all backends/curves" is a bad assumption, perhaps we could note that somewhere for future readers.

4. Aliasing Not() to IsZero().
   When working with boolean logic in the circuit design, I found Not() to have the same functionality as IsZero() but with a more intuitive name. Given we have AND, OR, XOR, etc. I think it might be nice to simply alias Not() to IsZero().

5. Implementing LessThan().
   assertEquals() solves the issue of equality, however inequalities are currently difficult to compute. I implemented a Compare(a, b []frontend.Variable) function that compares bit by bit and determines which one is larger. Happy to share my code if you'd like to see.

6. Documenting negative numbers.
   This probably is obvious to most people, but when I was learning I didn't realise that api.ToBinary() doesn't support negative numbers (finite fields... of course). It might be nice to have it in the docs too? It seems that frontend.Variable does indeed handle negative signs though.

7. Missing ReadRawFrom().
   Not sure if this was intentional, but WriteTo has a counterpart ReadFrom, but WriteRawTo doesn't have a counterpart ReadRawFrom currently.

[ThomasPiellard]

Hi @vck3000 , I am curious about the 5., do you have a way to return a boolean telling if a<b? Currently we have AssertIsLessOrEqual(a,b), constraining a to be <= b. But we do not have a function returning a boolean testing the result of this operation.

[vck3000]

Hi @ThomasPiellard,

Here's the code for my Compare() function. It can be adapted easily to return a boolean to check a < b if desired.

// Returns 1 if a > b
// Returns 0 if a == b
// Returns -1 if a < b
// Assume len(a) == len(b)
//
// Input bits must be little endian ordered.
func Compare(api frontend.API, aBits, bBits []frontend.Variable) frontend.Variable {
	api.AssertIsEqual(len(aBits), len(bBits))

	output := api.Constant(0)

	// Backwards due to little endian
	for i := len(aBits) - 1; i >= 0; i-- {
		api.AssertIsBoolean(aBits[i])
		api.AssertIsBoolean(bBits[i])

		aIsOne := aBits[i]
		bIsOne := bBits[i]

		aIsNotOne := api.IsZero(aIsOne) // alias for api.Not() would be nice here.
		bIsNotOne := api.IsZero(bIsOne)

		aBeatsB := api.And(aIsOne, bIsNotOne)
		bBeatsA := api.And(aIsNotOne, bIsOne)

		newOutput := api.Select(aBeatsB, 1, 0)
		newOutput = api.Select(bBeatsA, -1, newOutput)

		output = api.Select(api.IsZero(output), newOutput, output)
	}

	return output
}

It essentially checks to see which variable (a or b) is the first one to still have a 1 bit when the other is 0. It then persists that result to the end of the loop.

A shortcoming of this method is that the length of the bits must be compile time determined, as api.ToBinary() requires.

Another shortcoming is that negative number inputs don't work. I haven't looked into how api.ToBinary() handles the negative numbers.

Here's the unit tests:

type CompareCircuit struct {
	A              frontend.Variable
	B              frontend.Variable
	ExpectedOutput frontend.Variable
}

func (c *CompareCircuit) Define(curveID ecc.ID, api frontend.API) error {
	LEN := 30

	aBits := api.ToBinary(c.A, LEN)
	bBits := api.ToBinary(c.B, LEN)

	output := Compare(api, aBits, bBits)

	api.AssertIsEqual(output, c.ExpectedOutput)

	return nil
}

func TestCompare(t *testing.T) {
	t.Run("Simple a > b", func(t *testing.T) {
		testHelper.TestSingleCurve(t, &CompareCircuit{}, &CompareCircuit{
			A:              frontend.Value(11),
			B:              frontend.Value(10),
			ExpectedOutput: frontend.Value(1),
		})
	})

	t.Run("Simple a < b", func(t *testing.T) {
		testHelper.TestSingleCurve(t, &CompareCircuit{}, &CompareCircuit{
			A:              frontend.Value(9),
			B:              frontend.Value(10),
			ExpectedOutput: frontend.Value(-1),
		})
	})

	t.Run("Simple a == b", func(t *testing.T) {
		testHelper.TestSingleCurve(t, &CompareCircuit{}, &CompareCircuit{
			A:              frontend.Value(10),
			B:              frontend.Value(10),
			ExpectedOutput: frontend.Value(0),
		})
	})
}

I also implemented my own equals() function that returns a boolean. It's fairly trivial but might be nice to include also:

// Returns 1 if equal, 0 if not equal.
func equals(api frontend.API, a, b frontend.Variable) frontend.Variable {
	return api.IsZero(api.Sub(a, b))
}

Happy to make a PR if you guys like it.

[ThomasPiellard]

That is a nice solution, I think it is worth it to make it a PR. I think it would be better that to have the following signature
Compare(a b frontend.Variable) frontend.Variable (directly available from the API interface) and then do the binary decomposition inside Compare. By default the scalars are understood as being of size equal to the underlying group. You can also add the IsEqual function as well on the API interface, I agree that it is nice to have it directly off the shelf. Or you can just put an issue if you prefer, pointing to this discussion to add Compare and IsEqual on the API and we will add it.

For the moment we handle nothing but group elements (on Fr). We don't deal with negative numbers. We plan to add it at some point.

[vck3000]

Sounds good. Definitely agree on that function signature being more suitable.

I'll find some time next month after I finish my research to make the PRs, but if I don't get around to it in time, happy for anyone else to do it too.