wormhole - prover review

[n13]

These are minor

---

Key security framing: a malicious prover doesn't use this code

Right. A malicious prover would construct their own proofs without this crate.

---

Boundary interaction with the circuit proof_len bug

The fill_targets guard in StorageProof (called from this crate's fill_witness) has an off-by-one that aligns with the circuit bug:

        if self.proof.len() > MAX_PROOF_LEN {
            bail!(
                "proof length exceeds maximum allowed length: {} > {}",
                self.proof.len(),
                MAX_PROOF_LEN
            );
        }
        pw.set_target(targets.proof_len, F::from_canonical_usize(self.proof.len()))?;

The check is > when it should be >=. With MAX_PROOF_LEN = 20:

• proof.len() = 20 passes the guard and sets proof_len = 20
• In the circuit loop (i = 0..20, i.e. i = 0..19), is_leaf_node = (i == 20) is never true
• Leaf hash validation is completely skipped

This means even an honest prover with a 20-deep Merkle path silently produces a proof where the leaf data is unbound from the trie. The guard should be:

if self.proof.len() >= MAX_PROOF_LEN {

But to be clear: this is a secondary defense. The circuit must independently enforce proof_len < MAX_PROOF_LEN. A malicious prover bypasses fill_targets entirely.

---

new_from_bytes panics on deserialization failure

        let prover_only_data = ProverOnlyCircuitData::from_bytes(
            prover_only_bytes,
            &generator_serializer,
            &common_data,
        )
        .map_err(|e| anyhow!("Failed to deserialize prover only data: {}", e));

        let wormhole_circuit = WormholeCircuit::new(common_data.config.clone());
        let targets = Some(wormhole_circuit.targets());

        let circuit_data = ProverCircuitData {
            prover_only: prover_only_data.unwrap(),
            common: common_data,
        };

The .map_err(|e| anyhow!(...)) converts the error to anyhow::Error, but the function signature returns Result<Self, &'static str>. The type mismatch means ? can't be used, so .unwrap() is called instead -- panicking on any deserialization failure. This should either change the return type to anyhow::Result<Self> or use .map_err(|_| "Failed to deserialize prover only data")?.

---

Target/circuit-data mismatch risk in deserialization paths

Both new_from_bytes and new_from_files construct targets from a fresh WormholeCircuit but pair them with deserialized circuit data:

        let wormhole_circuit = WormholeCircuit::new(common_data.config.clone());
        let targets = Some(wormhole_circuit.targets());

        let circuit_data = ProverCircuitData {
            prover_only: prover_only_data.unwrap(),
            common: common_data,
        };

And identically in new_from_files:

        let wormhole_circuit = WormholeCircuit::new(common_data.config.clone());
        let targets = Some(wormhole_circuit.targets());

        let circuit_data = ProverCircuitData {
            prover_only: prover_only_data,
            common: common_data,
        };

The fresh WormholeCircuit and the deserialized circuit data are two independent circuits. The targets (wire indices) from the fresh circuit are only valid for the deserialized data if the circuit construction code hasn't changed since the binaries were generated. If a constraint, target, or gate was added/removed/reordered between the build that produced the binaries and the current code, the targets silently point to wrong wires.

The consequence: fill_witness sets wrong wire values, most likely producing an invalid proof (benign), but in adversarial corner cases could produce a proof with unintended public input values.

Compare to the safe path in new, where targets and circuit data come from the same instance:

    pub fn new(config: CircuitConfig) -> Self {
        let wormhole_circuit = WormholeCircuit::new(config);
        let partial_witness = PartialWitness::new();

        let targets = Some(wormhole_circuit.targets());
        let circuit_data = wormhole_circuit.build_prover();

        Self {
            circuit_data,
            partial_witness,
            targets,
        }
    }

A safer approach for the deserialization paths would be to serialize targets alongside the circuit data, or at minimum assert that common_data matches the freshly-built circuit's common_data.

---

Summary

Issue	Severity	Impact
proof.len() > MAX_PROOF_LEN off-by-one	Medium	Honest prover with depth-20 path silently skips leaf validation (interacts with circuit bug)
prover_only_data.unwrap() in new_from_bytes	Low	Panic instead of error return on bad input
Target/circuit-data mismatch in deserialization	Low	Wrong wire assignments if binaries are stale; likely causes proof failure