Skip to content

ZK Shuffle circuits#116

Open
satyambnsal wants to merge 2 commits into
mainfrom
feat/zk-shuffle-contracts
Open

ZK Shuffle circuits#116
satyambnsal wants to merge 2 commits into
mainfrom
feat/zk-shuffle-contracts

Conversation

@satyambnsal

Copy link
Copy Markdown

Changes Summary

  1. ZK shuffle contract that verifies shuffle encrypt proof and decrypt proof inside contract
  2. Script to call the contract

Copilot AI review requested due to automatic review settings January 26, 2026 17:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment on lines +76 to +190
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)
}

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +115
//! 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,
})
}

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,1155 @@
{

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

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

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).

Copilot uses AI. Check for mistakes.
Comment on lines +37 to +40
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 ./

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.

@crucible-burnt crucible-burnt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 be shuffle_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:

  1. Verification key generation process and trusted setup assumptions
  2. Public input binding — ensure all relevant inputs are committed
  3. Proof malleability — check if the verifier rejects modified proofs
  4. 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 crucible-burnt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 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 arithmetic
    • rayon for 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.example includes 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.

@crucible-burnt crucible-burnt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Crucible Security Review

Summary

Adds ZK Shuffle circuits contract. Large PR (6k+ lines) — majority is Cargo.lock and circuit boilerplate.

Security Assessment

  • Risk Level: Low — new contract addition, not modifying existing security-critical code

Notes:

  • .env.example includes testnet addresses and a TX hash — verify no sensitive keys are included (looks clean on inspection).
  • Cargo.lock committed (good for reproducibility).
  • This is a new feature contract; security review of the ZK verification logic itself would require a dedicated audit of the circuit constraints and verification paths.

Immunefi Pattern Check

  • No matches against known vulnerability classes.

False Report Risk

  • Low. New standalone contract.

Code Quality Notes

  • Large PR — consider splitting Cargo.lock updates from contract logic in future PRs for easier review.

Status

No security concerns from a surface review. ZK circuit correctness would need a dedicated cryptographic audit beyond this automated scan.

@crucible-burnt crucible-burnt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Crucible Security Review

Summary

New zk-shuffle contract implementing Groth16 proof verification for shuffle-encrypt and decrypt operations via XION's on-chain ZK module. Delegates all cryptographic verification to the chain's gRPC endpoint.

Security Assessment

  • Risk Level: Medium

Findings:

  1. Hardcoded vkey_id: 3 (zkshuffle.rs): Both verify_shuffle_proof and verify_decrypt_proof use vkey_id: 3 hardcoded in the QueryVerifyRequest. This means:

    • If the vkey at ID 3 changes, all verifications break or become insecure
    • There's no governance path to update this without redeploying
    • The verifier_name parameter is passed but vkey_id overrides it — unclear which takes precedence in the xion zk module
      Action: Make vkey_id configurable via contract state (set at instantiation or via admin update).
  2. No access control on execute: Anyone can call VerifyShuffleProof and VerifyDecryptProof. While verification itself is read-only in effect (just increments a counter), the gas cost of proof verification is non-trivial. An attacker could submit many invalid proofs to waste gas. Consider rate limiting or requiring a deposit.

  3. Verification counter has no practical utility: The VerificationState counter increments on success but isn't used for any logic. If this is just for metrics, consider using events/attributes only (already emitted) to save storage writes.

  4. Separate Cargo.lock: The contract has its own Cargo.lock (1848 lines) separate from the workspace. This can lead to dependency version drift and makes auditing harder. Consider integrating into the workspace Cargo.lock.

  5. Typo in vkey filename: vkeys/shuffle_encrupt.json — should be shuffle_encrypt.json. If the vkey is loaded at deploy time, this is cosmetic; if loaded at runtime by name, it could cause lookup failures.

Immunefi Pattern Check

  • The Groth16 verification delegates to xion's ZK module — the contract itself doesn't perform any cryptographic operations. Attack surface is in the chain module, not the contract.
  • No JWT, signer validation, or fee handling patterns present.

False Report Risk

  • The hardcoded vkey_id: 3 could be flagged as a "configuration vulnerability" — it's more of a maintainability concern than a security one, since the chain module controls what vkey ID 3 maps to.
  • The permissionless verify calls could be flagged as "gas griefing" — standard for public verification contracts but worth documenting.

Code Quality Notes

  • Clean separation between contract logic and ZK verification module
  • Good use of xion's gRPC interface for proof verification
  • The SnarkJsProof format conversion is correct for bn128/Groth16
  • Missing unit tests for the contract itself (only shell scripts for integration testing)
  • .env.example contains testnet-specific values (code ID, tx hash, contract address) — should be placeholders

Status

