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

Adds std/math/bits #276

Merged
merged 17 commits into from
Mar 8, 2022
Merged

Adds std/math/bits #276

merged 17 commits into from
Mar 8, 2022

Conversation

gbotrel
Copy link
Collaborator

@gbotrel gbotrel commented Mar 2, 2022

Closes #269 .
This PR adds std/math/bits with ToBase FromBase (ToBinary ToTernary) and ToNAF circuit functions.

ToTernary and ToNAF generates some unnecessary constraints but will be fixed with #243 .

This PR keeps api.ToBinary (which calls bits.ToBinary ) since other part of api depends on binary decomposition (range checks).

@gbotrel gbotrel marked this pull request as draft March 2, 2022 18:06
@gbotrel gbotrel marked this pull request as ready for review March 8, 2022 15:38
@gbotrel gbotrel requested a review from ivokub March 8, 2022 15:38
@gbotrel gbotrel added this to the v0.7.0 milestone Mar 8, 2022
@gbotrel gbotrel linked an issue Mar 8, 2022 that may be closed by this pull request
@ivokub
Copy link
Collaborator

ivokub commented Mar 8, 2022

Suggested edit:

diff --git a/std/math/bits/conversion.go b/std/math/bits/conversion.go
index 609da117..4abc7264 100644
--- a/std/math/bits/conversion.go
+++ b/std/math/bits/conversion.go
@@ -6,15 +6,20 @@ import (
 	"github.com/consensys/gnark/frontend"
 )
 
+// Base defines the base for decomposing the scalar into digits.
 type Base uint8
 
 const (
-	Binary  Base = 2
+	// Binary base decomposes scalar into bits (0-1)
+	Binary Base = 2
+
+	// Ternary base decomposes scalar into trits (0-1-2)
 	Ternary Base = 3
 	Quinary Base = 5
 )
 
