feat(stm): add missing off circuit checks - #3480
Conversation
Test Results 5 files ± 0 209 suites ±0 1h 45m 12s ⏱️ - 40m 24s For more details on these failures, see this check. Results for commit b082171. ± Comparison against base commit 58b575a. This pull request removes 51 and adds 8 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
8233f3e to
b082171
Compare
| pub(crate) fn finish_prepare<D: MembershipDigest>( | ||
| checks: IvcProverInputChecks, | ||
| certificate_proof: &SnarkProof<D>, | ||
| message: &[u8], | ||
| aggregate_verification_key_for_snark: &AggregateVerificationKeyForSnark<D>, | ||
| global: &Global, | ||
| protocol_message_preimage: &ProtocolMessagePreimage, | ||
| rolling_state: &IvcRollingState, | ||
| prover_setup: &IvcSnarkProverSetup, | ||
| ) -> StmResult<Self> { |
| pub(crate) fn finish_prepare<D: MembershipDigest>( | ||
| checks: IvcProverInputChecks, | ||
| certificate_proof: &SnarkProof<D>, | ||
| message: &[u8], | ||
| aggregate_verification_key_for_snark: &AggregateVerificationKeyForSnark<D>, | ||
| global: &Global, | ||
| protocol_message_preimage: &ProtocolMessagePreimage, | ||
| rolling_state: &IvcRollingState, | ||
| prover_setup: &IvcSnarkProverSetup, | ||
| ) -> StmResult<Self> { |
hjeljeli32
left a comment
There was a problem hiding this comment.
Good initial work 👍 . I left 2 comments.
| genesis_bootstrap.genesis_signature, | ||
| &combined_fixed_base_names, | ||
| ); | ||
| prepare_genesis_checks(&genesis_rolling_state, &protocol_message_preimage, &global)?; |
There was a problem hiding this comment.
@damrobi I think the genesis check uses the wrong preimage. If I understood well, protocol_message_preimage is the preimage of the current certificate. I think we should pass &genesis_bootstrap.genesis_protocol_message_preimage
|
|
||
| match current_rolling_state { | ||
| Some(rolling_state) => { | ||
| rolling_state.assert_protocol_parameters_unchanged()?; |
There was a problem hiding this comment.
Would it make sense to move this check into IvcProverInput::prepare_checks? Keeping it here means direct IvcProver::prove / IvcProverInput::prepare callers bypass this invariant.
Content
This PR includes the addition of some missing off circuit checks. The goal of those checks is to prevent a prover from trying to create a proof that will fail to verify since the proof creation is a very expensive operations.
Changes
protocol_parameters == next_protocol_parametersfor the values coming from the rolling state. An update of the parameters would break the recursive proof.Pre-submit checklist
Comments
Issue(s)
Closes #3381