Skip to content

Example Opacity Verifier#73

Open
ash-burnt wants to merge 13 commits intomainfrom
example/opacity-verifier
Open

Example Opacity Verifier#73
ash-burnt wants to merge 13 commits intomainfrom
example/opacity-verifier

Conversation

@ash-burnt
Copy link
Contributor

Description

Contract Details

Please append all the required information below for the contract(s) being added to contracts.json.

Required fields and their descriptions:

  • name: Contract name (required)
  • description: Brief description of the contract's purpose
  • code_id: Contract code ID on mainnet
  • hash: Contract hash in UPPERCASE
  • release:
  • author:
    • name: Organization name
    • url: Organization website URL
  • governance: "Genesis" or proposal number
  • deprecated: true if contract is deprecated (mixed inline with active contracts)

Example JSON structure:

{
  "name": "",
  "description": "",
  "code_id": "",
  "hash": "",
  "release": {
    "url": "",
    "version": ""
  },
  "author": {
    "name": "",
    "url": ""
  },
  "governance": "",
  "deprecated": false
}

Finding Code ID and Hash

To find the latest code ID and hash:

  1. Run the verification tool which will show all code IDs on chain:
    node scripts/verify-code-ids.js
  2. The new code ID will be shown in the mismatches as "exists on chain but not in contracts.json"
  3. You can also query the code hash via the chain's RPC endpoint:
    xiond query wasm code-info <code-id> --node https://rpc.xion-mainnet-1.burnt.com

Documentation Updates

The README.md is automatically generated from contracts.json. After making changes:

  1. Ensure you have the required dependencies:

    • Node.js: https://nodejs.org/
    • jq: brew install jq (macOS) or apt-get install jq (Ubuntu/Debian)
  2. Run the convert script to validate and update the README:

    ./convert.sh
  3. Commit both the contracts.json and generated README.md changes

⚠️ Important Notes:

  • Do not edit README.md manually. All changes must be made through contracts.json
  • Pull requests with manual README edits will be automatically rejected by CI
  • If you forget to run ./convert.sh locally, the CI will fail with a "README out of sync" error

Validation

The convert.sh script automatically performs these validations:

  • All required fields are present and properly formatted
  • Hash is 64 characters and uppercase hex
  • URLs are valid HTTPS links
  • Code IDs are unique
  • Contracts are ordered by code_id (both active and deprecated contracts follow the same ordering)
  • README.md stays in sync with contracts.json

If any validation fails, the script will show specific error messages to help you fix the issues.

Checklist

  • Added entry to contracts.json with all required fields
  • Contract name is clear and descriptive
  • Description explains the contract's purpose
  • Code ID matches the mainnet deployed code
  • Hash is in uppercase and matches the stored code
  • Release URL points to the correct tag/commit
  • Version matches the release tag or commit hash
  • Author information is correct with valid URL
  • Governance field correctly references proposal or "Genesis"
  • Deprecated flag is set appropriately
  • Entry is placed in code_id order (regardless of deprecated status)
  • Ran ./convert.sh and fixed any validation errors
  • Both contracts.json and generated README.md are included in the commit

Additional Notes

@crucible-burnt
Copy link
Contributor

🔍 Crucible Security Review

Summary

Adds Opacity Verifier example contract with Ethereum signature verification and allowlist-based key management. Educational/example contract for signature verification patterns.

Security Assessment

  • Risk Level: Low (example contract, not production)
  • Signature verification: Correct secp256k1 recovery using Cosmos SDK primitives
  • Address recovery: Proper Ethereum pattern (last 20 bytes of keccak256)
  • Access control: Admin-only list updates ✓
  • Allowlist checks: Simple and straightforward
  • Signature validation: Length check (65 bytes minimum) enforced

Immunefi Pattern Check

  • ✅ No state manipulation vulnerabilities
  • ✅ Proper admin gating
  • ✅ Signature verification uses correct APIs

False Report Risk

  • Add documentation that this is an example contract for educational purposes
  • Clarify Ethereum signature format in comments if this is production-adjacent

Code Quality

  • Clean error handling
  • Clear separation of concerns (eth_crypto, query, state)
  • Could add more comments on Ethereum address recovery nuances

Recommendation

Approve - Well-implemented example contract with proper signature verification. Add documentation clarifying example status.

Copy link
Contributor

@crucible-burnt crucible-burnt left a comment

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 Opacity Verifier contract implementing Ethereum signature verification against an admin-controlled allowlist of signing keys. Critical security component for validating Opacity Network attestations on-chain.

Security Assessment

  • Risk Level: Medium

Findings:

  1. Signature verification logic is correct (src/query.rs:5-24): Properly implements secp256k1 recovery → keccak256 → address derivation → allowlist check. The standard Ethereum personal_sign flow is followed.

  2. Admin authorization is properly enforced (src/contract.rs:50-52): All execute messages check info.sender != admin before proceeding. Good.

  3. Address normalization is sound (src/contract.rs:7-15): normalize_addr_hex strips 0x, lowercases, validates 40-char hex, and attempts decode. This prevents case-sensitivity issues in allowlist lookups.

  4. ⚠️ .unwrap() in query (src/query.rs:38): VERIFICATION_KEY_ALLOW_LIST.keys(...).map(|k| k.unwrap()).collect() will panic on corrupted storage. While queries don't affect state, this could be a DoS vector for RPC nodes. Consider using .filter_map(|k| k.ok()) or propagating the error.

  5. Allowlist replacement is atomic (src/contract.rs:62-67): UpdateAllowList clears the entire list then repopulates. If a key in the new list fails normalize_addr_hex, the transaction reverts atomically (CosmWasm guarantee), so no partial state corruption.

  6. Verify is a query, not execute: Good design — no state mutation during verification. However, this means there's no on-chain event trail of verifications. If audit trail matters, consider an execute variant.

  7. No migration entry point: The contract has MigrateMsg in the schema but I don't see a #[entry_point] pub fn migrate — verify this is intentional or add it for upgradeability.

Immunefi Pattern Check

  • No proto.Message singletons (Rust/CosmWasm contract)
  • Signature verification uses cosmwasm_std::Api::secp256k1_recover_pubkey — a well-tested host function
  • No fee bypass vectors — admin-only state mutations
  • Recovery ID normalization handles all valid values (0, 1, 27, 28) — no bypass possible

False Report Risk

  • The Ethereum-style signature verification could invite reports about "replay attacks" — but since this is a stateless query that returns bool, there's no replay risk. Consider documenting this.
  • The always_fail getrandom override in lib.rs is correctly required for CosmWasm determinism — not a vulnerability.

Code Quality Notes

  • Clean separation of concerns (eth_crypto, query, contract, state)
  • Tests use real Opacity Network public keys — good integration testing
  • Error types are well-defined with proper From impls
  • Consider adding tests for edge cases: empty signature, short signature, invalid recovery ID

Status

Solid implementation. The .unwrap() in key iteration and missing migrate entry point are the main items to address. No critical security issues found.

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.

2 participants