-// ToBase converts b in given base
+// ToBase decomposes scalar v into digits in given base using options opts. The
+// decomposition is in little-endian order.
 func ToBase(api frontend.API, base Base, v frontend.Variable, opts ...BaseConversionOption) []frontend.Variable {
 	switch base {
 	case Binary:
@@ -26,8 +31,9 @@ func ToBase(api frontend.API, base Base, v frontend.Variable, opts ...BaseConver
 	}
 }
 
-// FromBase compute from a set of digits its canonical representation
-// For example for base 2, it returns Σbi = Σ (2**i * b[i])
+// FromBase compute from a set of digits its canonical representation in
+// little-endian order.
+// For example for base 2, it returns Σbi = Σ (2**i * digits[i])
 func FromBase(api frontend.API, base Base, digits ...frontend.Variable) frontend.Variable {
 	if len(digits) == 0 {
 		panic("FromBase needs at least 1 digit")
@@ -47,10 +53,13 @@ type BaseConversionConfig struct {
 	Unconstrained bool
 }
 
+// BaseConversionOption configures the behaviour of scalar decomposition.
 type BaseConversionOption func(opt *BaseConversionConfig) error
 
-// WithNbDigits set the resulting number of digits to be used in the base conversion
-// nbDigits must be > 0
+// WithNbDigits set the resulting number of digits to be used in the base conversion.
+// nbDigits must be > 0. If nbDigits is lower than the length of full
+// decomposition, then nbDigits least significant digits are returned. If the
+// option is not set, then the full decomposition is returned.
 func WithNbDigits(nbDigits int) BaseConversionOption {
 	return func(opt *BaseConversionConfig) error {
 		if nbDigits <= 0 {
@@ -61,9 +70,9 @@ func WithNbDigits(nbDigits int) BaseConversionOption {
 	}
 }
 
-// WithUnconstrainedOutputs sets the bit conversion API to NOT constrain the output
+// WithUnconstrainedOutputs sets the bit conversion API to NOT constrain the output.
 // This is UNSAFE but is useful when the outputs are already constrained by other circuit
-// constraints
+// constraints.
 func WithUnconstrainedOutputs() BaseConversionOption {
 	return func(opt *BaseConversionConfig) error {
 		opt.Unconstrained = true
diff --git a/std/math/bits/conversion_binary.go b/std/math/bits/conversion_binary.go
index edcee4d3..f2840496 100644
--- a/std/math/bits/conversion_binary.go
+++ b/std/math/bits/conversion_binary.go
@@ -14,7 +14,8 @@ var (
 	// returns its i-th bit.
 	IthBit = hint.NewStaticHint(ithBit)
 
-	// NBits returns the n first bits of the input. Expects one argument: n.
+	// NBits returns the first bits of the input. The number of returned bits is
+	// defined by the length of the results slice.
 	NBits = hint.NewStaticHint(nBits)
 )
 
@@ -23,12 +24,12 @@ func init() {
 	hint.Register(NBits)
 }
 
-// ToBinary is an alias of ToBase(... Binary ...)
+// ToBinary is an alias of ToBase(api, Binary, v, opts)
 func ToBinary(api frontend.API, v frontend.Variable, opts ...BaseConversionOption) []frontend.Variable {
 	return ToBase(api, Binary, v, opts...)
 }
 
-// FromBinary is an alias of FromBase(... Binary ...)
+// FromBinary is an alias of FromBase(api, Binary, digits)
 func FromBinary(api frontend.API, digits ...frontend.Variable) frontend.Variable {
 	return FromBase(api, Binary, digits...)
 }
diff --git a/std/math/bits/conversion_ternary.go b/std/math/bits/conversion_ternary.go
index 2a5489c0..43ae04fd 100644
--- a/std/math/bits/conversion_ternary.go
+++ b/std/math/bits/conversion_ternary.go
@@ -9,18 +9,20 @@ import (
 	"github.com/consensys/gnark/frontend"
 )
 
+// NTrits returns the first trits of the input. The number of returned trits is
+// defined by the length of the results slice.
 var NTrits = hint.NewStaticHint(nTrits)
 
 func init() {
 	hint.Register(NTrits)
 }
 
-// ToTernary is an alias of ToBase(... Ternary ...)
+// ToTernary is an alias of ToBase(api, Ternary, v, opts...)
 func ToTernary(api frontend.API, v frontend.Variable, opts ...BaseConversionOption) []frontend.Variable {
 	return ToBase(api, Ternary, v, opts...)
 }
 
-// FromTernary is an alias of FromBase(... Ternary ...)
+// FromTernary is an alias of FromBase(api, Ternary, digits)
 func FromTernary(api frontend.API, digits ...frontend.Variable) frontend.Variable {
 	return FromBase(api, Ternary, digits...)
 }
@@ -96,7 +98,7 @@ func toTernary(api frontend.API, v frontend.Variable, opts ...BaseConversionOpti
 	return trits
 }
 
-// AssertIsTrit constrain digit to be 0, 1 or 2
+// AssertIsTrit constrains digit to be 0, 1 or 2.
 func AssertIsTrit(api frontend.API, v frontend.Variable) {
 	if c, ok := api.Compiler().ConstantValue(v); ok {
 		if c.IsUint64() && c.Uint64() <= 2 {
diff --git a/std/math/bits/naf.go b/std/math/bits/naf.go
index ba28670a..10187501 100644
--- a/std/math/bits/naf.go
+++ b/std/math/bits/naf.go
@@ -9,13 +9,15 @@ import (
 	"github.com/consensys/gnark/frontend"
 )
 
+// NNAF returns the NAF decomposition of the input. The number of digits is
+// defined by the number of elements in the results slice.
 var NNAF = hint.NewStaticHint(nNaf)
 
 func init() {
 	hint.Register(NNAF)
 }
 
-// ToNAF retuns the naf decomposition of given input
+// ToNAF retuns the NAF decomposition of given input.
 func ToNAF(api frontend.API, v frontend.Variable, opts ...BaseConversionOption) []frontend.Variable {
 	// parse options
 	cfg := BaseConversionConfig{

Copy link
Collaborator

@ivokub ivokub left a comment

Choose a reason for hiding this comment

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

Looks mostly good. Package documentation is also missing - maybe it would be good to add a few lines?

std/math/bits/conversion.go Outdated Show resolved Hide resolved
std/math/bits/conversion.go Outdated Show resolved Hide resolved
std/math/bits/conversion.go Show resolved Hide resolved
std/math/bits/conversion.go Outdated Show resolved Hide resolved
std/math/bits/naf.go Outdated Show resolved Hide resolved
test/assert.go Outdated Show resolved Hide resolved
@gbotrel gbotrel merged commit d9b9239 into develop Mar 8, 2022
@gbotrel gbotrel deleted the feat-math-bits branch March 8, 2022 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

implement in-circuit ToTernary
2 participants