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

update sp1 and risc0 #1307

Closed
wants to merge 46 commits into from
Closed

update sp1 and risc0 #1307

wants to merge 46 commits into from

Conversation

PatStiles
Copy link
Contributor

@PatStiles PatStiles commented Oct 22, 2024

Update SP1 and Risc0 verifier versions

closes #1205

This PR:

  • Bumps the risc0 and sp1 verifier versions of operators.
  • Provides retro-compatibility for each version. If proof verification for SP1 and Risc0 between the current and old ('_old') verifier versions fails within the operator it will try verifying using the older version of SP1/Risc0 before returning false.
  • Changes the prompt library in the quiz example from dialoguer -> inquire to accomodate a dependency conflict.

To Test:

Verify network bindings works on a local network:

  • make test_risc_zero_go_bindings_macos
  • make test_sp1_go_bindings_macos
  • run local testnet and confirm you can submit sp1 and risc0 proofs. via
    make batcher_send_risc0_task
    make batcher_send_sp1_task
  • confirm examples work:
    cd examples/zkquiz && make answer_quix
    cd examples/validating-public-input && make generate_risc_zero_fibonacci_proof

Verify Retro Compatiblility of Operator Verifiers:

To verify operator retro-compatibility we deploy the rest of infrastructure of Aligned and boot the retro-compatible operator. This tests that network retro-compatible operator can verify SP1/Risc0 proofs from both verifier versions.

Note:
Commands suffixed with _old indicate the deprecated version of the SP1/Risc0 verifier that is currently running on testnet

To test this have two repos of aligned_layer one set to testnet and one to 1205-bump-sp1-and-risc0-version.

In testnet repo:

go clean
go mod tidy
make go_deps
make deps
  • Then you must run anvil, aggregator and batcher:
make anvil_start_with_block_time
make aggregator_start
make batcher_start_local

In 1205-bump-sp1-and-risc0-version repo:

make deps
  • Then you can run operator
make operator_register_and_start
  • Send proof from the older SP1 and Risc0 versions from the testnet repo.
make batcher_send_burst_groth16
make batcher_send_sp1_burst
make batcher_send_risc0_burst

Observe the Risc0 proof verification failed. and SP1 proof verification failed. logs are emitted and the operator successfully verifying the proofs from the old SP1/Risc0 verifiers. If you send a proof from the latest version of SP1/Risc0 you should notice its verification fails in the batcher.

  • Keep the operator running on the 1205-bump-sp1-and-risc0-version repo.
  • In the testnet repo, kill the batcher.
  • Navigate to the 1205-bump-sp1-and-risc0-version and start the batcher within the repo.
  • Send proof from both the SP1 and Risc0 versions from the 1205-bump-sp1-and-risc0-version repo.
make batcher_send_sp1_burst
make batcher_send_risc0_burst

Observe the operator successfully verifying the proofs from the new SP1/Risc0 verifiers. If you send a proof from the older version of SP1/Risc0 you should notice its verification fails in the batcher.

Type of change

Please delete options that are not relevant.

  • New feature

Checklist

  • Linked to Github Issue
  • This change depends on code or research by an external entity
    • Acknowledgements were updated to give credit
  • Unit tests added
  • This change requires new documentation.
    • Documentation has been added/updated.
  • This change is an Optimization
    • Benchmarks added/run
  • Has a known issue

Copy link
Collaborator

@MarcosNicolau MarcosNicolau left a comment

Choose a reason for hiding this comment

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

Worked for me!

Makefile Outdated
@@ -17,7 +17,7 @@ ifeq ($(OS),Darwin)
endif

ifeq ($(OS),Linux)
LD_LIBRARY_PATH += $(CURDIR)/operator/risc_zero/lib
export LD_LIBRARY_PATH+=$(CURDIR)/operator/risc_zero_old/lib:$(CURDIR)/operator/risc_zero/lib
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not have the export keyword

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not? Isn't the point to have this on the environment for child processes?

Copy link
Contributor

Choose a reason for hiding this comment

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

we had a bug related to this previosly which was solved in #1299. Maybe the problem was another?

Comment on lines +1 to +2
lib/librisc_zero_verifier_ffi.so
lib/librisc_zero_verifier.dylib
Copy link
Contributor

Choose a reason for hiding this comment

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

were these changes necessary?

Copy link
Contributor Author

@PatStiles PatStiles Oct 23, 2024

Choose a reason for hiding this comment

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

I removed the _ffi suffix initially as I found it redundant. When compiling on linux the linker was unable to find the shared library for risc0 without having the _ffi suffix added which leads to the difference in naming between the .so and .dylib. I was not sure why we initially were ignoring the .a file so I changed to ignoring the .so. I don't have a strong opinion on the naming for the macos shared library.

@Oppen
Copy link
Collaborator

Oppen commented Oct 23, 2024

I believe there might be missing steps in the testing procedure:

Welcome to the zkQuiz! Answer questions, generate a zkProof, and claim your NFT!
error: the following required arguments were not provided:
  --keystore-path <KEYSTORE_PATH>
  --verifier-contract-address <VERIFIER_CONTRACT_ADDRESS>
Usage: quiz-script --keystore-path <KEYSTORE_PATH> --verifier-contract-address <VERIFIER_CONTRACT_ADDRESS>              

@PatStiles
Copy link
Contributor Author

@Oppen apologies corrected the example commands in the description above.

@JuArce JuArce changed the title 1205 update sp1 risc0 update sp1 and risc0 Oct 24, 2024
Copy link
Collaborator

@JuArce JuArce left a comment

Choose a reason for hiding this comment

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

Close

@@ -493,7 +493,7 @@ make install_aligned_compiling

The SP1 and Risc0 proofs need the proof file and the vm program file.
The current SP1 version used in Aligned is
`v1.0.1` and the current Risc0 version used in Aligned is v1.0.1.
`v2.0.0` and the current Risc0 version used in Aligned is `v1.1.2`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't it v3.0.0-rc3 instead of v2.0.0

@JuArce JuArce closed this Oct 24, 2024
@JuArce JuArce deleted the 1205-update-sp1-risc0 branch October 24, 2024 21:18
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.

Operator: Bump SP1 and Risc0 dependencies
6 participants