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

fix: fixes #1157 ensures calls to AttachDebugInfo are surrounded with… #1160

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

gbotrel
Copy link
Collaborator

@gbotrel gbotrel commented Jun 10, 2024

Fixes #1157

@gbotrel gbotrel requested a review from ivokub June 10, 2024 19:59
@ivokub
Copy link
Collaborator

ivokub commented Jun 11, 2024

diff --git a/frontend/cs/scs/api_assertions.go b/frontend/cs/scs/api_assertions.go
index 1b59b3d3..56ba0221 100644
--- a/frontend/cs/scs/api_assertions.go
+++ b/frontend/cs/scs/api_assertions.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"math/big"
 
+	"github.com/consensys/gnark/constraint"
 	"github.com/consensys/gnark/debug"
 	"github.com/consensys/gnark/frontend"
 	"github.com/consensys/gnark/frontend/internal/expr"
@@ -182,8 +183,10 @@ func (builder *builder) AssertIsLessOrEqual(v frontend.Variable, bound frontend.
 }
 
 func (builder *builder) mustBeLessOrEqVar(a frontend.Variable, bound expr.Term) {
-
-	debugInfo := builder.newDebugInfo("mustBeLessOrEq", a, " <= ", bound)
+	var debugInfo []constraint.DebugInfo
+	if debug.Debug {
+		debugInfo = []constraint.DebugInfo{builder.newDebugInfo("mustBeLessOrEq", a, " <= ", bound)}
+	}
 
 	nbBits := builder.cs.FieldBitLen()
 
@@ -216,34 +219,17 @@ func (builder *builder) mustBeLessOrEqVar(a frontend.Variable, bound expr.Term)
 		if ai, ok := builder.constantValue(aBits[i]); ok {
 			// a is constant; ensure l == 0
 			l.Coeff = builder.cs.Mul(l.Coeff, ai)
-			if debug.Debug {
-				builder.addPlonkConstraint(sparseR1C{
-					xa: l.VID,
-					qL: l.Coeff,
-				}, debugInfo)
-			} else {
-				builder.addPlonkConstraint(sparseR1C{
-					xa: l.VID,
-					qL: l.Coeff,
-				})
-			}
-
+			builder.addPlonkConstraint(sparseR1C{
+				xa: l.VID,
+				qL: l.Coeff,
+			}, debugInfo...)
 		} else {
 			// l * a[i] == 0
-			if debug.Debug {
-				builder.addPlonkConstraint(sparseR1C{
-					xa: l.VID,
-					xb: aBits[i].(expr.Term).VID,
-					qM: l.Coeff,
-				}, debugInfo)
-			} else {
-				builder.addPlonkConstraint(sparseR1C{
-					xa: l.VID,
-					xb: aBits[i].(expr.Term).VID,
-					qM: l.Coeff,
-				})
-			}
-
+			builder.addPlonkConstraint(sparseR1C{
+				xa: l.VID,
+				xb: aBits[i].(expr.Term).VID,
+				qM: l.Coeff,
+			}, debugInfo...)
 		}
 
 	}
@@ -272,7 +258,10 @@ func (builder *builder) MustBeLessOrEqCst(aBits []frontend.Variable, bound *big.
 	}
 
 	// debugInfo info
-	debugInfo := builder.newDebugInfo("mustBeLessOrEq", aForDebug, " <= ", bound)
+	var debugInfo []constraint.DebugInfo
+	if debug.Debug {
+		debugInfo = []constraint.DebugInfo{builder.newDebugInfo("mustBeLessOrEq", aForDebug, " <= ", bound)}
+	}
 
 	// t trailing bits in the bound
 	t := 0
@@ -302,20 +291,11 @@ func (builder *builder) MustBeLessOrEqCst(aBits []frontend.Variable, bound *big.
 			l := builder.Sub(1, p[i+1], aBits[i]).(expr.Term)
 			//l = builder.Sub(l, ).(term)
 
-			if debug.Debug {
-				builder.addPlonkConstraint(sparseR1C{
-					xa: l.VID,
-					xb: aBits[i].(expr.Term).VID,
-					qM: builder.tOne,
-				}, debugInfo)
-			} else {
-				builder.addPlonkConstraint(sparseR1C{
-					xa: l.VID,
-					xb: aBits[i].(expr.Term).VID,
-					qM: builder.tOne,
-				})
-			}
-
+			builder.addPlonkConstraint(sparseR1C{
+				xa: l.VID,
+				xb: aBits[i].(expr.Term).VID,
+				qM: builder.tOne,
+			}, debugInfo...)
 		} else {
 			builder.AssertIsBoolean(aBits[i])
 		}

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 good to me. Have a look at alternative way to avoid initializing debuginfo when not in debug mode and having single builder.AddPlonkConstraint call regardless of debug tag.

@gbotrel gbotrel merged commit 9452701 into master Jun 11, 2024
7 checks passed
@gbotrel gbotrel deleted the fix/issue1157 branch June 11, 2024 15:14
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.

bug: AttachDebugInfo() contributes to most memory usage regardless of debug.Debug flag value
2 participants