Always prove to produce proofs for BatchProofs #1540

kpp · 2024-11-29T12:34:27Z

Description

We run every seqcom on l1 block in execute mode (no proving but producing fake receipts).
So we have to implement producing fake proof for all batch proofs but with sampling to produce a real proof once a day.

In order to do that there is a new proving mode: prove-with-fakes.

Linked Issues

Testing

I tested locally for:

index e4bb86e5..37228afa 100644
--- a/bin/citrea/tests/bitcoin_e2e/light_client_test.rs
+++ b/bin/citrea/tests/bitcoin_e2e/light_client_test.rs
@@ -45,6 +45,7 @@ impl TestCase for LightClientProvingTest {
     fn batch_prover_config() -> BatchProverConfig {
         BatchProverConfig {
             enable_recovery: false,
+            proof_sampling_number: 999999999usize,
             ..Default::default()
         }
     }
@@ -52,6 +53,7 @@ impl TestCase for LightClientProvingTest {
     fn light_client_prover_config() -> LightClientProverConfig {
         LightClientProverConfig {
             enable_recovery: false,
+            proof_sampling_number: 0,
             ..Default::default()
         }
     }
@@ -453,7 +455,7 @@ impl TestCase for LightClientProvingTestMultipleProofs {
 }

So light client proving for fake batch proofs works.

eyusufatik

explained in DMs

eyusufatik

sorry I misunderstood

crates/sovereign-sdk/full-node/db/sov-db/src/ledger_db/mod.rs

crates/prover-services/src/lib.rs

crates/prover-services/src/parallel/mod.rs

codecov · 2024-12-03T19:14:51Z

Codecov Report

Attention: Patch coverage is 27.65957% with 34 lines in your changes missing coverage. Please review.

Project coverage is 75.3%. Comparing base (4710eac) to head (5783dfa).

Files with missing lines	Patch %	Lines
crates/prover-services/src/parallel/mod.rs	16.6%	20 Missing ⚠️
crates/prover-services/src/lib.rs	44.4%	5 Missing ⚠️
bin/citrea/src/rollup/bitcoin.rs	0.0%	3 Missing ⚠️
bin/citrea/src/rollup/mock.rs	0.0%	3 Missing ⚠️
bin/citrea/src/rollup/mod.rs	50.0%	1 Missing ⚠️
crates/batch-prover/src/da_block_handler.rs	80.0%	1 Missing ⚠️
...full-node/sov-stf-runner/src/prover_service/mod.rs	0.0%	1 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
...reign-sdk/full-node/db/sov-db/src/ledger_db/mod.rs	`71.8% <ø> (-0.5%)`	⬇️
...ule-system/sov-modules-rollup-blueprint/src/lib.rs	`95.8% <ø> (ø)`
bin/citrea/src/rollup/mod.rs	`74.6% <50.0%> (-0.2%)`	⬇️
crates/batch-prover/src/da_block_handler.rs	`78.1% <80.0%> (+0.2%)`	⬆️
...full-node/sov-stf-runner/src/prover_service/mod.rs	`61.5% <0.0%> (-5.2%)`	⬇️
bin/citrea/src/rollup/bitcoin.rs	`0.0% <0.0%> (ø)`
bin/citrea/src/rollup/mock.rs	`58.2% <0.0%> (-0.5%)`	⬇️
crates/prover-services/src/lib.rs	`44.4% <44.4%> (ø)`
crates/prover-services/src/parallel/mod.rs	`67.2% <16.6%> (-6.9%)`	⬇️

... and 4 files with indirect coverage changes

eyusufatik

are you sure light client proofs with work with fake proofs and real proofs? (running in dev mode ofc)

kpp · 2024-12-04T19:12:36Z

Yes. Before we added a new proving mode prove-with-fakes I tested it with:

index e4bb86e5..37228afa 100644
--- a/bin/citrea/tests/bitcoin_e2e/light_client_test.rs
+++ b/bin/citrea/tests/bitcoin_e2e/light_client_test.rs
@@ -45,6 +45,7 @@ impl TestCase for LightClientProvingTest {
     fn batch_prover_config() -> BatchProverConfig {
         BatchProverConfig {
             enable_recovery: false,
+            proof_sampling_number: 999999999usize,
             ..Default::default()
         }
     }
@@ -52,6 +53,7 @@ impl TestCase for LightClientProvingTest {
     fn light_client_prover_config() -> LightClientProverConfig {
         LightClientProverConfig {
             enable_recovery: false,
+            proof_sampling_number: 0,
             ..Default::default()
         }
     }
@@ -453,7 +455,7 @@ impl TestCase for LightClientProvingTestMultipleProofs {
 }

I can't do it now with prover-with-fakes though, because BatchProverConfig is in citrea-e2e repo.

crates/sovereign-sdk/full-node/sov-stf-runner/src/prover_service/mod.rs

yaziciahmet · 2024-12-05T12:18:06Z

I don't quite understand the motivation behind this PR. As far as I observed:

if prove specified, we don’t call prove-l1 at all unless random sampling satisfies
if prove-with-fakes specified, we always call prove-l1, but we randomly decide in prover-service to generate either fake or real proofs in a per-proof basis

What is the effect of this to fullnodes? How will fullnodes know whether the proof is fake or not when verifying if some of them can be fake and some of them can be real?

How will this be used in conjunction with the RISC0_DEV_MODE env variable?

What does this bring us in general?

eyusufatik · 2024-12-06T09:32:36Z

I don't quite understand the motivation behind this PR. As far as I observed:

if prove specified, we don’t call prove-l1 at all unless random sampling satisfies

if prove-with-fakes specified, we always call prove-l1, but we randomly decide in prover-service to generate either fake or real proofs in a per-proof basis

What is the effect of this to fullnodes? How will fullnodes know whether the proof is fake or not when verifying if some of them can be fake and some of them can be real?

How will this be used in conjunction with the RISC0_DEV_MODE env variable?

What does this bring us in general?

we get to test da consumption and gather logs on testnet without any additional costs.

looks like our testnet zk aproach will be based on this

yaziciahmet · 2024-12-06T12:19:54Z

crates/prover-services/src/parallel/mod.rs

+        ProofGenMode::ProveWithSampling => {
+            // `make_proof` is called with a probability in this case.
+            // When it's called, we have to produce a real proof.
+            vm.run(elf, true)
+        }
+        ProofGenMode::ProveWithSamplingWithFakeProofs(proof_sampling_number) => {


Only nit I have is that ProveWithSampling is a bit confusing to me, because in the eyes of ProverService it is just Prove, and sampling happens above in the call stack in da_block_handler. So i think we should just call them Prove and ProveWithSampling respectively, so its obvious from the ProofGenMode whether ProverService should do any sampling itself.

I am not going to block the PR for this nit, but would love to see this change.

Also, what if instead of sampling on per proof basis for ProveWithSamplingWithFakeProofs, we sample per DA block. Meaning, we again decide the sampling on da_block_handler level, but what we decide is to whether proofs in current DA should be run in execute or prove mode. It would simplify the flow quite a bit. Just throwing this out as a thought. WDYT?

We talked with Esad, the approach doesn't matter much in this PR. We may open a new issue as a followup

kpp added 2 commits November 29, 2024 00:28

Refactor ProofGenMode

1ec12c2

Always prove, even if it's a fake proof

9c1520f

auto-assign bot requested review from ercecan, eyusufatik, jfldde, rakanalh and yaziciahmet November 29, 2024 12:34

auto-assign bot assigned kpp Nov 29, 2024

kpp changed the title ~~Kpp/always prove batch~~ Always prove to produce proofs for BatchProofs Nov 29, 2024

Merge branch 'nightly' into kpp/always_prove_batch

ecfd7be

eyusufatik requested changes Dec 2, 2024

View reviewed changes

eyusufatik approved these changes Dec 2, 2024

View reviewed changes

kpp added 2 commits December 2, 2024 23:54

Don't add duplicate SequencerCommitment on prove_l1

34fcc8b

Merge branch 'nightly' into kpp/always_prove_batch

dffa000

kpp commented Dec 2, 2024

View reviewed changes

crates/sovereign-sdk/full-node/db/sov-db/src/ledger_db/mod.rs Show resolved Hide resolved

yaziciahmet requested changes Dec 3, 2024

View reviewed changes

crates/prover-services/src/lib.rs Show resolved Hide resolved

crates/prover-services/src/parallel/mod.rs Outdated Show resolved Hide resolved

crates/prover-services/src/parallel/mod.rs Outdated Show resolved Hide resolved

kpp added 3 commits December 3, 2024 16:36

Merge branch 'nightly' into kpp/always_prove_batch

0468e6c

Add a different proving mode

26f8178

Review fixes

6a7eb77

Merge branch 'nightly' into kpp/always_prove_batch

22bf47c

eyusufatik approved these changes Dec 4, 2024

View reviewed changes

yaziciahmet reviewed Dec 5, 2024

View reviewed changes

crates/sovereign-sdk/full-node/sov-stf-runner/src/prover_service/mod.rs Show resolved Hide resolved

yaziciahmet approved these changes Dec 6, 2024

View reviewed changes

Merge branch 'nightly' into kpp/always_prove_batch

5783dfa

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Always prove to produce proofs for BatchProofs #1540

Always prove to produce proofs for BatchProofs #1540

kpp commented Nov 29, 2024 •

edited

Loading

eyusufatik left a comment

eyusufatik left a comment

codecov bot commented Dec 3, 2024 •

edited

Loading

eyusufatik left a comment

kpp commented Dec 4, 2024

yaziciahmet commented Dec 5, 2024 •

edited

Loading

eyusufatik commented Dec 6, 2024

yaziciahmet Dec 6, 2024 •

edited

Loading

yaziciahmet Dec 6, 2024

kpp Dec 6, 2024

Always prove to produce proofs for BatchProofs #1540

Are you sure you want to change the base?

Always prove to produce proofs for BatchProofs #1540

Conversation

kpp commented Nov 29, 2024 • edited Loading

Description

Linked Issues

Testing

eyusufatik left a comment

Choose a reason for hiding this comment

eyusufatik left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 3, 2024 • edited Loading

Codecov Report

eyusufatik left a comment

Choose a reason for hiding this comment

kpp commented Dec 4, 2024

yaziciahmet commented Dec 5, 2024 • edited Loading

eyusufatik commented Dec 6, 2024

yaziciahmet Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

yaziciahmet Dec 6, 2024

Choose a reason for hiding this comment

kpp Dec 6, 2024

Choose a reason for hiding this comment

kpp commented Nov 29, 2024 •

edited

Loading

codecov bot commented Dec 3, 2024 •

edited

Loading

yaziciahmet commented Dec 5, 2024 •

edited

Loading

yaziciahmet Dec 6, 2024 •

edited

Loading