Skip to content

Wormhole Implementation#407

Open
n13 wants to merge 58 commits intomainfrom
illuzen/wormhole
Open

Wormhole Implementation#407
n13 wants to merge 58 commits intomainfrom
illuzen/wormhole

Conversation

@n13
Copy link
Copy Markdown
Collaborator

@n13 n13 commented Mar 2, 2026

No description provided.

@n13
Copy link
Copy Markdown
Collaborator Author

n13 commented Mar 2, 2026

Draft PR overview / Review.


PR Overview

Branch: illuzen/wormhole — 20 commits, 272 files changed, ~80k lines added. The bulk is generated Planck pallet bindings, but the core wormhole implementation is substantial and spans three layers:

Layer Key Changes
Rust SDK (quantus_sdk/rust) wormhole.rs (1281 lines) — address derivation, ZK proof generation, aggregation, circuit binary management
Dart SDK (quantus_sdk/lib) wormhole_service.dart, wormhole_utxo_service.dart, substrate_service.dart, FFI bridge + generated Planck bindings
Miner App miner_wallet_service.dart, withdrawal_service.dart, circuit_manager.dart, transfer_tracking_service.dart, withdrawal_screen.dart, balance card updates

Architecture Summary

User → WithdrawalScreen → WithdrawalService
         ↓                    ↓
  TransferTrackingService   WormholeService → FFI → Rust wormhole.rs
         ↓                    ↓                         ↓
  mining_transfers.json   ProofGenerator          qp-wormhole-circuit
                          ProofAggregator         qp-wormhole-prover
                              ↓                   qp-wormhole-aggregator
                     author_submitExtrinsic

The flow: miner earns rewards to a wormhole address → tracks transfer UTXOs → generates ZK proofs (one per UTXO) → aggregates into a batch proof → submits unsigned extrinsic to chain for withdrawal.


Cross-Verification with qp-zk-circuits

The qp-zk-circuits/wormhole/ directory provides the circuit implementation that quantus-apps depends on (v1.0.7). Key alignment points:

Aspect qp-zk-circuits quantus-apps Status
Nullifier H(H("~nullif~" || secret || transfer_count)) Uses qp_wormhole_circuit::nullifier::Nullifier::from_preimage Aligned
Unspendable account H(H("wormhole" || secret)) Uses qp_wormhole_circuit::unspendable_account Aligned
Storage proof ordering prepare_proof_for_circuit (root-to-leaf) Uses same function from qp_zk_circuits_common Aligned
Public input layout 21 felts per leaf proof Proof generator builds matching layout Aligned
Aggregation aggregate_with_wrapper deduplicates exits, pads with dummies WormholeProofAggregator wraps qp_wormhole_aggregator Aligned

Potential concern: Block hash computation — the circuit uses hash_no_pad_bytes over field elements while the SDK's compute_block_hash_internal SCALE-encodes then uses hash_variable_length_bytes. If these don't produce the same output, proofs will fail. Worth verifying in integration testing.


Bugs and Issues Found

High Severity

  1. _isNullifierConsumed() always returns false in TransferTrackingService (lines ~263-264) — spent transfers are never filtered, creating double-spend risk. This is marked TODO but is a critical gap.

  2. _addressToHex() is a no-op stub in WormholeUtxoService — returns the SS58 string unchanged instead of converting to 32-byte hex. The Rust side calls parse_hex_32(&utxo.funding_account_hex) and would panic on an SS58 string. This will break proof generation.

Medium Severity

  1. Dead code in WithdrawalService_getTransfersFromChain() immediately throws; associated methods (_getTransferProofInfo, _getMintingAccount) and hash helpers (_twox128, _blake2128Concat, _simpleHash) are unreachable. ~50 lines of dead/placeholder code.

  2. blockNumber stored as 0 in TrackedTransfer (line 365 of transfer_tracking_service.dart) — block number is never persisted, affecting debugging and potential future queries.

  3. Prover reloaded from disk per proofclone_prover() loads ~171MB from disk for each individual proof. If multiple UTXOs are withdrawn, this could be very slow.

  4. CircuitGenerationProgress declared but unused — no progress reporting during the 10-30 minute circuit generation.

Low Severity

  1. No RustLib.init() guard — wormhole APIs will crash if called before SDK initialization.
  2. Sensitive data (secretHex) not zeroed — passed through FFI and stored in Dart without zeroize.
  3. Mutex panic on poisonaggregator.inner.lock() will panic if a previous proof generation panicked.
  4. log::info! in hot paths — proof generation and aggregation log at info level; should be debug.

Existing Tests

