Repository navigation
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f95b17c5aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Commit: 80fc0ff SP1 Execution Results
|
f95b17c to
46304ae
Compare
🔒 AI Security Review (claude-opus-5)✅ No security issues found. |
46304ae to
eff60a2
Compare
eff60a2 to
631a7b2
Compare
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🔒 AI Security Review (claude-opus-5)✅ No security issues found. |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
631a7b2 to
8f4d9e7
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f4d9e7062
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🔒 AI Security Review (claude-opus-5)✅ No security issues found. |
8f4d9e7 to
b229b63
Compare
d865ee5 to
df1a211
Compare
🔒 AI Security Review (claude-opus-5)✅ No security issues found. |
df1a211 to
2716717
Compare
🔒 AI Security Review (claude-opus-5-5)✅ No security issues found. |
2716717 to
f2880be
Compare
🔒 AI Security Review (claude-opus-5-5)✅ No security issues found. |
f2880be to
6112179
Compare
🔒 AI Security Review (claude-opus-5-5)✅ No security issues found. |
6112179 to
787b236
Compare
🔒 AI Security Review (claude-opus-5-5)✅ No security issues found. |
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 787b236b5b
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
787b236 to
d9243cf
Compare
🔒 AI Security Review (claude-opus-5-5)✅ No security issues found. |
d9243cf to
e381d78
Compare
🔒 AI Security Review (claude-opus-5-5)✅ No security issues found. |
|
@purusang @krsnapaudel this PR changed after your approvals, so it needs another look. The checkpoint guest's verifying key is derived from the guest ELF, and we're posting the key built from All of that is reverted, and the checks now live in the chain worker:
The two STF tests that covered the removed code are replaced by chain-worker tests. A new test checks that a block with a skipped slot, a wrong epoch, or a slot of 2^63 is rejected with no canonical read while its manifests aren't buried. #2300 and #2277 are restacked on top, and rejections there now carry The description has the guest-crate diff for the whole stack. The only change in a guest crate is the test module #2277 appends to |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e381d78b1d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
e381d78 to
d0072c7
Compare
🔒 AI Security Review (claude-opus-5-5)✅ No security issues found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0072c7f7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Closing in favor of #2388, a smaller fix for STR-4286 in the chain worker. |
Description
Validate every carried ASM manifest against buried canonical L1/ASM data before the STF buffers logs or persists state. All of the validation runs in the chain worker; this PR does not change any crate compiled into the checkpoint guest.
Before reading any canonical data, the worker rejects a header that doesn't continue its parent, and checks that manifest heights continue consecutively from the parent state's last L1 height over the whole range. The height check reports the same
AsmManifestHeightMismatchandAsmManifestHeightOverflowerrors the STF raises per manifest. A block that fails either check is rejected right away instead of waiting on L1. The worker then requires each manifest to be buried (is_l1_reorg_safe) and compares its complete hash with the canonical manifest at that height, for terminal and nonterminal blocks.Missing or unburied manifests defer execution through the retry layer. A content mismatch against buried canonical data returns
WorkerError::ManifestContentMismatch, and FCM rejects the block and clears its sequencer watermark. Includes chain-worker regressions for forged logs, header and height checks before any canonical read, and canonical lookup boundaries.Type of Change
Notes to Reviewers
The checkpoint guest's verifying key is derived from the guest ELF, so this stack must not change non-test code in any crate the guest compiles. An earlier revision of this PR added an STF height preflight,
ol/stfdispatch wrappers, and anExecErrorvariant. Those are gone. The height check is a private helper inchain-worker/src/provenance.rs, and the mismatch is a chain-worker error. Input validation uses the existingsequencer::verify_block_structure, and the chain worker calls the existing publicstrata_ol_stf_v1::verify_header_continuitydirectly, sostrata-ol-stf-v1is now a regular dependency of the chain worker. NeitherCargo.lockchanges.The continuity check runs in
fetch_block_with_parent, right after the parent header loads. The worker derives the parent's commitment from the block's slot, so checking there means a block that skips a slot is rejected instead of failing as a missing pre-state. Before calling the shared verifier, the worker rejects any slot gap with checked arithmetic, because the verifier subtracts slots asi64and overflows at 2^63 and above.Guest-crate check for the whole stack, run against the 21 local packages in
provers/sp1/guest-checkpoint/Cargo.lock:The one hunk (
@@ -386,3 +386,82 @@, from #2277) appends a#[cfg(test)] mod testsafter the last line of the file and removes nothing. This PR and the other five change no guest crate.provers/sp1/guest-checkpoint/Cargo.lockhas no diff. The rootCargo.lockgains one line, from atracing-subscriberdev-dependency that #2277's tests use.Part 5 of 7 for STR-4286.
Merge order:
Is this PR addressing any specification, design doc or external reference document?
Checklist
AI assistance: Claude implemented the rework and wrote the PR text; Codex reviewed the diff.
Related Issues
STR-4286