Skip to content

Verify Denim proof parity across native SP1 and Nitro execution - #4583

Draft
0x00101010 wants to merge 1 commit into
200ms/denim-nitro-parityfrom
200ms/denim-proof-backend-parity
Draft

Verify Denim proof parity across native SP1 and Nitro execution#4583
0x00101010 wants to merge 1 commit into
200ms/denim-nitro-parityfrom
200ms/denim-proof-backend-parity

Conversation

@0x00101010

Copy link
Copy Markdown
Contributor

Summary

  • add an explicit CPU compressed-proof and verification test for the frozen Denim fixture
  • compare the compressed proof's public values byte-for-byte with the native expected commitments
  • keep the presubmit SP1 command limited to deterministic local ELF execution
  • expose the expensive compressed cycle through a manually dispatched release-gate job

Verification

  • just fix
  • just succinct test-denim-range
  • cargo test --locked -p base-proof-zk-backend --test denim_sp1 --no-run

The compressed cycle is intentionally not a presubmit check. Before release sign-off, dispatch the Denim SP1 workflow or run just succinct prove-denim-range; both generate and verify the real compressed proof.

Stack

Depends on #4582 (which depends on #4581).

@github-actions

Copy link
Copy Markdown
Contributor

Review: No findings

This PR adds a compressed-proof generation and verification test for the Denim fixture, gated behind #[ignore] and a manually-dispatched CI job. Changes are limited to test infrastructure (test file, workflow, just recipe) and don't touch production code paths.

Reviewed:

  • denim_sp1.rs: New proves_and_verifies_compressed_denim_fixture test — error handling via ?/anyhow::Result is clean, manual tokio runtime for the async expected_public_values() call is a reasonable pattern given the rest of the test is blocking CPU prover work.
  • denim-sp1.yml: workflow_dispatch trigger added; compressed job correctly guarded with if: github.event_name == 'workflow_dispatch'; 360-minute timeout is appropriate for compressed proving.
  • succinct.just: test-denim-range split into per-test invocations and new prove-denim-range recipe — both correctly reference the test names and pass --ignored.

No correctness, safety, or architectural concerns.

@github-actions

Copy link
Copy Markdown
Contributor

Base Std historical fork tests

Fork Result Passed Failed Skipped base/base base-anvil base-std
Beryl pass 616 0 13 69360e1a 8d0f5b8a 4658f1b7
Cobalt pass 721 0 14 69360e1a 9df661bc e30b3421

View run

@github-actions

Copy link
Copy Markdown
Contributor

Caution

This PR may regress performance. 1 benchmark(s) slower by more than 10% beyond the noise band: execution/Insertion - 65,536 nodes (+10.6%).

Benchmark results (advisory)

Median time on the PR head versus the base branch, measured on the same host. Wall-clock, so a change is only flagged when it clears ±10% and the confidence intervals do not overlap. Only benchmarks past the ±10% threshold (plus new or dropped ones) are listed. This check never blocks a merge.

Benchmark Base Head Δ median
execution/Insertion - 65,536 nodes 32.74 ms 36.21 ms +10.6% ⚠️ slower
execution/Open 1024 nodes - 4096 nodes 38.24 µs 46.45 µs +21.5% · within noise
execution/Open 1024 nodes - 65,536 nodes 72.93 µs 47.85 µs -34.4% · within noise

46 benchmark(s) within ±10% omitted.

View run · Re-run benchmarks

@cb-heimdall

Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants