Makefile cleanup + recursion test guard fix (follow-ups to #742)#749
Merged
Conversation
- Collapse the three byte-identical guest-ELF pattern recipes (rust, bench, recursion) into a single `build_guest_elf` canned recipe parameterized by source dir ($1) and built-binary name suffix ($2). The cargo invocation was identical across all three; only the directory and the `-bench` copy suffix differed. Verified with `make -n` that the rust/bench rules still copy `release/<stem>` and the recursion rule copies `release/<stem>-bench`. - Clarify the compile-programs NOTE: the recursion smoke tests are #[ignore]d (not run by `make test`/`test-executor`, only `test-prover-all`), but their guest ELFs are still compiled on every build so they keep building. - recursion_smoke_test: the pre-check asserted `blob.len() < MAX_PRIVATE_INPUT_SIZE` while the executor rejects only `len > MAX` (memory.rs:228), so a blob exactly at the cap is accepted by the VM but would have failed the test guard. Use `<=`.
Oppen
approved these changes
Jun 30, 2026
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
2 participants
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.
Small follow-up cleanups on top of #742 (
pr/recursion-guest), from reviewing that PR. Targets the PR branch so it can fold into #742 before it merges.1. Dedupe the three guest-ELF Makefile recipes
The
rust,bench, and newrecursion%.elfpattern recipes had a byte-identicalcargo buildinvocation — they differed only in the source directory and whether the copied binary has a-benchsuffix. Collapsed them into one canned recipe:$(1)= source dir,$(2)= binary-name suffix (empty for rust/bench,-benchfor the recursion suite).make -nthat the expansion is identical to before: rust/bench rules copyrelease/<stem>, recursion copiesrelease/<stem>-bench, and thecd && cargo buildstays a single shell command.$(call)s.2. Clarify the
compile-programsNOTEThe old comment said the recursion ELFs are "temporarily unused by the main tests," which reads oddly now that
compile-programsbuilds them on everymake test. Reworded to: the recursion smoke tests are#[ignore]d (not run by the main test targets, onlytest-prover-all), but their ELFs are still compiled on every build so they keep building.3. Fix an off-by-one in the recursion test's input-size guard
run_recursion_pipeline_with_optionsassertedblob.len() < MAX_PRIVATE_INPUT_SIZE, but the executor rejects onlylen > MAX(executor/src/vm/memory.rs:228) — a blob exactly at the cap is accepted by the VM but would have failed the test guard. Changed to<=to match the executor's actual boundary. (Harmless today, test-only.)No behavior change to the build output or the proving path.