Functional ZK verification contract with delegation to chain module. Key recommendations: (1) make vkey_id configurable, (2) fix the typo in vkey filename, (3) add unit tests, (4) consider workspace Cargo.lock integration.

@crucible-burnt crucible-burnt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Crucible Security Review

Summary

New CosmWasm contract (zk-shuffle) for verifying Groth16 ZK proofs for card shuffle and decryption operations, delegating verification to XION's x/zk module via gRPC queries.

Security Assessment

  • Risk Level: Medium

1. Hardcoded vkey_id values (zkshuffle.rs:98, :152)

  • verify_shuffle_proof hardcodes vkey_id: 3 and verify_decrypt_proof hardcodes vkey_id: 2. These IDs are environment-specific (testnet) and will break if vkeys are re-registered or on a different deployment. The vkey_name parameter is already passed — consider removing vkey_id or setting it to 0 to let the module resolve by name only.

2. No access control on execute messages (contract.rs)

  • VerifyShuffleProof and VerifyDecryptProof are callable by anyone. While verification itself is read-only at the module level, the contract writes state (increments counters). In production, this means anyone can inflate verification counters. If counters are used for any downstream logic, this could be exploitable.

3. Dependency on custom fork (Cargo.toml:49-52)

  • cosmos-sdk-proto is sourced from https://github.com/burnt-labs/cosmos-rust branch feat/xion-zk. This is expected for the custom xion.v1.zk proto types, but the branch should be pinned to a specific commit hash (rev = "abc123") to prevent supply-chain issues from force-pushes.

4. .env.example contains testnet-specific data (.env.example)

  • Includes real TX_HASH, CODE_ID, CONTRACT_ADDRESS from testnet. Not a security risk, but could mislead users into interacting with stale deployments. Consider documenting this is testnet-only.

5. Typo in vkey filename (vkeys/shuffle_encrupt.json)

  • Should be shuffle_encrypt.json. If the vkey_name in the zk module is shuffle_encrypt but the file is named shuffle_encrupt, this creates confusion.

Immunefi Pattern Check

  • ✅ Proof verification is delegated to the chain's zk module — no custom cryptographic implementation
  • ✅ No fee manipulation or gas abuse vectors (standard CosmWasm gas metering applies)
  • ℹ️ The Groth16 proof format matches SnarkJS conventions (pi_a, pi_b, pi_c with affine point encoding)
  • ⚠️ The vkey_id + vkey_name dual-specification could theoretically cause the module to verify against a different key than intended if they point to different vkeys. Recommend using name-only resolution.

False Report Risk

  • The lack of access control could invite reports about "unauthorized proof submission" — worth adding a comment that this is intentional for a testing/demo contract
  • The hardcoded vkey_ids could invite reports about "wrong verification key" attacks

Code Quality Notes

  • verify_shuffle_proof and verify_decrypt_proof are nearly identical (~40 lines each) — should be refactored into a single verify_proof helper
  • No unit tests for contract logic (only serialization tests in zkshuffle.rs)
  • Good: Contract properly uses query_grpc for module-level verification rather than implementing its own verifier
  • Cargo.lock is committed (1848 lines) — standard for contract repos but increases review surface

Status