Rust (quantus_sdk/rust/src/api/wormhole.rs) — 10 unit tests

Test Coverage
test_derive_wormhole_pair Output format and field lengths
test_derive_deterministic Same mnemonic → same result
test_different_indices_produce_different_addresses Index 0 vs 1
test_different_purposes_produce_different_addresses Purpose 0 vs 1
test_get_wormhole_derivation_path Path string format
test_compute_nullifier_deterministic Same inputs → same nullifier
test_compute_nullifier_different_transfer_count Different count → different nullifier
test_quantize_amount Planck → quantized roundtrip
test_derive_address_from_secret Address from pair matches standalone derivation

qp-zk-circuits — Comprehensive

The circuit repo has thorough tests: circuit fragment tests, E2E prove-verify, aggregation with dummies, negative cases for block consistency and asset ID mismatches.

Dart — Zero wormhole tests

No Dart tests exist for any wormhole-related code.


Missing Tests

Rust (quantus_sdk) — Missing

Function Why it matters
first_hash_to_address Must match circuit's unspendable account derivation
compute_transfer_proof_storage_key Incorrect key = no valid storage proof
encode_digest_from_rpc_logs SCALE encoding must match chain format
compute_block_hash Must match circuit's block hash — critical integration point
compute_output_amount (fee logic) Edge cases: 0 fee, max fee, rounding
WormholeProofGenerator / WormholeProofAggregator E2E proof generation (needs circuit binaries, could be integration test)
check_circuit_binaries_exist File presence validation
Error paths Invalid mnemonic, invalid hex, missing files

Dart — All missing

Service Critical tests needed
WormholeService deriveMinerRewardsKeyPair, preimageToAddress, computeNullifier, quantizeAmount/dequantizeAmount
WormholeUtxoService GraphQL query construction, transfer parsing, nullifier filtering, _addressToHex (currently broken)
MinerWalletService Mnemonic save/load, key pair derivation, canWithdraw logic
WithdrawalService Transfer selection, fee calculation, proof orchestration (mocked), extrinsic submission
TransferTrackingService Event decoding, disk persistence, load/clear logic, nullifier checking
CircuitManager Status checking, file presence validation
WithdrawalScreen Form validation, address validation, amount validation, withdrawal flow

Summary

The wormhole implementation is architecturally sound — it correctly layers Rust ZK circuits → FFI bridge → Dart services → UI. The circuit integration with qp-zk-circuits is well-aligned through shared crate dependencies. However:

  • 2 bugs need fixing before merge (_isNullifierConsumed stub, _addressToHex stub)
  • Dead code in WithdrawalService should be cleaned up
  • Rust unit tests cover address derivation and nullifiers well, but miss storage key computation, block hash, and digest encoding
  • Dart has zero wormhole tests — this is the biggest gap given how much new service logic was added
  • Performance concern with prover reload per proof deserves attention for multi-UTXO withdrawals

@illuzen illuzen marked this pull request as ready for review April 2, 2026 07:55
source: hosted
version: "0.8.13+6"
version: "0.8.13+3"
image_picker_linux:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

hmm why is image picker in the mobile app changed?

can we remove this change, or do we actually need this version of the picker for some reason

Copy link
Copy Markdown
Collaborator Author

@n13 n13 left a comment

Choose a reason for hiding this comment

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

I think we should delete the change to the mobile lock file

Also this should be merged into the new_testnet_planck branch, not main

Main is still on old testnet (for not much longer but anyway)

Also CI failing

@n13
Copy link
Copy Markdown
Collaborator Author

n13 commented Apr 2, 2026

Notes:
We already stored secrets in a secure storage, we can store the secretHex the same way we store the mnemonic


PR #407 — Wormhole Implementation Review

Branch: illuzen/wormhole | 56 commits | 285 files changed | +69k / -3k lines | CI: passing

Overview

This PR implements the wormhole withdrawal system across three layers: Rust ZK proof generation, Dart SDK services, and the miner app UI. The bulk of the line count (~50k) is generated Planck pallet bindings. The core implementation is ~5k lines of hand-written code.

Architecture — Looks Good

The layered design is clean and well-separated:

User → WithdrawalScreen → WithdrawalService (miner-app, thin wrapper)
  → WormholeWithdrawalService (SDK, orchestration)
    → WormholeProofGenerator → FFI → Rust wormhole.rs → qp-zk-circuits
    → WormholeProofAggregator → FFI → Rust wormhole.rs → qp-wormhole-aggregator
    → SubstrateService.submitUnsignedExtrinsic → chain

