Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Add method to verify the prestate in the execution witness #44

Merged
merged 9 commits into from
May 14, 2024

Conversation

kevaundray
Copy link
Collaborator

This adds a method that verifies the proof in the execution witness.

It takes in:

  • The prestate root as a string in hex, with 0x prefixed
  • The execution witness as a string, exactly as it is in the block.

package.json Outdated Show resolved Hide resolved
@@ -26,13 +27,17 @@ export const loadVerkleCrypto = async (): Promise<VerkleCrypto> => {
): Commitment =>
updateCommitmentBase(verkleFFI, commitment, commitmentIndex, oldScalarValue, newScalarValue)

const verifyExecutionWitnessPreState = (prestateRoot: string, execution_witness_json: string): boolean =>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is essentially doing nothing, wasn't sure if you wanted to keep this so it follows the same pattern as everything else. Will remove if so

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference between this function and what is being done in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

The difference is that my ethereumjs PR is verifying that the prestate is minimal (i.e. it contains no extra data in the witness), whereas this verifies and proves that the prestate belongs the the parent state root.

src.rs/Cargo.toml Outdated Show resolved Hide resolved
@kevaundray kevaundray marked this pull request as ready for review May 13, 2024 19:45
Copy link
Contributor

@gabrocheleau gabrocheleau left a comment

Choose a reason for hiding this comment

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

Thank you very much for this @kevaundray !

I've modified the test data so that it taps into existing kaustinen6 blocks. The proof verification works with one of the test blocks, but not with one of the larger ones (~100 txs and a massive witness). I've commented out that second one for now.

@gabrocheleau gabrocheleau merged commit 4f60f7c into master May 14, 2024
3 checks passed
@holgerd77 holgerd77 deleted the kw/verify-proof branch May 14, 2024 15:35
@holgerd77
Copy link
Member

but not with one of the larger ones (~100 txs and a massive witness)

@gabrocheleau what happens on the larger ones? Timing out, or a concrete error message (in that case, can you post)?

@gabrocheleau
Copy link
Contributor

but not with one of the larger ones (~100 txs and a massive witness)

@gabrocheleau what happens on the larger ones? Timing out, or a concrete error message (in that case, can you post)?

here is the log:

 FAIL  src.ts/tests/ffi.spec.ts > bindings > verifyExecutionProof: block with many txs
RuntimeError: unreachable
 ❯ __rust_start_panic wasm:/wasm/00264a8e:1:483948
 ❯ rust_panic wasm:/wasm/00264a8e:1:483012
 ❯ std::panicking::rust_panic_with_hook::hbf46ef0245cc9589 wasm:/wasm/00264a8e:1:453971
 ❯ std::panicking::begin_panic_handler::{{closure}}::hc07db454214d2c87 wasm:/wasm/00264a8e:1:465026
 ❯ std::sys_common::backtrace::__rust_end_short_backtrace::hf9e2f055fb5ef672 wasm:/wasm/00264a8e:1:483791
 ❯ rust_begin_unwind wasm:/wasm/00264a8e:1:474051
 ❯ core::panicking::panic_fmt::h14c85a61aa3d538e wasm:/wasm/00264a8e:1:477599
 ❯ core::option::expect_failed::h2966ad4c676148ac wasm:/wasm/00264a8e:1:474179
 ❯ verkle_trie::proof::VerkleProof::check::h8b40c80b7f75ebef wasm:/wasm/00264a8e:1:28591
 ❯ ffi_interface::verify_execution_witness::heca47ad411dad6cd wasm:/wasm/00264a8e:1:356435

From Kev: it is failing due to an invariant failing

We can still make progress on the stateless client front with what we have here, but this will need fine-tuning in order to properly verify all blocks. (I believe Kev is looking into this). The current version will at least give us some API to interact with and test.

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.

None yet

4 participants