-
Notifications
You must be signed in to change notification settings - Fork 130
fix(tracer): decouple reported line count from actual trace height in arithmetic modules (#1955) #1959
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: rdin777 <[email protected]>
Signed-off-by: rdin777 <[email protected]>
Signed-off-by: rdin777 <[email protected]>
|
|
tracer/arithmetization/src/main/java/net/consensys/linea/zktracer/module/mul/MulOperation.java
Outdated
Show resolved
Hide resolved
Signed-off-by: rdin777 <[email protected]>
Signed-off-by: rdin777 <[email protected]>
Signed-off-by: rdin777 <[email protected]>
tracer/arithmetization/src/main/java/net/consensys/linea/zktracer/module/mul/MulOperation.java
Show resolved
Hide resolved
Signed-off-by: rdin777 <[email protected]>
Signed-off-by: rdin777 <[email protected]>
Signed-off-by: rdin777 <[email protected]>
|
"All automated checks are now passing. I have addressed the feedback from the bot by: Translating all comments to English and fixing indentation. Replacing the silent default case in computeLineCount() with an IllegalStateException to ensure consistency with the trace() method and prevent silent sizing errors. The PR is ready for human review. Looking forward to your feedback!" |
Description (Описание)
Overview
This PR addresses the issue of "silent errors" in .1t trace files where the reported module height was incorrectly tied to raw operation counts instead of actual trace expansion. By refactoring how row counts are computed, we ensure that the tracer accurately reports its height to the generator, preventing the insertion of null rows that lead to constraint failures.
Changes
MulOperation.java: Replaced the hardcoded return 1 in computeLineCount() with a dynamic switch based on opCode. This allows for future expansion of complex multiplication or exponentiation instructions.
ExpOperation.java: Refactored computeLineCount() to return height based on the specific expInstruction (EXPLOG vs MODEXPLOG), providing a foundation for the upcoming "small fields" update.
Mul.java / Exp.java: These modules now correctly propagate the expanded height through operations.lineCount(), ensuring that columnHeaders and commit phases use synchronized dimensions.
Impact
Resolves the mismatch described in Issue #1955.
Fixes the "silent null rows" bug in .1t file generation.
Simplifies the integration of the upcoming "small fields" refactoring by DavePearce.
Fixes #1955.
Note
Compute line counts via opcode/instruction-specific logic in
EucOperation,ExpOperation, andMulOperation(currently 1 row each), with strict validation for unexpected cases.tracer/.../euc/EucOperation.java:computeLineCount()override returning1.tracer/.../exp/ExpOperation.java:computeLineCount()toswitchonexpCall.expInstruction(); throws on unexpected cases.tracer/.../mul/MulOperation.java:computeLineCount()toswitchonopCode; throws on unexpected cases.Written by Cursor Bugbot for commit 46442d7. This will update automatically on new commits. Configure here.