Skip to content

fix: use proper predicate keys derived from proof backend verifying keys#70

Open
prajwolrg wants to merge 6 commits intomainfrom
STR-2931-proper-predicate
Open

fix: use proper predicate keys derived from proof backend verifying keys#70
prajwolrg wants to merge 6 commits intomainfrom
STR-2931-proper-predicate

Conversation

@prajwolrg
Copy link
Copy Markdown
Collaborator

@prajwolrg prajwolrg commented Apr 9, 2026

Description

  • Replace hardcoded PredicateKey::always_accept() with real predicate keys derived from each proof host's verifying key (SP1Groth16 for sp1 builds, Bip340Schnorr for native builds)
  • Introduce ProofBackend struct that centralizes zkvm-specific host construction and predicate key resolution behind feature gates, replacing scattered #[cfg] blocks in bootstrap
  • Update to latest moho types (StepMohoAttestation, StepMohoProof, RecursiveMohoProof) and bump moho dependency to 18d515b

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor
  • New or updated tests
  • Dependency update
  • Security fix

Notes to Reviewers

This is built on top of alpenlabs/moho#79.

Checklist

  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation if needed.
  • My changes do not introduce new warnings.
  • I have added tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.

Related Issues

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

Commit: 5237e04
SP1 Execution Results

program cycles success
ASM STF 118,483,841 yes
Moho Recursive 11,586,163 yes

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 9, 2026

Codecov Report

❌ Patch coverage is 73.52941% with 18 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
bin/asm-runner/src/prover/backend.rs 57.14% 18 Missing ⚠️
Files with missing lines Coverage Δ
bin/asm-runner/src/bootstrap.rs 97.82% <100.00%> (+6.61%) ⬆️
bin/asm-runner/src/main.rs 97.50% <ø> (ø)
bin/asm-runner/src/prover/input.rs 99.31% <100.00%> (+<0.01%) ⬆️
crates/proof/statements/src/program.rs 84.44% <100.00%> (ø)
bin/asm-runner/src/prover/backend.rs 57.14% <57.14%> (ø)

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@prajwolrg prajwolrg force-pushed the STR-2931-proper-predicate branch from 248e411 to 23f956f Compare April 10, 2026 02:05
@prajwolrg prajwolrg changed the title test fix: use appropriate predicate key Apr 10, 2026
@prajwolrg prajwolrg self-assigned this Apr 10, 2026
@prajwolrg prajwolrg changed the title fix: use appropriate predicate key fix: use proper predicate keys derived from proof backend verifying keys Apr 10, 2026
@prajwolrg prajwolrg marked this pull request as ready for review April 10, 2026 10:33
moho-runtime-impl = { git = "https://github.com/alpenlabs/moho", rev = "28ef75cf39758c4214cb383a4de2b2dcf0a3dd67" }
moho-runtime-interface = { git = "https://github.com/alpenlabs/moho", rev = "28ef75cf39758c4214cb383a4de2b2dcf0a3dd67" }
moho-types = { git = "https://github.com/alpenlabs/moho", rev = "28ef75cf39758c4214cb383a4de2b2dcf0a3dd67" }
moho-recursive-proof = { git = "https://github.com/alpenlabs/moho", rev = "18d515b3ff60b1f48e4070d0b4b9a5e7af82f99b" }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical: guest-moho still pins the old moho revision — causes SP1 CI failure

The workspace moho deps were bumped here to 18d515b, but guest-builder/sp1/guest-moho/Cargo.toml line 9 still references the old revision 28ef75c.

The host-side code now serializes inputs using the new moho types (StepMohoAttestation, StepMohoProof, RecursiveMohoProof), but the guest program deserializes them with the old types — causing an OffsetOutOfBounds SSZ deserialization panic inside the SP1 guest. This is the direct cause of the failing prover-perf (SP1) CI check.

# guest-builder/sp1/guest-moho/Cargo.toml line 9 — needs:
moho-recursive-proof = { git = "https://github.com/alpenlabs/moho", rev = "18d515b3ff60b1f48e4070d0b4b9a5e7af82f99b" }

The corresponding guest-builder/sp1/guest-moho/Cargo.lock will also need to be regenerated.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Thanks! Fixed in b35148e.

pub(crate) asm_predicate: PredicateKey,
pub(crate) moho_predicate: PredicateKey,
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Consider consuming ProofBackend via pattern destructuring at the call site instead of field-by-field access. This makes the intent clearer and prevents accidental partial moves:

let ProofBackend { asm_host, moho_host, asm_predicate, moho_predicate } = ProofBackend::new()?;

Minor — the current pub(crate) field approach works fine.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check 2c5fd26.

/// [`ProofBackend::new`] and consumed by the proof orchestrator (hosts) and
/// the input builder (predicates).
pub(crate) struct ProofBackend {
pub(crate) asm_host: ProofHost,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Consider adding #[derive(Debug)] here (if ProofHost implements Debug). The workspace lints warn on missing_debug_implementations, and while pub(crate) structs are likely exempt, the derive would be consistent with the project's conventions and useful for logging/diagnostics during startup.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check 2c5fd26.

@prajwolrg prajwolrg requested a review from storopoli April 12, 2026 13:34
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