ci(l1,l2): add required check for outdated Cargo.lock #5722
Conversation
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
tomip01
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new CI check to the L1 workflow to ensure that Cargo.lock files for zkVM backends and TEE quote-gen remain up-to-date in the main branch. This prevents issues that can occur when dependencies are added to crates that are dependencies of zkVM backends, but the corresponding Cargo.lock files aren't updated.
Key changes:
- Adds
check-guest-program-cargo-lockjob that runsmake update-cargo-lockand verifies no diffs exist in the Cargo.lock files - Integrates this new job as a dependency of the
all-testsrequired check job
Comments suppressed due to low confidence (1)
.github/workflows/pr-main_l1.yaml:350
- The
all-testsjob's conditional logic and success check are incomplete. The job was updated to depend oncheck-guest-program-cargo-lock, but:
- The
ifcondition at line 338 doesn't check ifcheck-guest-program-cargo-lockwas skipped (unlike the checks forrun-assertoorandrun-hive) - The steps section (lines 340-350) doesn't verify that
check-guest-program-cargo-locksucceeded
This means if check-guest-program-cargo-lock fails, the all-tests job could still pass, defeating the purpose of adding this required check. Add a condition like && needs.check-guest-program-cargo-lock.result != 'skipped' to line 338, and add a success check in the steps section similar to the ones for other jobs.
needs: [detect-changes, run-assertoor, run-hive, check-guest-program-cargo-lock]
# Make sure this job runs even if the previous jobs failed or were skipped
if: ${{ needs.detect-changes.outputs.run_tests == 'true' && always() && needs.run-assertoor.result != 'skipped' && needs.run-hive.result != 'skipped' }}
steps:
- name: Check if any job failed
run: |
if [ "${{ needs.run-assertoor.result }}" != "success" ]; then
echo "Job Assertoor Tx Check failed"
exit 1
fi
if [ "${{ needs.run-hive.result }}" != "success" ]; then
echo "Job Hive failed"
exit 1
fi
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Oppen
reviewed
Dec 23, 2025
.github/workflows/pr-main_l1.yaml
Outdated
Comment on lines
320
to
331
| - name: Update Cargo.lock files | ||
| run: | | ||
| make update-cargo-lock | ||
| - name: Check Cargo.lock files are committed | ||
| run: | | ||
| git diff --exit-code -- \ | ||
| crates/l2/prover/src/guest_program/src/sp1/Cargo.lock \ | ||
| crates/l2/prover/src/guest_program/src/risc0/Cargo.lock \ | ||
| crates/l2/prover/src/guest_program/src/zisk/Cargo.lock \ | ||
| crates/l2/prover/src/guest_program/src/openvm/Cargo.lock \ | ||
| crates/l2/tee/quote-gen/Cargo.lock |
Contributor
There was a problem hiding this comment.
cargo check --locked already does this.
Contributor
Author
There was a problem hiding this comment.
It is a bit slower though
tomip01
force-pushed
the
add_requiered_cargo_lock_check
branch
from
022819e to
96eb2b6
Compare
tomip01
requested review from
ManuelBilbao,
avilagaston9 and
ilitteri
as code owners
ilitteri
approved these changes
Dec 23, 2025
Benchmark Results ComparisonNo significant difference was registered for any benchmark run. Detailed ResultsBenchmark Results: BubbleSort
Benchmark Results: ERC20Approval
Benchmark Results: ERC20Mint
Benchmark Results: ERC20Transfer
Benchmark Results: Factorial
Benchmark Results: FactorialRecursive
Benchmark Results: Fibonacci
Benchmark Results: FibonacciRecursive
Benchmark Results: ManyHashes
Benchmark Results: MstoreBench
Benchmark Results: Push
Benchmark Results: SstoreBench_no_opt
|
Oppen
approved these changes
Jan 5, 2026
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.
Motivation
We don't want to have in main outdated
Cargo.locks. This can happen if a dependency is added in a crate that is a dependency of any zkVM backendDescription
update-cargo-lockcheck-cargo-lockmake check-cargo-lockon every PR to ensure allCargo.locks are up to date.You can check a test run here