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

Automatic JSONs generation for e2e Pasta flow #38

Conversation

artem-bakuta
Copy link

@artem-bakuta artem-bakuta commented Feb 2, 2024

Partially fixes lurk-lab/solidity-verifier#24

Added creating and verifying keys for e2e contract from pasta branch. Created separate test case for generating proof and public parameters JSON files.

To run the script, lib.rs should include those serialisation changes.

Access to script is here

btw: In future in can be modified to be used with Grumpkin.

To run the scrypt use:

python3 VkCompressionSnarkVerifier.py {repo_url} {git_commit_sha} {anvil_node_url} {anvil_node_private_key}

Example:

python3 VkCompressionSnarkVerifier.py https://github.com/artem-bakuta/Nova.git 24563368378592c41ddb551238b593863f7b136b  http://127.0.0.1:8545 0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80

For generating json files use next inside this pull request:

cargo +nightly test solidity_compatibility_e2e_pasta --release

@storojs72
Copy link

storojs72 commented Feb 2, 2024

The missing link to script is here: https://gist.github.com/artem-bakuta/143c0c759cab99596e44ef7086b2b6bc

@artem-bakuta
Copy link
Author

I guess the link to VkCompressionSnarkVerifier.py script is missing. Can you add it?

My fault, resolved

@storojs72 storojs72 self-requested a review February 2, 2024 18:04
@storojs72 storojs72 changed the title Solidity key verifier for Pasta Automatic JSONs generation for e2e Pasta flow Feb 5, 2024
Copy link

@storojs72 storojs72 left a comment

Choose a reason for hiding this comment

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

Hi, @artem-bakuta!

Thanks for you work, I totally support this direction! Enforcing Rust <-> Solidity compatibility is extremely important thing. I have run the script and achieved the automatic generation of proof / public parameters JSONs with expected hashes:

artemstorozhuk@Artems-MacBook-Pro solidity-verifier % md5 pp-compressed-snark.json 
MD5 (pp-compressed-snark.json) = a6e009c674842f71f716abf1a9cc149f
artemstorozhuk@Artems-MacBook-Pro solidity-verifier % md5 pp-verifier-key.json    
MD5 (pp-verifier-key.json) = f4ea64d05738661da50f5cedb8cb82b9
artemstorozhuk@Artems-MacBook-Pro solidity-verifier % md5 verify_cache/nova/compressed-snark.json 
MD5 (verify_cache/nova/compressed-snark.json) = a6e009c674842f71f716abf1a9cc149f
artemstorozhuk@Artems-MacBook-Pro solidity-verifier % md5 verify_cache/nova/vk.json              
MD5 (verify_cache/nova/vk.json) = f4ea64d05738661da50f5cedb8cb82b9

I have couple of remarks, though:

  • we don't need loader.py in this repository. The intention of that loader is only taking your JSONs and feeding them into the deployed contract. So it's place is in solidity-verifier repository;
  • The test we use for JSONs generating is only for a technical reasons here, I would change it little bit (see details in below comments).

For integrity, I believe, this PR should have some companion one to pasta branch of solidity-verifier, where we can commit the script (with some changes obviously), removing hardcoded JSONs from there and also changes to CI configuration

src/lib.rs Outdated

// SAVE_GENERATED_KEYS_TO_JSON=true cargo +nightly test test_ivc_nontrivial_with_compression_pasta --release -- --nocapture
#[test]
fn test_ivc_nontrivial_with_compression_pasta() {
Copy link

@storojs72 storojs72 Feb 5, 2024

Choose a reason for hiding this comment

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

I would change the name to something like "solidity_compatibility_e2e_pasta"

Copy link
Author

Choose a reason for hiding this comment

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

will do

src/lib.rs Outdated
#[test]
fn test_ivc_nontrivial_with_compression_pasta() {
//get boolean args from commandline to generate keys to json
let generate_keys_to_json = std::env::var("SAVE_GENERATED_KEYS_TO_JSON").unwrap_or_default() == "true";

Choose a reason for hiding this comment

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

I would not use the separate ENVVAR for it - the intention of this test is just generation of JSONs, and it is not necessary to run it on CI, that is why you can use test_ivc_nontrivial_with_compression_with::<G1, G2>(true); explicitly and make this test as ignored (in your script you can easily run the ignored tests via cargo test -- --ignored)

Copy link
Author

Choose a reason for hiding this comment

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

Good notice will change

src/lib.rs Outdated
@@ -1,7 +1,5 @@
//! This library implements Nova, a high-speed recursive SNARK.
#![deny(
warnings,

Choose a reason for hiding this comment

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

can't we return back these lint checks?

Copy link
Author

Choose a reason for hiding this comment

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

for some reasons my rust compiler fails with this, will look for workaround

@artem-bakuta
Copy link
Author

Updated lib.rs and Python script

Copy link

@storojs72 storojs72 left a comment

Choose a reason for hiding this comment

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

We don't need loader.py in this repository. It's place in solidity-verifier

@@ -1477,12 +1479,10 @@ mod tests {

// SAVE_GENERATED_KEYS_TO_JSON=true cargo +nightly test test_ivc_nontrivial_with_compression_pasta --release -- --nocapture
#[test]

Choose a reason for hiding this comment

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

Add ignore, please:

#[test]
#[ignore]
fn solidity_compatibility_e2e_pasta() {
. . .

src/lib.rs Outdated
@@ -1469,4 +1476,13 @@ mod tests {
test_ivc_base_with::<G1, G2>();
test_ivc_base_with::<bn256::Point, grumpkin::Point>();
}

// SAVE_GENERATED_KEYS_TO_JSON=true cargo +nightly test test_ivc_nontrivial_with_compression_pasta --release -- --nocapture

Choose a reason for hiding this comment

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

I think this comment should be just:

cargo +nightly test test_ivc_nontrivial_with_compression_pasta --release -- --nocapture --ignored

src/lib.rs Outdated
@@ -1141,11 +1147,12 @@ mod tests {

#[test]
fn test_ivc_nontrivial_with_compression() {
//get boolean args from commandline to generate keys to json

Choose a reason for hiding this comment

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

do we need this comment?

Copy link

@storojs72 storojs72 left a comment

Choose a reason for hiding this comment

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

LGTM now! Thanks for your work!

@storojs72 storojs72 merged commit d5f5fb5 into lurk-lang:solidity-verifier-pp-spartan Feb 7, 2024
@artem-bakuta artem-bakuta deleted the solidity-verifier-pp-spartan-keys_verifier branch February 7, 2024 13:43
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.

3 participants