feat: verify-stark guest library and SDK integration#2555
Conversation
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
| */ | ||
| assert_array_eq(&mut builder.when_first_row(), local.next_bytes, next.state); |
There was a problem hiding this comment.
ATTN: @shuklaayush @jonathanpwang. We have a ticket to change this, but wanted to note that an AIR is being changed
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
Claude finished @stephenh-axiom-xyz's task in 4m 0s —— View job Code Review: feat: verify-stark guest library and SDK integration
Overall this is a well-structured PR that cleanly separates the verify-stark circuit, guest library, and SDK integration across three logical stages. The abstraction boundaries are sensible (especially Medium: Trailing singleton
|
| Location | Context |
|---|---|
guest-libs/verify-stark/circuit/src/extension.rs:36 |
decode_from_bytes(...).unwrap() on proof bytes |
guest-libs/verify-stark/circuit/src/extension.rs:89 |
.last().unwrap() on transcript log |
crates/verify/src/deferral.rs:70 |
usize::try_from(...).unwrap() on label_to_index result |
crates/sdk/src/stdin.rs:83 |
decode_from_bytes(input).unwrap() in DeferralInput::into_inputs |
For the verifier path (crates/verify), panics on malformed input are denial-of-service vectors. Consider replacing with ? / ok_or(...) where feasible.
Low: TODO left in production code — hash function choice
guest-libs/verify-stark/circuit/src/extension.rs:96 and crates/sdk/src/prover/deferral/circuit.rs:93
// TODO[INT-6415]: hash slice, not compress
let input_commit = poseidon2_compress_with_capacity(ts_commit, cached_commit).0;Using compress vs hash_slice has different security properties (domain separation, variable-length input handling). This appears in two places. Flagging in case this should be resolved before merge.
Nit: #[cfg(all(test, feature = "cuda"))] for tests
guest-libs/verify-stark/circuit/src/lib.rs:42-43
#[cfg(all(test, feature = "cuda"))]
mod tests;Tests are gated behind the cuda feature. This means CI without CUDA will never run these tests. Is this intentional? The test file at guest-libs/verify-stark/circuit/src/tests.rs does use cfg_if to select CPU vs GPU provers, suggesting it could work without CUDA. Consider #[cfg(test)] to ensure CPU-mode test coverage.
Architecture: Clean design decisions
-
DeferralCircuitProvertrait — nice abstraction allowing external deferral circuits to plug into the continuations framework without circular dependencies. -
DeferralProof::Absentvariant — elegantly handles the case where no deferrals exist for a circuit index, allowing the aggregation tree to propagate computed PVs without generating dummy proofs. -
Merkle proof verification (
crates/verify/src/deferral.rs) — theskip_depthapproach with zero-padded proofs is clean and avoids special-casing at the aggregation level. -
Guest library (
guest-libs/verify-stark/guest/src/lib.rs) — minimal API surface (get_proof_output,verify_proof_output). Clean separation between guest-facing and host-facing code. -
End-to-end test (
crates/sdk/src/tests.rs::test_verify_stark_deferral) — thorough 10-step test covering the full flow from fibonacci proof through deferral to final verification.
Summary
| Severity | Count | Key Items |
|---|---|---|
| Medium | 3 | Trailing Absent depth mismatch; prove trait panics; EVM path ignores deferrals |
| Low-Medium | 2 | Misleading AIR comment; deferral flag check simplification |
| Low | 2 | Unwrap calls on untrusted data; TODO hash function choice |
| Nit | 1 | Tests gated behind cuda feature |
The most actionable items are the Absent depth tracking in reduce_def_round and adding a guard/assertion in the EVM prover path when deferrals are configured.
Note: The previous review's top finding (from_pk dropping def_hook_commit for leaf prover) appears to have been fixed — from_pk now correctly passes def_hook_commit through.
Resolves INT-6548, INT-6194, INT-6641.
High-Level Overview
This PR adds end-to-end
verify-starkdeferral support in three stages:verify-starkcircuit/prover code into a dedicated crate.verify-starkguest library and extension wiring.openvm-verify-stark-hostand the SDK.The branch also includes follow-up correctness fixes (Merkle proof shape, output hashing alignment,
def_vk_commitsource) and updated tests.1) Move The Verify-Stark Circuit Crate
What changed
guest-libs/verify-stark/circuit(openvm-verify-stark-circuit).crates/continuations:crates/continuations/src/circuit/deferral/verify/*crates/continuations/src/prover/deferral/verify/*guest-libs/verify-stark/circuit/src/*(AIRs, tracegen, prover, tests).crates/continuationsrefactor to support the move:deferral/aggregation/{hook,inner}todeferral/{hook,inner}.DeferralCircuitProvertrait as the integration interface for external deferral circuits.circuit::utils,root::NUM_DIGESTS_IN_VK_COMMIT, etc.).Reviewer entry points
guest-libs/verify-stark/circuit/src/lib.rsguest-libs/verify-stark/circuit/src/prover/mod.rscrates/continuations/src/prover/deferral/mod.rscrates/continuations/src/prover/mod.rs2) Add Verify-Stark Guest Library + Extension Support
What changed
guest-libs/verify-stark/guest(openvm-verify-stark-guest).get_proof_outputverify_proof_outputverify_stark_deferral_fnget_raw_deferral_resultsget_deferral_stateopenvm-verify-starkpath dependency with:openvm-verify-stark-circuitopenvm-verify-stark-guestDeferralExtensionnow carriesdef_vk_commits(serialized, transpiler-facing).fnsremains runtime-only (serde(skip)).SdkVmConfignow supports optional deferral extension in transpiler + prover extension setup.DeferralStateandInputMapValderive serde traits.Reviewer entry points
guest-libs/verify-stark/guest/src/lib.rsguest-libs/verify-stark/circuit/src/extension.rsextensions/deferral/circuit/src/extension/mod.rscrates/sdk-config/src/lib.rscrates/vm/src/arch/deferral.rs3) Add Deferrals To openvm-verify-stark-host And SDK
Host-side verification (
crates/verify)NonRootStarkProofnow optionally includesdeferral_merkle_proofs(encoded/decoded with proof bytes).deferral.rsaddsDeferralMerkleProofsand path verification logic.VerificationBaselinenow includesexpected_def_vk_commit: Option<Digest>.verify_vm_stark_proof_decodednow enforces deferral invariants when deferrals are expected:VerifyStarkErrorwith deferral-specific failure modes.SDK proving flow (
crates/sdk)sdk/src/prover/deferral/mod.rssdk/src/prover/deferral/circuit.rssdk/src/prover/deferral/merkle.rsAggProversplit into VM/deferral/mixed operations:prove_vm,prove_def,prove_mixed, and proof-type-awarewrap_proof.GenericSdkandStarkProvernow support optional deferral path proving:GenericSdk::with_deferral_prover(...)prove(..., def_inputs: &[DeferralInput])deferral_merkle_proofsto producedNonRootStarkProofwhen deferrals are enabled.expected_def_vk_commitfor verifier-side checks.VersionedNonRootStarkProof) to carry optional deferral proof bytes.Reviewer entry points
crates/verify/src/lib.rscrates/verify/src/deferral.rscrates/sdk/src/prover/stark.rscrates/sdk/src/prover/agg.rscrates/sdk/src/prover/deferral/mod.rscrates/sdk/src/types.rsTest Coverage And Follow-Up Fixes
crates/sdk/src/tests.rs::test_verify_stark_deferralguest-libs/verify-stark/circuit/src/tests.rstests/verify.rsremoved.def_vk_commitbaseline source corrected to use aggregation prover DAG commits.