feat: inclusion proof program #115

tchataigner · 2024-07-22T16:51:50Z

This PR creates a storage inclusion program for our Ethereum Light Client. It comes with a benchmark and tests.

Tests

The unit tests can be found under ethereum/light-client/src/proofs/inclusion.rs. A basic PR will trigger an execution test while the tests for SNARK and STARK proving are to be manually triggered because of computational costs.

Benchmark

The benchmark for this program can be found in ethereum/light-client/benches/inclusion.rs. This benchmark can either be on the STARK or SNARK proof based on an environment variable MODE=<STARK | SNARK>.

The current number for this inclusion proof is:

{
  // Time in milliseconds, 7~ minutes
  "proving_time": 441123,
  "verification_time": 2
}

The benchmark can be ran with:

MODE=SNARK SHARD_BATCH_SIZE=0 SHARD_SIZE=4194304 RUSTFLAGS="-C target-cpu=native -C opt-level=3" cargo +nightly-2024-05-31 bench --bench inclusion

Changelog

Implemented the inclusion program to execute the following:
1. Check signature on the attested block in the latest finalized update
2. Verify the EIP1186 proof on the state root hash of the block finalized by the update
Created a StorageInclusionProver that handles everything around this program: execute, STARK/SNARK proving and verifying
Implemented the secondary server to run the proving and verifying
Implemented request forwarding from the primary server to the secondary one
A bit of Rust documentation and MdBook documentation

Related issues

Closes #70

storojs72 · 2024-07-23T12:23:40Z

ethereum/programs/committee-change/src/main.rs

-    // Commit the two hashes
-    sphinx_zkvm::io::commit(&old_sync_committee_hash.hash());
+    // Commit the signer hash, the current and next sync committee hashes, and the block height
+    sphinx_zkvm::io::commit(update.attested_header().beacon().slot());


Is this: update.attested_header().beacon().slot() correspond to a block height?

Yes, I have changed it to the finalized one now though

storojs72 · 2024-07-23T12:26:21Z

ethereum/programs/committee-change/src/main.rs

-    sphinx_zkvm::io::commit(&old_sync_committee_hash.hash());
+    // Commit the signer hash, the current and next sync committee hashes, and the block height
+    sphinx_zkvm::io::commit(update.attested_header().beacon().slot());
+    sphinx_zkvm::io::commit(&signer_sync_committee_hash.hash());


Is my understanding correct, that:

signer_sync_commitee_hash corresponds to H_29;

updated_sync_committee_hash corresponds to H_30;

next_sync_committee_hash corresponds to H_31;

according to the explanation from @huitseeker

wwared

Looks great! Main question is the last one about committing additional EIP1186 data in the inclusion program

ethereum/core/src/types/utils.rs

ethereum/programs/inclusion/src/main.rs

wwared · 2024-07-23T14:12:15Z

ethereum/programs/inclusion/src/main.rs

+    sphinx_zkvm::io::commit(
+        eip1186_proof
+            .key_hash()
+            .expect("EIP1186Proof::key_hash: could not get key hash")
+            .as_ref(),
+    );


Should we commit additional data related to the EIP1186 proof? Specifically I'm thinking of the value correspondingly associated with the specific key/storage/slot/address.

+1. We do commit merkle tree key / value in aptos case.

I've updated the program to have all the keys/values as outputs. I have removed what I had set previously to uniquely identify the proof, as it is redundant.

Co-authored-by: wwared <[email protected]>

huitseeker

Overall looks good to me, I've left a few comments inline.

huitseeker · 2024-07-23T14:20:01Z

ethereum/core/src/types/utils.rs

+    for (i, element) in list.iter().enumerate() {
+        if i < list.len() - 1 {
+            element_offset += element.len();
+            bytes.extend_from_slice(&(element_offset as u32).to_le_bytes());
+        }


Nit: I think you can avoid the enumerate.

let mut element_offset = (list.len() * OFFSET_BYTE_LENGTH) as u32; for element in list { bytes.extend_from_slice(&element_offset.to_le_bytes()); element_offset += element.len() as u32; serialized_elements.extend_from_slice(element); }

Makes sense!

huitseeker · 2024-07-23T14:23:44Z

ethereum/core/src/types/utils.rs

+        loop {
+            if cursor == first_offset {
+                break;
+            }


Could this be replaced by ?

while cursor < first_offset { ... }

huitseeker · 2024-07-23T14:26:16Z

ethereum/core/src/types/utils.rs

+                })?
+                .to_vec(),
+        );
+    } else if !bytes.is_empty() {


Could we negate this and front-load this condition?

if bytes.len() <= OFFSET_BYTE_LENGTH { if bytes.is_empty() { return Ok(list_bytes); } else { return Err(TypesError::UnderLength { structure: "StorageProof".into(), minimum: OFFSET_BYTE_LENGTH, actual: bytes.len(), }); } }

Yes, and it should improve readability 😄

huitseeker · 2024-07-23T15:23:42Z

ethereum/light-client/src/proofs/inclusion.rs

+    }
+
+    #[test]
+    #[ignore = "This test is too slow for CI"]


