feat(sozo): add voyager contract verification - #3288
Conversation
glihm
left a comment
glihm
left a comment
There was a problem hiding this comment.
Amazing work there @ametel01 and thank you for taking the time on that.
I've made some rust comments and design comments.
Very happy to discuss those and if you don't have the bandwidth, we could keep that for a future refactor. But it shouldn't affect that much your current work, only on the config and option. 👍
Changes the verification option to use an enum for specifying the service. This allows for better type safety and clarity when selecting a verification service, replacing the previous string-based approach. Also introduces dedicated structs for better organization.
Simplifies contract verification by removing the optional `verification_config` and introducing a `VerificationConfig` enum to handle different verification service options (Voyager, Custom, None). This change streamlines the migration process and improves code clarity.
Decouples verification config creation from migration initialization, allowing for a more flexible and fluent builder pattern. This change provides an easier way to configure and apply verification to migrations by using builder pattern.
Renames the `with_verification_config` function to `with_verification` for better clarity and consistency. This change simplifies the API by providing a more intuitive name that directly reflects the function's purpose of enabling verification for a migration.
Migrates contract verification process to utilize `WorldLocal` instead of directly parsing the manifest file. This change ensures that contract verification leverages the local world state, providing a more accurate and reliable verification process. It also adds the `dojo-world` dependency to the voyager crate. A target directory is now configurable, allowing for flexible artifact loading during migration.
glihm
left a comment
glihm
left a comment
There was a problem hiding this comment.
Great iteration on the review, some additional comments to ensure extensibility and we should be almost good to go.
Does Voyager have an API for testing where we could eventually add an integration test calling the API, or not at the moment?
Will run some tests with some dojo projects and we should be good to move forward for this first iteration.
1. Removed the deprecated method: The discover_contract_artifacts method that was reading from manifest files has been completely removed from analyzer.rs:108-176. 2. Updated the test: The test test_discover_contract_artifacts_from_world now uses the new discover_contract_artifacts_from_world method that relies on WorldLocal instead of the deprecated manifest-based approach. 3. Cleaned up unused code: Removed the unused extract_contract_name_from_tag method and its associated test, as well as the unused find_manifest_file helper method. 4. Removed unused imports: Cleaned up the Manifest import and starknet_crypto::Felt import that were no longer needed. 5. Fixed clippy warnings: Removed the problematic assert!(true) that was flagged by clippy. The codebase now uses the modern discover_contract_artifacts_from_world method that works with WorldLocal instances, which is more robust and doesn't rely on the deprecated manifest file parsing approach. All tests are updated and passing.
Adds support for non-watch contract verification mode, which submits all contracts for verification in parallel without waiting for each one to complete. Provides a summary URL for tracking verification status in non-watch mode. Also includes job status URLs in the verification result messages.
Sadly we don't have a testing api, i'm running tests locally with docker but voyager is closed source so it will not work for you. You can use --verifiy voyager-dev which is our development deployment, you should be able to verify any class declared on mainnet already. |
glihm
changed the title
Related to that, we could add some integration test with the spawn and move example. We could have this example being deployed on mainnet, and after that check it on the |
2 participants
Description
Full PR description in #3287
Tests
Added to documentation?
Checklist
scripts/rust_fmt.sh,scripts/cairo_fmt.sh)scripts/clippy.sh,scripts/docs.sh)