feat(e2e): enhance the SNARK test with a prover warmup - #3533
Draft
jpraynaud wants to merge 5 commits into
Draft
Conversation
The dual bundles are handed out when a Lagrange era is run, the runner is built with the SNARK feature and the node versions accept them, legacy keys otherwise.
The prover setup is materialized in the background when the node starts, so the first signing round does not pay it inside its aggregation.
The circuit keys are no longer derived at every aggregation and are materialized before the signing round, so the epochs no longer have to absorb the setup.
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Enhances nightly SNARK e2e scenarios with prover warmup, dual genesis keys, and a unified CI matrix.
Changes:
- Adds background SNARK setup warming.
- Selects compatible legacy or dual genesis keys.
- Enables SNARK scenarios with shorter epochs and shared build arguments.
File summaries
| File | Description |
|---|---|
| mithril-test-lab/mithril-end-to-end/src/stress_test/aggregator_helpers.rs | Updated as part of this pull request. |
| mithril-test-lab/mithril-end-to-end/src/mithril/mod.rs | Updated as part of this pull request. |
| mithril-test-lab/mithril-end-to-end/src/mithril/infrastructure.rs | Updated as part of this pull request. |
| mithril-test-lab/mithril-end-to-end/src/mithril/genesis_keys.rs | Updated as part of this pull request. |
| mithril-test-lab/mithril-end-to-end/src/mithril/client.rs | Updated as part of this pull request. |
| mithril-test-lab/mithril-end-to-end/src/mithril/aggregator.rs | Updated as part of this pull request. |
| mithril-test-lab/mithril-end-to-end/src/main.rs | Updated as part of this pull request. |
| mithril-stm/src/proof_system/snark_setup_warmer.rs | Updated as part of this pull request. |
| mithril-stm/src/proof_system/mod.rs | Updated as part of this pull request. |
| mithril-stm/src/lib.rs | Updated as part of this pull request. |
| mithril-common/src/crypto_helper/mod.rs | Updated as part of this pull request. |
| mithril-aggregator/src/commands/serve_command.rs | Updated as part of this pull request. |
| .github/workflows/test-e2e.yml | Updated as part of this pull request. |
Review details
Suppressed comments (3)
mithril-stm/src/proof_system/snark_setup_warmer.rs:33
IvcSnarkaggregation callssnark_aggregate_signature_proverfirst and thenivc_chain_prover, but this branch only warmsivc_setup. The first IVC aggregation therefore still materializes the certificate SNARK setup on the signing path, so the warmup does not cover the full cold-start cost. Warm the certificate setup here as well before the IVC setup.
AggregateSignatureType::IvcSnark => {
SnarkProverSetupReuse::Enabled.ivc_setup(parameters)?;
}
mithril-test-lab/mithril-end-to-end/src/mithril/aggregator.rs:50
- Because this public config field is newly introduced, it needs a doc comment under the repository's public-API documentation rule. Without one, the new
genesis_keysfield is undocumented while being exposed as part ofAggregatorConfig.
pub genesis_keys: GenesisKeys,
mithril-test-lab/mithril-end-to-end/src/mithril/infrastructure.rs:55
- Because this public config field is newly introduced, it needs a doc comment under the repository's public-API documentation rule. Without one, this struct is missing documentation for the new
genesis_keysfield.
pub genesis_keys: GenesisKeys,
- Files reviewed: 13/13 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+19
to
+26
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub struct GenesisKeys { | ||
| /// Hex encoded genesis verification key | ||
| pub verification_key: &'static str, | ||
|
|
||
| /// Hex encoded genesis secret key | ||
| pub secret_key: &'static str, | ||
| } |
Test Results 5 files 221 suites 34m 3s ⏱️ Results for commit 23698b1. |
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
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.
Content
This PR includes the changes that enhance the e2e nightly SNARK scenarios, by warming up the prover setup before the first signing round:
minimal-snarkfrom 2400 to 600 slots with a 60 s run intervalminimal-ivc-snarkfrom 2400 to 900 slots with a 120 s run interval.Measured on the nightly workflow (run 34484111857):
minimal-snarkminimal-ivc-snarkPre-submit checklist
Issue(s)
Closes #3390