Supporting services are well-factored:

  • MinerStateService — centralized reactive state with streams
  • TransferTrackingService — monitors blocks for transfers, persists to disk
  • WormholeAddressManager — HD address derivation and tracking (with MnemonicProvider abstraction for testability)
  • CircuitManager — asset extraction and status checking
  • WormholeUtxoService — GraphQL queries against Subsquid indexer

Previously Reported Bugs — Fixed

The _addressToHex stub (#410) and compute_block_hash_internal mismatch (#409) have been resolved.


Issues Found

HIGH — Sensitive secrets persisted in plaintext

WormholeAddressManager serializes secretHex to wormhole_addresses.json on disk as plain JSON. This is the wormhole secret — if compromised, an attacker can generate proofs and drain all funds from those addresses.

  Map<String, dynamic> toJson() => {'address': address, 'purpose': purpose, 'index': index, 'secretHex': secretHex};

Recommendation: Either re-derive secrets on demand from the mnemonic (which is stored in secure storage), or encrypt the file. Storing raw secrets in plaintext undermines the secure mnemonic storage.

HIGH — _isNullifierConsumed returns false on RPC failure

In transfer_tracking_service.dart, if the RPC call to check nullifier consumption fails, the method silently returns false (meaning "not spent"). This causes the UI to show inflated balances and could waste expensive proof generation on already-spent UTXOs.

    } catch (e) {
      _log.e('Failed to check nullifier', error: e);
      return false;
    }

Recommendation: Propagate the error (or at least return true as a safe default to avoid double-spend attempts), or mark the transfer status as "unknown" and require a successful check before allowing withdrawal.

MEDIUM — DRY violations: utility functions duplicated

_hexToBytes, _bytesToHex, and _twox128 are duplicated across:

  • wormhole_withdrawal_service.dart (lines 996-1017)
  • transfer_tracking_service.dart (lines 367-380, 500-511)

Both implement identical logic. Per your DRY rule, these should be extracted into a shared utility.

MEDIUM — Redundant block header fetch in withdrawal

_fetchBlockHeader() fetches the header, then _fetchStorageProof() fetches it again for the same block to get the state root (line 743-760 of wormhole_withdrawal_service.dart). The state root is already available from the header. This doubles the RPC calls during proof generation.

MEDIUM — WormholeService() instantiated repeatedly

In wormhole_withdrawal_service.dart, WormholeService() is constructed ad-hoc in withdraw() (line 175), _generateProofForTransfer() (line 420), and _fetchBlockHeader() (line 639). It should be a field on the class.

MEDIUM — Aggregator clear() is a no-op

The Rust WormholeProofAggregator::clear() does nothing (line 914-919 of wormhole.rs) and logs a warning. The Dart side still exposes it as a real API. The withdrawal service works around this by creating new aggregators per batch, but the API is misleading to consumers.

LOW — print() instead of proper logging

substrate_service.dart uses bare print() statements (lines ~130, 134 of the diff) in queryBalanceRaw.

LOW — Mutex panic on poison in Rust

WormholeProofAggregator.inner.lock() will panic (crash the app) if a previous proof generation panicked and poisoned the mutex. A more resilient pattern would handle the poisoned case gracefully.

LOW — Missing RustLib.init() guard

Calling wormhole FFI functions before QuantusSdk.init() will crash without a helpful error message. No guard exists.


Test Coverage

Rust (12 tests) — Solid core coverage:

  • Address derivation (determinism, uniqueness across purposes/indices)
  • Nullifier computation
  • Amount quantization/dequantization
  • Block hash SDK-vs-circuit consistency (2 test cases with known fixtures)
  • P2/P3 Poseidon2 implementation consistency

Missing Rust tests:

  • compute_transfer_proof_storage_key
  • encode_digest_from_rpc_logs
  • compute_output_amount edge cases (0 fee, max fee, overflow)
  • Error paths (invalid mnemonic, invalid hex, missing circuit files)

Dart — Zero wormhole tests. This is the biggest gap. Key untested logic:

  • Transfer selection (_selectTransfers) — greedy largest-first algorithm
  • Output/change amount calculation in proof generation
  • Secret resolution logic (_resolveTransferSecret)
  • TrackedTransfer JSON serialization round-trip
  • WormholeAddressManager state management and persistence
  • CircuitManager file extraction flow

Summary

The architecture is sound and the Rust/circuit integration is well-aligned. The code is generally clean and well-documented. The two high-severity issues (secrets in plaintext, silent RPC failure) should be addressed before merge. The DRY violations and redundant RPC calls are worth cleaning up. The lack of Dart tests is a significant gap for a system handling funds.

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