Functional for testnet testing. Before any mainnet deployment: fix the hardcoded vkey_ids (#1), pin the cosmos-sdk-proto dependency (#3), add access control or document its absence (#2), and add integration tests.

@crucible-burnt crucible-burnt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Crucible Security Review

Summary

New zk-shuffle CosmWasm contract for verifying Groth16 shuffle-encrypt and decrypt proofs via XION's module-level zk API. This is a testnet verification contract with no funds handling.

Security Assessment

  • Risk Level: Medium
  • No access control on execute: execute_verify_shuffle_proof and execute_verify_decrypt_proof have no sender validation — anyone can call them. Since these only verify proofs and increment counters, this is likely intentional for a testing contract, but worth confirming. If this contract evolves to manage game state, access control will be critical.
  • Hardcoded vkey_id values: vkey_id: 3 for shuffle and vkey_id: 2 for decrypt are hardcoded in zkshuffle.rs. If the zk module's key registry changes, these will silently break. Consider making them configurable via instantiate params or a governance-updatable config.
  • No input size validation: public_inputs: Vec<Uint256> is unbounded. A malicious caller could submit an extremely large vector, potentially causing high gas consumption. The zk module query should reject mismatched input counts, but the serialization cost is borne by the caller before the query. Low risk given gas metering, but worth noting.
  • Code duplication: verify_shuffle_proof and verify_decrypt_proof are nearly identical (~50 lines each), differing only in vkey_id. Consider refactoring to a single verify_proof function parameterized by vkey_id/name.
  • Committed .env.example: Contains testnet contract address and code ID. Fine for testnet, just ensure no mainnet secrets leak into this pattern.

Immunefi Pattern Check

  • No matches against known vulnerability classes. This contract doesn't handle funds, auth, or IBC — it's purely a ZK verification wrapper.
  • No JWT, fee bypass, expiry, or signer validation patterns involved.

False Report Risk

  • Low: The lack of access control could be flagged by auditors as a vulnerability, but for a proof-verification-only contract this is acceptable. Adding a brief comment in contract.rs explaining this is intentional (e.g., "Permissionless verification — no state mutation beyond counters") would preempt false reports.
  • The Unauthorized error variant in error.rs is defined but never used — could confuse auditors into thinking auth was intended but not implemented. Either remove it or add a TODO comment.

Code Quality Notes

  • Clean contract structure with proper separation (msg, state, types, contract, error, zkshuffle modules).
  • Good unit tests for proof conversion and serialization.
  • Cargo.lock is committed (6086 lines) — standard for CosmWasm contracts but adds review noise.
  • use serde_json as _; in zkshuffle.rs is unusual — appears to be ensuring the crate is linked. If it's just for serde_json::to_vec, the normal import suffices.
  • Dockerfile configures SSH and git for private dependency fetching — the cosmos-sdk-proto git dependency points to burnt-labs/cosmos-rust on a feature branch. Ensure this branch is stable before mainnet deployment.
  • No integration tests with mock querier for the gRPC path.

Status

Solid initial implementation for a testnet ZK verification contract. Medium risk due to missing access control documentation and hardcoded vkey IDs. Recommended: add intent comments for permissionless access, refactor duplicated verification logic, and add configurable vkey management before any mainnet consideration. No blockers for testnet deployment.

@crucible-burnt crucible-burnt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Crucible Security Review

Summary

New zk-shuffle CosmWasm contract for on-chain ZK proof verification of card shuffle and decrypt operations via XION's zk module gRPC queries. Significant new contract with cryptographic operations.

Security Assessment

  • Risk Level: Medium-High

Critical Findings:

  1. Hardcoded vkey_id values (zkshuffle.rs:101 and zkshuffle.rs:165): vkey_id: 3 for shuffle and vkey_id: 2 for decrypt are hardcoded alongside vkey_name. The XION zk module's ProofVerify query uses vkey_name when set (takes precedence over vkey_id), but if the module behavior changes or vkey_name is empty, the wrong vkey could be used. These IDs should either be configurable via InstantiateMsg or the vkey_id field should be set to 0 when using name-based lookup.

  2. No access control on ExecuteMsg (contract.rs:35-50): Anyone can call VerifyShuffleProof and VerifyDecryptProof. While proof verification itself is safe (read-only query), the state mutation (incrementing counters) is unprotected. If this is a test contract, acceptable — but for production, consider restricting callers or making it clear this is intentionally permissionless.

  3. .env.example contains real testnet data (.env.example:4-8): Contains actual TX_HASH, CODE_ID, CONTRACT_ADDRESS for testnet. While testnet data isn't secret, it could cause confusion if someone copies these values expecting them to work.

  4. Typo in vkey filename: vkeys/shuffle_encrupt.json should be shuffle_encrypt.json — this typo will propagate into any tooling that references this file.

  5. serde_json unused import (zkshuffle.rs:12): use serde_json as _; is imported but only used within function bodies. This is fine for preventing unused crate warnings, but verify it's needed.

Immunefi Pattern Check

  • State manipulation: Counter increments (shuffle_verifications, decrypt_verifications) are protected by successful proof verification — no bypass path.
  • No fee bypass: Contract doesn't handle funds.
  • Proto.Message singletons: Uses cosmos_sdk_proto types for gRPC queries — these are properly constructed per-call, no singleton reuse.
  • No matches against known Immunefi vulnerability classes.

False Report Risk

  • The hardcoded vkey_id values could be flagged as "wrong key used for verification" — document why these specific IDs are used.
  • The Unauthorized and NotSupported error variants in error.rs are defined but unused — auditors may flag dead code.
  • AlreadyDecrypted and MissingCallback errors are also unused — suggests this is a stripped-down version of a larger contract. Document this to avoid confusion.

Code Quality Notes

  • Good use of XION's native gRPC query for proof verification instead of re-implementing crypto in WASM.
  • Well-structured proof conversion (groth16_proof_to_snarkjs) with clear documentation of the affine point format.
  • Unit tests cover proof conversion and serialization but not the actual contract execute/query paths (would need mock querier).
  • Cargo.lock committed — appropriate for binary/contract crates.
  • Duplicate code: verify_shuffle_proof and verify_decrypt_proof are nearly identical. Extract a common verify_groth16_proof helper.

Status

New contract with medium-high risk due to ZK verification surface and hardcoded vkey IDs. The hardcoded IDs are the primary concern — recommend making them configurable. No critical security vulnerabilities, but several code quality improvements recommended before production use.

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