fix(zk-verifier): bound deserialization input and stop circuit-id aliasing

[nol4lej]

What this fixes

Four hardening changes in the ZK verifier. None is reachable today — all four are one careless caller away from mattering.

Unbounded allocation from a length prefix

A verifying key's point vector is length-prefixed, and ark-serialize calls Vec::with_capacity on that prefix before reading a single element. A key declaring 2^40 points asks the allocator for roughly 48 GB on nothing but submitted bytes — a trap in Wasm, an OOM on a native path.

Both on-chain callers already bound their argument at 8 KiB, so nothing was exposed. But the bound lived in the caller: to_ark_vk, prepare and num_public_inputs are public API with no length precondition, so any future caller — a runtime API, an offchain worker, an unsigned path — inherited the reservation unguarded.

The guard now lives in the deserializer. MAX_VK_BYTES is 8 KiB, matching the extrinsic so the crate is not the narrower gate; MAX_PROOF_BYTES is 1 KiB against a compressed Groth16 proof that is a fixed 128 bytes. prepare and num_public_inputs route through to_ark_vk, so they inherit the check.

A declared limit that was never applied

MAX_PUBLIC_INPUTS had zero call sites outside its own definition and re-export. The pallet bounds its extrinsic argument, but the ZkVerifierPort path does not go through that extrinsic — so the constant documented a limit nothing enforced.

Now applied in to_field_elements, the single point every input passes through. Every circuit in use declares 7 inputs or fewer, so the limit cannot bite a real proof.

Circuit ids that aliased

expected_public_inputs takes a u8, and ensure_vk_arity reached it through circuit_id.0 as u8. Id 257 therefore mapped onto 1: a key was validated against the transfer circuit's arity and then stored under an id no lookup could reach.

Now guarded with u8::try_from — the same way purge_circuit already guarded the same table. Root-gated, so this amplifies an operator error rather than enabling an attack; what made it worth closing is that it failed silently.

Ids inside u8 but outside the known table are unaffected: they carry no expected arity, so only "deserializes as a BN254 key" applies.

Genesis accepted keys it never checked

build validated length and nothing else, so a well-sized but meaningless key was stored and the chain only discovered it when the first real proof failed to verify — at which point nothing distinguishes a bad key from a bad proof.

Genesis now routes through the same ensure_vk_arity as registration.

Smaller items

Saturating arithmetic in estimate_verification_cost, since release builds do not enable overflow-checks and plain */+ would wrap into a plausible-looking underestimate. A defensive clamp in hash_pair_poseidon mirroring recipient_to_field: BN254 Fr always yields 32 bytes so it never binds, but this runs on block import, where slicing past the end panics the node rather than failing a call. And a helper doc that claimed the transfer circuit has arity 5 against a constant of 7 — rewritten to read the constant rather than restate the number, which is how the two drifted apart.

Breaking change

A genesis config carrying placeholder verification keys now prevents the chain from starting, where before it started and failed later. Two of this pallet's own tests were doing exactly that (vec![0xCCu8; 300]) and were updated to real keys.

Worth checking testnet chain specs before deploying — this is the only change here that can bite at startup.

On the tests

Each guard was verified by removing it and confirming the test fails.

Two early drafts did not fail. They fed the deserializer filler bytes, which also fail to deserialize — so the error arrived by another route and the test passed whether or not the guard existed. They proved nothing.

Rewritten with artifacts that would otherwise succeed: a genuine BN254 key at arity 400 (over 8 KiB, and deserializable), and a real proof padded past the bound. deserialize_compressed ignores trailing bytes, so the padded proof stays valid and the size guard is the only thing rejecting it. Both now fail when the guard is removed.

The same check was applied to the aliasing test: it fails with as u8, passes with u8::try_from.

Two constant-only assertions were converted to const blocks. Clippy flagged one — with constants on both sides the compiler folds the condition and drops the assert, so the test ran without checking anything. As const assertions they fail the build instead.

Verification

Unit tests	490 (11 new)
Clippy, CI feature set, -D warnings	clean
Staged tree builds on its own	yes
Dev-node E2E	9/9

The E2E covers what unit tests cannot: genesis keys pass the new check at node startup, all three real verifying keys register through setup-dev-local.sh (488, 488 and 392 bytes), an 8 KiB filler key is refused with InvalidVerificationKey, circuit id 257 is refused and stores nothing, and the chain keeps producing blocks throughout.

One E2E expectation was wrong at first: a 9 KiB key exceeds the extrinsic's own BoundedVec, so the transaction never decodes and never reaches dispatch. That is Substrate working as designed — the node logs an invalid-transaction notice and carries on. The test now asserts rejection at admission, and a separate 8 KiB case exercises the in-crate guard, which is the one that actually reaches the pallet.

Versions

orbinum-zk-verifier 1.3.0 → 1.4.0 (new public constants), pallet-zk-verifier 0.11.0 → 0.12.0 (stricter validation).