feat(arithmetization): make the arithmetization directory as Go package - #3867
Conversation
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
There was a problem hiding this comment.
Pull request overview
This PR turns arithmetization/ into a standalone Go module that (1) embeds the R5 interpreter sources and (2) standardizes all local/CI usage of zkc to run via the Go tool pinned in arithmetization/go.mod, reducing drift for downstream users.
Changes:
- Added a Go module under
arithmetization/and introduced a Go package that embeds the R5 interpreter sources and exposes aCompiledBinaryFile(...)helper. - Reworked arithmetization test tooling/Make targets to use
go tool zkc(and a newprepare-zkctarget) instead of installingzkcontoPATH. - Updated GitHub Actions workflows/actions and docs/scripts to align with the module-pinned
zkctooling and Go version bump.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| arithmetization/src/test/zkc/poseidon2/Makefile | Switches test execution to go tool zkc. |
| arithmetization/src/test/scripts/weekly_zkc_metrics.sh | Updates script commentary to reflect new zkc workflow. |
| arithmetization/src/test/scripts/README-weekly-metrics.md | Updates weekly metrics docs to use module-pinned zkc and new make targets. |
| arithmetization/src/test/scripts/keccak_zkc_vs_reference_speedup/main.go | Updates invocation examples to use go -C arithmetization run .... |
| arithmetization/src/test/README.md | Updates test docs for module-pinned zkc usage. |
| arithmetization/src/test/Makefile | Replaces install-zkc flow with prepare-zkc and defaults to go -C ... tool zkc. |
| arithmetization/src/embedded.go | Adds embedded FS for the R5 interpreter sources (src.MainDir). |
| arithmetization/README.md | Documents zkc as a Go tool pinned by arithmetization/go.mod. |
| arithmetization/Makefile | Replaces repo-clone/install logic with prepare-zkc based on Go tool pin + optional override. |
| arithmetization/gopkg/embedded/embedded.go | Adds embedded package that compiles embedded sources into a constraints binary file. |
| arithmetization/gopkg/embedded/embedded_test.go | Adds a basic compile smoke test for CompiledBinaryFile. |
| arithmetization/go.sum | Adds Go module dependency checksums for arithmetization module. |
| arithmetization/go.mod | Introduces arithmetization Go module and pins zkc via tool directive. |
| .github/workflows/riscv-guests-zkc-interpreter-run.yml | Updates workflow to Go 1.25.7 and prepare-zkc usage. |
| .github/workflows/arithmetization-zkc-riscv-check-compilation.yml | Updates inputs/docs and cache behavior for go.mod-pinned vs overridden zkc. |
| .github/workflows/arithmetization-weekly-zkc-metrics.yml | Bumps Go version and adjusts comments for ref resolution. |
| .github/workflows/arithmetization-riscv-act4-test.yml | Updates inputs/docs and cache-save rules for pinned vs overridden zkc. |
| .github/workflows/arithmetization-keccak-zkc-vs-reference-benchmark.yml | Updates Go version and migrates benchmark to module-pinned zkc. |
| .github/workflows/arithmetization-guest-programs-trace.yml | Updates inputs/docs and removes explicit “install zkc” step assumptions. |
| .github/workflows/arithmetization-guest-programs-exec.yml | Updates inputs/docs and removes explicit “install zkc” step assumptions. |
| .github/workflows/arithmetization-benchmark-zkc-native-keccak.yml | Uses pinned zkc module version as default ref and trims trailing noise. |
| .github/workflows/arithmetization-benchmark-zkc-interpreter.yml | Uses pinned zkc module version as default ref and trims trailing noise. |
| .github/actions/setup-zkc-measurement/action.yml | Switches setup action to “prepare” module-pinned zkc and export tool info. |
| .github/actions/setup-arithmetization-riscv/action.yml | Updates optional zkc override handling + Go version bump. |
Suppressed comments (2)
arithmetization/gopkg/embedded/embedded.go:77
- Same as above: the WalkDir callback should check the incoming "err" before using d/path, otherwise errors can be masked or turn into panics.
if err := fs.WalkDir(subFs, ".", func(path string, d fs.DirEntry, err error) error {
// skip predecoding directory, as it contains files that are not part of
// the main R5 interpreter source code.
if d.IsDir() && path == predecodingDir {
return fs.SkipDir
}
if d.IsDir() {
return nil
}
arithmetization/src/test/README.md:344
- The example command includes
-q, which is documented as removed from zkc elsewhere in this repository. Removing it avoids failures when rerunning ACT4 tests manually.
go tool zkc exec -q act4/bin/logs/<test-name>.json ../../main/riscv/main.zkc
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
Previously missed (3) — in code that hasn't changed since the last review.
arithmetization/src/test/Makefile:249
- In
vector-execbatched mode, invoking$(MAKE) zkc-execreruns theprepare-zkcphony target (andgo mod download) in a nested make, which is unnecessary overhead. Sincevector-execalready depends onprepare-zkc, call$(ZKC) execdirectly here.
This issue also appears on line 267 of the same file.
$(MAKE) --no-print-directory zkc-exec ZKC_REF= JSON=$(VECTOR_JSON_FILE) > /dev/null 2> "$$err"; \
arithmetization/src/test/Makefile:453
act4-execruns many JSONs and currently shells out to a nestedmake zkc-execper test, which re-runs the phonyprepare-zkc(andgo mod download) each time. Sinceact4-execalready depends onprepare-zkc, call$(ZKC) execdirectly in the loop to avoid repeated Go module work.
$(MAKE) --no-print-directory zkc-exec ZKC_REF= JSON="$$json" > /dev/null 2> "$$base.err"; \
arithmetization/src/test/Makefile:6
elf-traceis defined in this Makefile but is missing from the.PHONYlist, so a file namedelf-tracecould prevent the target from running.
.PHONY: exec debug compile elf-exec elf-debug prepare-zkc zkc-exec zkc-trace zkc-debug clean clean-all vector-build vector-json vector-exec keccak-rust-build keccak-rust-json keccak-rust-exec keccak-zig-build keccak-zig-json keccak-zig-exec blake-rust-build blake-rust-json blake-rust-exec act4-build act4-exec require-test require-src require-vector-file require-n-vectors require-vector-build-artifacts require-vector-json-mode require-json
arithmetization/src/test/README.md:220
- This README table still documents
ZKC_EXEC_FLAGSdefaulting to--fast -q, but the Makefile now setsZKC_EXEC_FLAGS ?=--fast(and other docs note-qno longer exists). This is misleading for users overriding flags.
| `ZKC_EXEC_FLAGS` | `--fast -q` | Flags passed to `zkc exec` within `zkc-exec` and `elf-exec` |
arithmetization/src/test/Makefile:267
- In
vector-execper-vector mode, each loop iteration invokes a nestedmake zkc-exec, which re-triggersprepare-zkc(andgo mod download) because it is phony. This can drastically slow large vector runs; call$(ZKC) execdirectly inside the loop instead.
$(MAKE) --no-print-directory zkc-exec ZKC_REF= JSON="$$json" > /dev/null 2> "$$err"; \
Signed-off-by: Ivo Kubjas <ivo.kubjas@consensys.net>
Currently, the arithmetization provides only the R5 interpreter sources. So when downstream users in prover-ray defines a Go test (see https://github.com/LFDT-Lineth/lineth-monorepo/blob/main/prover-ray/zkcdriver/r5_test.go) pointing to arithmetization, then it may drift as it has a local zkc dependency pointing to a previous version.
This PR now starts embedding the R5 interpreter sources and defines a getter
github.com/LFDT-Lineth/lineth-monorepo/arithmetization/gopkg/embedded.CompiledBinaryFile()so that we get the compiled R5 interpreter compiled using the zkc version defined inarithmetization/go.mod. When arithmetization updates the minimal requires zkc version there then it will be recursively picked up when we update thegithub.com/LFDT-Lineth/lineth-monorepo/arithmetizationimport in prover ray.Additionally - now instead of having a custom Make target for downloading and installing zkc, we just use the
go tool zkccommand instead which uses the version pinned inarithmetization/go.mod. It simplifies the process a bit for developers as we remove one additional checkout/build step so that Go handles it automatically.Also, this PR should allow us to prepare to include the predecoding of R5 binaries and
elf-to-jsonintoarithmetizationdirectory, where it imo belongs. Right now theelf-to-jsonbehaviour is duplicated betweenarithmetization/test/scripts/elf-to-jsonandprover-ray/backend/zkc_r5(where the latter now broke aself-to-jsonin arithmetization does predecoding as well).This PR depends on LFDT-Lineth/zkc#2193, so don't merge before that one is.Checklist
PR.