Could we open an issue to run this on a nightly basis? /cc @samuelburnham

Created #117

wwared

Looks great on my end

storojs72

LGTM

* feat: inclusion program * refactor: output correct committee hash + finalized slot * fix: fix tests after transition to finalized block height * refactor: apply suggestion Co-authored-by: wwared <[email protected]> * refactor: address reviews --------- Co-authored-by: wwared <[email protected]>

tchataigner force-pushed the feature/inclusion-proof-program branch from 09a05d9 to 0dc1ee6 Compare July 22, 2024 19:57

tchataigner marked this pull request as ready for review July 22, 2024 19:57

tchataigner force-pushed the feature/inclusion-proof-program branch 4 times, most recently from c816e56 to 2f4f6f4 Compare July 22, 2024 21:30

feat: inclusion program

f33b126

tchataigner force-pushed the feature/inclusion-proof-program branch from 2f4f6f4 to f33b126 Compare July 22, 2024 21:41

tchataigner requested review from huitseeker and wwared July 23, 2024 07:40

storojs72 self-requested a review July 23, 2024 07:54

storojs72 previously approved these changes Jul 23, 2024

View reviewed changes

refactor: output correct committee hash + finalized slot

f26dbca

tchataigner dismissed storojs72’s stale review via f26dbca July 23, 2024 12:49

storojs72 reviewed Jul 23, 2024

View reviewed changes

fix: fix tests after transition to finalized block height

39791ba

wwared previously approved these changes Jul 23, 2024

View reviewed changes

refactor: apply suggestion

e95b849

Co-authored-by: wwared <[email protected]>

tchataigner dismissed wwared’s stale review via e95b849 July 23, 2024 14:55

wwared previously approved these changes Jul 23, 2024

View reviewed changes

huitseeker reviewed Jul 23, 2024

View reviewed changes

tchataigner mentioned this pull request Jul 23, 2024

Run nightly benchmarks for Aptos & Ethereum #117

Open

refactor: address reviews

8bdbfc5

tchataigner dismissed wwared’s stale review via 8bdbfc5 July 23, 2024 22:44

tchataigner requested review from huitseeker, wwared and storojs72 July 23, 2024 22:44

wwared approved these changes Jul 23, 2024

View reviewed changes

storojs72 approved these changes Jul 24, 2024

View reviewed changes

tchataigner merged commit 968739a into dev Jul 24, 2024
6 checks passed

tchataigner deleted the feature/inclusion-proof-program branch July 24, 2024 07:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: inclusion proof program #115

feat: inclusion proof program #115

tchataigner commented Jul 22, 2024 •

edited

Loading

storojs72 Jul 23, 2024

tchataigner Jul 23, 2024 •

edited

Loading

storojs72 Jul 23, 2024

tchataigner Jul 23, 2024

wwared left a comment

wwared Jul 23, 2024

storojs72 Jul 23, 2024

tchataigner Jul 23, 2024

huitseeker left a comment

huitseeker Jul 23, 2024

tchataigner Jul 23, 2024

huitseeker Jul 23, 2024

huitseeker Jul 23, 2024

tchataigner Jul 23, 2024 •

edited

Loading

huitseeker Jul 23, 2024

tchataigner Jul 23, 2024

wwared left a comment

storojs72 left a comment

feat: inclusion proof program #115

feat: inclusion proof program #115

Conversation

tchataigner commented Jul 22, 2024 • edited Loading

Tests

Benchmark

Changelog

Related issues

Choose a reason for hiding this comment

tchataigner Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wwared left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huitseeker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tchataigner Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wwared left a comment

Choose a reason for hiding this comment

storojs72 left a comment

Choose a reason for hiding this comment

tchataigner commented Jul 22, 2024 •

edited

Loading

tchataigner Jul 23, 2024 •

edited

Loading

tchataigner Jul 23, 2024 •

edited

Loading