Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a ZK shuffle contract for the XION blockchain that verifies shuffle-encrypt and decrypt zero-knowledge proofs. The contract provides a minimal implementation to test proof verification at the XION module level using Groth16 proofs.
Changes:
- CosmWasm contract that verifies shuffle encrypt and decrypt proofs via XION's ZK module
- Shell scripts for testing proof verification with sample proof data
- Verification keys, deployment configurations, and comprehensive documentation
Reviewed changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| contracts/zk-shuffle/src/contract.rs | Main contract entry points for instantiate, execute, and query functions |
| contracts/zk-shuffle/src/zkshuffle.rs | ZK proof verification logic using XION's gRPC ZK module interface |
| contracts/zk-shuffle/src/types.rs | Groth16Proof type definition |
| contracts/zk-shuffle/src/state.rs | State management for tracking verification counts |
| contracts/zk-shuffle/src/msg.rs | Message types for contract interactions |
| contracts/zk-shuffle/src/error.rs | Contract error definitions |
| contracts/zk-shuffle/vkeys/shuffle_encrupt.json | Verification key for shuffle encrypt circuit (filename has typo) |
| contracts/zk-shuffle/vkeys/decrypt.json | Verification key for decrypt circuit |
| contracts/zk-shuffle/scripts/*.sh | Shell scripts for testing proof verification |
| contracts/zk-shuffle/Cargo.toml | Dependencies including cosmos-sdk-proto from burnt-labs fork |
| contracts/zk-shuffle/README.md | Comprehensive deployment and usage documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn verify_shuffle_proof( | ||
| deps: Deps, | ||
| proof: &([Uint256; 2], [[Uint256; 2]; 2], [Uint256; 2]), | ||
| public_inputs: &[Uint256], | ||
| verifier_name: &str, | ||
| ) -> Result<bool, ContractError> { | ||
| let snarkjs_proof = groth16_proof_to_snarkjs(&proof.0, &proof.1, &proof.2); | ||
| let public_inputs_str = public_inputs_to_string(public_inputs); | ||
|
|
||
| let verify_request = QueryVerifyRequest { | ||
| proof: serde_json::to_vec(&snarkjs_proof).map_err(|e| { | ||
| ContractError::Std(cosmwasm_std::StdError::generic_err(format!( | ||
| "Failed to serialize proof: {}", | ||
| e | ||
| ))) | ||
| })?, | ||
| public_inputs: public_inputs_str, | ||
| vkey_name: verifier_name.to_string(), | ||
| vkey_id: 3, | ||
| }; | ||
|
|
||
| let request_bytes = verify_request.to_bytes().map_err(|e| { | ||
| ContractError::Std(cosmwasm_std::StdError::generic_err(format!( | ||
| "Failed to encode verify request: {}", | ||
| e | ||
| ))) | ||
| })?; | ||
|
|
||
| let response: cosmwasm_std::Binary = deps | ||
| .querier | ||
| .query_grpc( | ||
| "/xion.zk.v1.Query/ProofVerify".to_string(), | ||
| cosmwasm_std::Binary::from(request_bytes), | ||
| ) | ||
| .map_err(|e| { | ||
| ContractError::Std(cosmwasm_std::StdError::generic_err(format!( | ||
| "Failed to query zk module: {}", | ||
| e | ||
| ))) | ||
| })?; | ||
|
|
||
| let verify_response: ProofVerifyResponse = ProofVerifyResponse::decode(response.as_slice()) | ||
| .map_err(|e| { | ||
| ContractError::Std(cosmwasm_std::StdError::generic_err(format!( | ||
| "Failed to decode verify response: {}", | ||
| e | ||
| ))) | ||
| })?; | ||
|
|
||
| Ok(verify_response.verified) | ||
| } | ||
|
|
||
| /// Verify a decryption proof using the XION zk module | ||
| /// | ||
| /// # Arguments | ||
| /// * `deps` - Deps for querier access | ||
| /// * `proof` - Groth16 proof (a, b, c components) | ||
| /// * `public_inputs` - Public inputs for the proof circuit | ||
| /// * `verifier_name` - Name of the verifier key stored in the zk module | ||
| /// | ||
| /// # Returns | ||
| /// * `Ok(true)` if proof is valid | ||
| /// * `Ok(false)` if proof is invalid | ||
| /// * `Err(ContractError)` if verification fails due to other errors | ||
| pub fn verify_decrypt_proof( | ||
| deps: Deps, | ||
| proof: &([Uint256; 2], [[Uint256; 2]; 2], [Uint256; 2]), | ||
| public_inputs: &[Uint256], | ||
| verifier_name: &str, | ||
| ) -> Result<bool, ContractError> { | ||
| let snarkjs_proof = groth16_proof_to_snarkjs(&proof.0, &proof.1, &proof.2); | ||
| let public_inputs_str = public_inputs_to_string(public_inputs); | ||
|
|
||
| let verify_request = QueryVerifyRequest { | ||
| proof: serde_json::to_vec(&snarkjs_proof).map_err(|e| { | ||
| ContractError::Std(cosmwasm_std::StdError::generic_err(format!( | ||
| "Failed to serialize proof: {}", | ||
| e | ||
| ))) | ||
| })?, | ||
| public_inputs: public_inputs_str, | ||
| vkey_name: verifier_name.to_string(), | ||
| vkey_id: 2, | ||
| }; | ||
|
|
||
| let request_bytes = verify_request.to_bytes().map_err(|e| { | ||
| ContractError::Std(cosmwasm_std::StdError::generic_err(format!( | ||
| "Failed to encode verify request: {}", | ||
| e | ||
| ))) | ||
| })?; | ||
|
|
||
| let response: cosmwasm_std::Binary = deps | ||
| .querier | ||
| .query_grpc( | ||
| "/xion.zk.v1.Query/ProofVerify".to_string(), | ||
| cosmwasm_std::Binary::from(request_bytes), | ||
| ) | ||
| .map_err(|e| { | ||
| ContractError::Std(cosmwasm_std::StdError::generic_err(format!( | ||
| "Failed to query zk module: {}", | ||
| e | ||
| ))) | ||
| })?; | ||
|
|
||
| let verify_response: ProofVerifyResponse = ProofVerifyResponse::decode(response.as_slice()) | ||
| .map_err(|e| { | ||
| ContractError::Std(cosmwasm_std::StdError::generic_err(format!( | ||
| "Failed to decode verify response: {}", | ||
| e | ||
| ))) | ||
| })?; | ||
|
|
||
| Ok(verify_response.verified) | ||
| } |
There was a problem hiding this comment.
The code has significant duplication between verify_shuffle_proof and verify_decrypt_proof functions. The only differences are the vkey_id values (3 vs 2). Consider refactoring to a single generic verify_proof function that takes vkey_id as a parameter to reduce code duplication and improve maintainability.
| //! Minimal zkShuffle contract for testing proof verification at XION module level | ||
|
|
||
| #[cfg(not(feature = "library"))] | ||
| use cosmwasm_std::entry_point; | ||
| use cosmwasm_std::{ | ||
| to_json_binary, Addr, Binary, Deps, DepsMut, Env, MessageInfo, Response, StdResult, | ||
| }; | ||
|
|
||
| use crate::error::ContractError; | ||
| use crate::msg::{ExecuteMsg, InstantiateMsg, QueryMsg, VerificationCountResponse}; | ||
| use crate::state::{VerificationState, VERIFICATION_STATE}; | ||
| use crate::types::Groth16Proof; | ||
| use crate::zkshuffle::{verify_decrypt_proof, verify_shuffle_proof}; | ||
|
|
||
| const CONTRACT_NAME: &str = "crates.io:zk-shuffle"; | ||
| const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); | ||
|
|
||
| #[cfg_attr(not(feature = "library"), entry_point)] | ||
| pub fn instantiate( | ||
| deps: DepsMut, | ||
| _env: Env, | ||
| _info: MessageInfo, | ||
| _msg: InstantiateMsg, | ||
| ) -> Result<Response, ContractError> { | ||
| let state = VerificationState::new(); | ||
| VERIFICATION_STATE.save(deps.storage, &state)?; | ||
|
|
||
| Ok(Response::new().add_attribute("action", "instantiate")) | ||
| } | ||
|
|
||
| #[cfg_attr(not(feature = "library"), entry_point)] | ||
| pub fn execute( | ||
| deps: DepsMut, | ||
| _env: Env, | ||
| _info: MessageInfo, | ||
| msg: ExecuteMsg, | ||
| ) -> Result<Response, ContractError> { | ||
| match msg { | ||
| ExecuteMsg::VerifyShuffleProof { | ||
| proof, | ||
| public_inputs, | ||
| } => execute_verify_shuffle_proof(deps, proof, public_inputs), | ||
| ExecuteMsg::VerifyDecryptProof { | ||
| proof, | ||
| public_inputs, | ||
| } => execute_verify_decrypt_proof(deps, proof, public_inputs), | ||
| } | ||
| } | ||
|
|
||
| fn execute_verify_shuffle_proof( | ||
| deps: DepsMut, | ||
| proof: Groth16Proof, | ||
| public_inputs: Vec<cosmwasm_std::Uint256>, | ||
| ) -> Result<Response, ContractError> { | ||
| let proof_tuple = (proof.a, proof.b, proof.c); | ||
| let verifier_name = "shuffle_encrypt"; | ||
| let verified = | ||
| verify_shuffle_proof(deps.as_ref(), &proof_tuple, &public_inputs, verifier_name)?; | ||
|
|
||
| if !verified { | ||
| return Err(ContractError::InvalidProof); | ||
| } | ||
| let mut state = VERIFICATION_STATE.load(deps.storage)?; | ||
| state.shuffle_verifications += 1; | ||
| VERIFICATION_STATE.save(deps.storage, &state)?; | ||
|
|
||
| Ok(Response::new() | ||
| .add_attribute("action", "verify_shuffle_proof") | ||
| .add_attribute("result", "success") | ||
| .add_attribute( | ||
| "total_shuffle_verifications", | ||
| state.shuffle_verifications.to_string(), | ||
| )) | ||
| } | ||
|
|
||
| fn execute_verify_decrypt_proof( | ||
| deps: DepsMut, | ||
| proof: Groth16Proof, | ||
| public_inputs: Vec<cosmwasm_std::Uint256>, | ||
| ) -> Result<Response, ContractError> { | ||
| let proof_tuple = (proof.a, proof.b, proof.c); | ||
| let verifier_name = "decrypt"; | ||
| let verified = | ||
| verify_decrypt_proof(deps.as_ref(), &proof_tuple, &public_inputs, verifier_name)?; | ||
|
|
||
| if !verified { | ||
| return Err(ContractError::InvalidProof); | ||
| } | ||
| let mut state = VERIFICATION_STATE.load(deps.storage)?; | ||
| state.decrypt_verifications += 1; | ||
| VERIFICATION_STATE.save(deps.storage, &state)?; | ||
|
|
||
| Ok(Response::new() | ||
| .add_attribute("action", "verify_decrypt_proof") | ||
| .add_attribute("result", "success") | ||
| .add_attribute( | ||
| "total_decrypt_verifications", | ||
| state.decrypt_verifications.to_string(), | ||
| )) | ||
| } | ||
|
|
||
| #[cfg_attr(not(feature = "library"), entry_point)] | ||
| pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult<Binary> { | ||
| match msg { | ||
| QueryMsg::VerificationCount {} => to_json_binary(&query_verification_count(deps)?), | ||
| } | ||
| } | ||
|
|
||
| fn query_verification_count(deps: Deps) -> StdResult<VerificationCountResponse> { | ||
| let state = VERIFICATION_STATE.load(deps.storage)?; | ||
| Ok(VerificationCountResponse { | ||
| shuffle_verifications: state.shuffle_verifications, | ||
| decrypt_verifications: state.decrypt_verifications, | ||
| }) | ||
| } |
There was a problem hiding this comment.
The contract module (contract.rs) lacks test coverage for the execute and query functions. The only tests are in zkshuffle.rs for utility functions. Consider adding integration tests or unit tests with mocked dependencies for execute_verify_shuffle_proof, execute_verify_decrypt_proof, and query_verification_count to ensure proper error handling and state management.
| @@ -0,0 +1,1155 @@ | |||
| { | |||
There was a problem hiding this comment.
The filename contains a spelling error: "shuffle_encrupt.json" should be "shuffle_encrypt.json" to match the naming used throughout the codebase (e.g., "shuffle_encrypt" in contract.rs line 56).
| docker run --rm -v "$(pwd)":/code -v ~/.ssh:/root/.ssh:ro -e SSH_AUTH_SOCK=/ssh-agent \ | ||
| --mount type=volume,source="$(basename "$(pwd)")_cache",target=/target \ | ||
| --mount type=volume,source=registry_cache,target=/usr/local/cargo/registry \ | ||
| wasm_optimizer ./ |
There was a problem hiding this comment.
Mounting the host ~/.ssh directory into the build container (-v ~/.ssh:/root/.ssh:ro) exposes your SSH private keys to any code running inside the optimizer image. A compromised base image or malicious build dependency (e.g., a build.rs script from a third-party crate) could read /root/.ssh and exfiltrate your keys over the network, leading to repository or infrastructure compromise. To mitigate this, avoid mounting raw SSH key material into the container (prefer agent forwarding or narrowly scoped deploy keys) and restrict network access for the build container where possible.
crucible-burnt
left a comment
There was a problem hiding this comment.
Review — ZK Shuffle Circuits
6k line addition — new contract for ZK shuffle encrypt/decrypt proof verification. This is a significant crypto primitive.
High-level observations:
- New contract at
contracts/zk-shuffle/with Groth16 verification for shuffle encrypt + decrypt proofs - Includes verification keys, test data, and shell scripts for testing
⚠️ vkeys/shuffle_encrupt.json— typo: should beshuffle_encrypt.json
Recommendation: This contract deals with zero-knowledge proof verification — a domain where subtle bugs can break soundness guarantees. Strongly recommend a dedicated ZK/crypto specialist review before merging. Specifically:
- Verification key generation process and trusted setup assumptions
- Public input binding — ensure all relevant inputs are committed
- Proof malleability — check if the verifier rejects modified proofs
- Gas costs — Groth16 verification on-chain can be expensive; verify gas benchmarks
Happy to do a line-by-line code review, but the cryptographic correctness needs a specialist.
crucible-burnt
left a comment
There was a problem hiding this comment.
🔍 Crucible Security Review
Summary
Adds ZK Shuffle contract infrastructure for zero-knowledge card shuffling. Large PR adding new contract with cryptographic dependencies.
Security Assessment
- Risk Level: Medium
⚠️ (new ZK functionality requires careful review) - New contract at
contracts/zk-shuffle/with:- Cargo.lock with cryptographic dependencies (ark-bls12-381, ark-ec, ark-ff, ark-poly)
- Environment configuration examples
- Dependencies include:
ark-*suite for elliptic curve and field arithmeticrayonfor parallel computation- Standard CosmWasm dependencies
Immunefi Pattern Check
- New attack surface: ZK proof verification contracts are high-value targets
- Requires dedicated cryptographic review beyond automated scanning
- No known matches to existing vulnerability patterns (new code)
False Report Risk
- ZK implementations commonly attract security researcher attention
- Recommend thorough documentation of cryptographic assumptions
Code Quality Notes
- Large dependency tree — review for pinned versions and supply chain security
.env.exampleincludes testnet configuration — appropriate for development- Contract code not visible in diff excerpt — full review needed
- Recommendation: Ensure ZK circuits have been formally verified or audited
Status
Requires deeper cryptographic review. Standard security patterns appear followed, but ZK implementations need specialist attention. Flag for dedicated audit before mainnet deployment.
Changes Summary