refactor(prover): Remove redundant and unsafe utility functions#3273
Open
Tabaie wants to merge 3 commits into
Open
refactor(prover): Remove redundant and unsafe utility functions#3273Tabaie wants to merge 3 commits into
Tabaie wants to merge 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3273 +/- ##
=========================================
Coverage 55.36% 55.36%
Complexity 5247 5247
=========================================
Files 1126 1126
Lines 44644 44644
Branches 5356 5356
=========================================
Hits 24719 24719
Misses 19189 19189
Partials 736 736
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
gusiri
previously approved these changes
Jun 4, 2026
gusiri
approved these changes
Jun 4, 2026
AlexandreBelling
approved these changes
Jun 5, 2026
Contributor
AlexandreBelling
left a comment
There was a problem hiding this comment.
Approved on the basis that this should reduce the bug surface of the codebase.
Contributor
|
Let's wait before the performances and bug fixes release is wrapped before merging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes some utility functions in the BLS12-377 Keccak prover that already exist in the root project. In particular removes the unsound and unused
ToBytesfunction. Changes also include eliminating anything pertaining to an extension field in the BLS12-377 context.Note
Low Risk
Mechanical import migration and removal of duplicate/unused helpers; main behavioral risk is subtle API differences in disjoint-set/collection usage in module discovery, which should be covered by existing tests.
Overview
This PR consolidates shared helpers by pointing the BLS12-377 Keccak prover at the monorepo-wide
prover/utilspackage instead of the nested.../keccak/prover/utilscopy. The diff is almost entirely import rewrites across circuits, crypto, maths, protocol, and zkevm code; behavior should stay the same for symbols that already lived in both places.Tests and discovery:
TestReduceBytesis dropped fromcircuits/internal/utils_test.goalong with its BN254/random-byte setup—aligned with dropping duplicate or unsafe byte-reduction helpers from the local utils tree (e.g. unusedToBytes). In distributed module discovery, disjoint-set usage moves toprover/utils/collection(NewDisjointSetFromList,Ds.Parentinstead ofDs.Rank), while keccak-localutils/collectionSettypes are kept under akcollectionalias.Keccak-specific subpackages (
utils/parallel,utils/arena,utils/gnarkutil,utils/types, etc.) are unchanged in this diff.Reviewed by Cursor Bugbot for commit 83defd8. Bugbot is set up for automated code reviews on this repo. Configure here.