Fix/zk verifier hardening

frame/zk-verifier/CHANGELOG.md

@@ -6,6 +6,39 @@ All notable changes to this pallet are documented here.
 
 ## [Unreleased]
 
+## [0.19.0] - 2026-10-21
+
+### Added
+
+- `DeniedVkHashes`: an on-chain, append-only record of `blake2_256` hashes of keys
+  found unsound. A denied key is refused by `register_verification_key`,
+  `batch_register_verification_keys`, `set_active_version` and `unretire_version`
+  (`DeniedVerificationKey`), and a stored one verifies nothing, active or not
+  (`UnsupportedCircuitVersion`; `is_supported_version` is false). No call removes an
+  entry, `remove_verification_key` and `purge_circuit` included, so a Root mistake —
+  unretiring, re-registering under another version — cannot bring the key back.
+- `deny_version(circuit_id, version)` (Root, call index 8): retires the version and
+  records its key's hash (`VerificationKeyDenied`). Never the active version; a hash
+  already recorded is refused (`KeyAlreadyDenied`).
+- `GenesisConfig::denied_vk_hashes`: the record a new chain starts with, written before
+  any genesis key.
+- `RemovedVkHashes`: `remove_verification_key` records the hash of the key the version
+  held, and that version can then only take the same key again (`VersionReused`
+  otherwise, also in `batch_register_verification_keys`). Before, removing a version
+  and registering it with another key changed its meaning for any client that cached
+  it. Append-only: `purge_circuit` does not clear it.
+
+### Changed
+
+- `ZkVerifierPort::verification_weight` adds one storage read, the `VkHashes` lookup of
+  the denied-key check.
+- `WeightInfo::deny_version`: estimated from `retire_version` until benchmarked.
+- `register_verification_key` and `batch_register_verification_keys` add one read per
+  key (`RemovedVkHashes`), `remove_verification_key` one read and one write.
+- Registration refuses a key without an independent `delta` (`delta = ±gamma` or
+  `± G2`, a phase-2-less snarkjs key), with which anyone could forge proofs
+  (`InvalidVerificationKey`, through `orbinum-zk-verifier`).
+
 ## [0.18.1] - 2026-10-21
 
 ### Changed

frame/zk-verifier/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "pallet-zk-verifier"
-version = "0.18.1"
+version = "0.19.0"
 description = "Zero-Knowledge proof verification pallet for Orbinum"
 authors = { workspace = true }
 license = "GPL-3.0-or-later"
@@ -21,6 +21,7 @@ serde = { version = "1.0", default-features = false, features = ["derive", "allo
 
 # Substrate primitives
 
+sp-core = { version = "43.0.0", default-features = false, features = ["serde"] }
 sp-io = { version = "48.0.0", default-features = false }
 sp-runtime = { version = "48.0.0", default-features = false }
 sp-std = { version = "14.0.0", default-features = false }
@@ -43,6 +44,7 @@ orbinum-zk-verifier = { path = "../../primitives/zk-verifier", default-features
 [dev-dependencies]
 orbinum-zk-core = { path = "../../primitives/zk-core", default-features = false }
 orbinum-zk-verifier = { path = "../../primitives/zk-verifier", default-features = false }
+serde_json = "1.0"
 # Build real Groth16 VKs of a given arity in tests (arity is now validated on register).
 ark-bn254 = { version = "0.5.0", default-features = false, features = ["curve"] }
 ark-ec = { version = "0.5.0", default-features = false }
@@ -54,6 +56,7 @@ std = [
 	"parity-scale-codec/std",
 	"scale-info/std",
 	"serde/std",
+	"sp-core/std",
 	"sp-io/std",
 	"sp-runtime/std",
 	"sp-std/std",

frame/zk-verifier/src/benchmarking.rs

@@ -41,15 +41,15 @@ mod benchmarks {
 	/// relies on `skip-proof-verification` to record weight despite the failed pairing.
 	fn synthetic_vk(arity: usize) -> Vec<u8> {
 		use ark_bn254::{Bn254, G1Affine, G2Affine};
-		use ark_ec::AffineRepr;
+		use ark_ec::{AffineRepr, CurveGroup};
 		use ark_groth16::VerifyingKey as ArkVk;
 		use orbinum_zk_verifier::VerifyingKey;
 
 		let vk = ArkVk::<Bn254> {
 			alpha_g1: G1Affine::generator(),
 			beta_g2: G2Affine::generator(),
 			gamma_g2: G2Affine::generator(),
-			delta_g2: G2Affine::generator(),
+			delta_g2: (G2Affine::generator() + G2Affine::generator()).into_affine(),
 			gamma_abc_g1: (0..=arity).map(|_| G1Affine::generator()).collect(),
 		};
 		VerifyingKey::from_ark_vk(&vk)
@@ -208,6 +208,22 @@ mod benchmarks {
 		assert!(!RetiredVersions::<T>::contains_key(circuit_id, retired));
 	}
 
+	#[benchmark]
+	fn deny_version() {
+		let circuit_id = CircuitId::TRANSFER;
+		let active = 1u32;
+		let target = 2u32;
+
+		insert_sample_vk::<T>(circuit_id, active);
+		insert_sample_vk::<T>(circuit_id, target);
+		ActiveCircuitVersion::<T>::insert(circuit_id, active);
+
+		#[extrinsic_call]
+		_(RawOrigin::Root, circuit_id, target);
+
+		assert!(RetiredVersions::<T>::contains_key(circuit_id, target));
+	}
+
 	/// Uses circuit id 99 rather than the retired 5: the extrinsic only accepts
 	/// ids the runtime does not implement, and 99 stays unknown even if 5 is
 	/// ever reassigned.

frame/zk-verifier/src/lib.rs

@@ -146,20 +146,46 @@ pub mod pallet {
 		ValueQuery,
 	>;
 
+	/// `blake2_256` of every key denied on this chain (`deny_version`, genesis): the
+	/// record of circuits found unsound. Such a key is refused on register, activate
+	/// and unretire, and verifies nothing if stored. Append-only: no call removes an
+	/// entry, `remove_verification_key` and `purge_circuit` included.
+	#[pallet::storage]
+	pub type DeniedVkHashes<T: Config> = StorageMap<_, Identity, [u8; 32], (), OptionQuery>;
+
+	/// `blake2_256` of the key a removed version held. That version can hold only the
+	/// same key again, so a client that cached what it means is never served another
+	/// key under it. Append-only: nothing clears it, `purge_circuit` included.
+	#[pallet::storage]
+	pub type RemovedVkHashes<T: Config> = StorageDoubleMap<
+		_,
+		Blake2_128Concat,
+		CircuitId,
+		Blake2_128Concat,
+		u32,
+		[u8; 32],
+		OptionQuery,
+	>;
+
 	// ── Genesis ───────────────────────────────────────────────────────────────
 
 	#[pallet::genesis_config]
 	#[derive(frame_support::DefaultNoBound)]
 	pub struct GenesisConfig<T: Config> {
 		/// One key per circuit, registered as version 1 and activated.
 		pub verification_keys: Vec<(CircuitId, Vec<u8>)>,
+		/// `DeniedVkHashes` a new chain starts with, recorded before any key.
+		pub denied_vk_hashes: Vec<sp_core::H256>,
 		#[serde(skip)]
 		pub _phantom: PhantomData<T>,
 	}
 
 	#[pallet::genesis_build]
 	impl<T: Config> BuildGenesisConfig for GenesisConfig<T> {
 		fn build(&self) {
+			for hash in &self.denied_vk_hashes {
+				DeniedVkHashes::<T>::insert(hash.0, ());
+			}
 			for (circuit_id, vk_bytes) in &self.verification_keys {
 				let key_data: VkBytes = vk_bytes
 					.clone()
@@ -218,6 +244,12 @@ pub mod pallet {
 		BatchVerificationKeysRegistered { count: u32 },
 		/// Every version of a retired circuit was purged from storage.
 		CircuitPurged { circuit_id: CircuitId, removed: u32 },
+		/// A version was retired for good and its key's hash recorded as denied.
+		VerificationKeyDenied {
+			circuit_id: CircuitId,
+			version: u32,
+			vk_hash: [u8; 32],
+		},
 	}
 
 	// ── Errors ────────────────────────────────────────────────────────────────
@@ -260,6 +292,12 @@ pub mod pallet {
 		// Purge
 		CircuitStillInUse,
 		CircuitHasNoStorage,
+		/// The key's hash is in `DeniedVkHashes`.
+		DeniedVerificationKey,
+		/// `deny_version`: the key's hash is already denied.
+		KeyAlreadyDenied,
+		/// The version was removed while holding another key; it can hold only that one.
+		VersionReused,
 	}
 
 	// ── Extrinsics ────────────────────────────────────────────────────────────
@@ -280,7 +318,8 @@ pub mod pallet {
 		/// semantics: any other change takes a new circuit id, and a superseded
 		/// version is retired with `retire_version`.
 		#[pallet::call_index(0)]
-		#[pallet::weight(T::WeightInfo::register_verification_key())]
+		#[pallet::weight(T::WeightInfo::register_verification_key()
+			.saturating_add(T::DbWeight::get().reads(1)))]
 		pub fn register_verification_key(
 			origin: OriginFor<T>,
 			circuit_id: CircuitId,
@@ -310,15 +349,16 @@ pub mod pallet {
 				!RetiredVersions::<T>::contains_key(circuit_id, version),
 				Error::<T>::UnsupportedCircuitVersion
 			);
-			Self::ensure_vk_arity(circuit_id, &info.key_data)?;
+			Self::ensure_vk_admitted(circuit_id, &info.key_data)?;
 
 			Self::activate(circuit_id, version);
 			Ok(())
 		}
 
 		/// Remove a version and all its data (Root only); never the active one.
 		#[pallet::call_index(2)]
-		#[pallet::weight(T::WeightInfo::remove_verification_key())]
+		#[pallet::weight(T::WeightInfo::remove_verification_key()
+			.saturating_add(T::DbWeight::get().reads_writes(1, 1)))]
 		pub fn remove_verification_key(
 			origin: OriginFor<T>,
 			circuit_id: CircuitId,
@@ -334,6 +374,9 @@ pub mod pallet {
 				.ok_or(Error::<T>::ActiveVersionNotSet)?;
 			ensure!(active != version, Error::<T>::CannotRemoveActiveVersion);
 
+			if let Some(hash) = VkHashes::<T>::get(circuit_id, version) {
+				RemovedVkHashes::<T>::insert(circuit_id, version, hash);
+			}
 			Self::remove_vk(circuit_id, version);
 			Self::deposit_event(Event::VerificationKeyRemoved {
 				circuit_id,
@@ -386,7 +429,8 @@ pub mod pallet {
 
 		/// Atomically register (and optionally activate) up to 10 VKs (Root only).
 		#[pallet::call_index(4)]
-		#[pallet::weight(T::WeightInfo::batch_register_verification_keys(entries.len() as u32))]
+		#[pallet::weight(T::WeightInfo::batch_register_verification_keys(entries.len() as u32)
+			.saturating_add(T::DbWeight::get().reads(entries.len() as u64)))]
 		pub fn batch_register_verification_keys(
 			origin: OriginFor<T>,
 			entries: BoundedVec<VkEntry, ConstU32<10>>,
@@ -457,7 +501,7 @@ pub mod pallet {
 			);
 			let info = VerificationKeys::<T>::get(circuit_id, version)
 				.ok_or(Error::<T>::VerificationKeyNotFound)?;
-			Self::ensure_vk_arity(circuit_id, &info.key_data)?;
+			Self::ensure_vk_admitted(circuit_id, &info.key_data)?;
 			RetiredVersions::<T>::remove(circuit_id, version);
 			Self::deposit_event(Event::VersionUnretired {
 				circuit_id,
@@ -466,6 +510,40 @@ pub mod pallet {
 			Ok(())
 		}
 
+		/// Deny a version for good (Root only): it is retired and its key's hash joins
+		/// `DeniedVkHashes`, so that key cannot be registered, activated, unretired or
+		/// verified again under any circuit or version. Never the active one, and no call
+		/// undoes it.
+		#[pallet::call_index(8)]
+		#[pallet::weight(T::WeightInfo::deny_version())]
+		pub fn deny_version(
+			origin: OriginFor<T>,
+			circuit_id: CircuitId,
+			version: u32,
+		) -> DispatchResult {
+			ensure_root(origin)?;
+			let info = VerificationKeys::<T>::get(circuit_id, version)
+				.ok_or(Error::<T>::VerificationKeyNotFound)?;
+			ensure!(
+				ActiveCircuitVersion::<T>::get(circuit_id) != Some(version),
+				Error::<T>::CannotRetireActiveVersion
+			);
+			let vk_hash = sp_io::hashing::blake2_256(&info.key_data);
+			ensure!(
+				!DeniedVkHashes::<T>::contains_key(vk_hash),
+				Error::<T>::KeyAlreadyDenied
+			);
+
+			DeniedVkHashes::<T>::insert(vk_hash, ());
+			RetiredVersions::<T>::insert(circuit_id, version, ());
+			Self::deposit_event(Event::VerificationKeyDenied {
+				circuit_id,
+				version,
+				vk_hash,
+			});
+			Ok(())
+		}
+
 		/// Erase every trace of a circuit the runtime no longer implements (Root
 		/// only).
 		///
@@ -549,12 +627,18 @@ pub mod pallet {
 			set_active: bool,
 		) -> DispatchResult {
 			ensure!(!key_data.is_empty(), Error::<T>::EmptyVerificationKey);
-			Self::ensure_vk_arity(circuit_id, &key_data)?;
+			Self::ensure_vk_admitted(circuit_id, &key_data)?;
 			// Never overwrite: a replaced key would silently change what verifies.
 			ensure!(
 				!VerificationKeys::<T>::contains_key(circuit_id, version),
 				Error::<T>::CircuitAlreadyExists
 			);
+			// Nor reuse a removed version for another key.
+			ensure!(
+				RemovedVkHashes::<T>::get(circuit_id, version)
+					.is_none_or(|h| h == sp_io::hashing::blake2_256(&key_data)),
+				Error::<T>::VersionReused
+			);
 			Self::store_vk(circuit_id, version, key_data)?;
 			Self::deposit_event(Event::VerificationKeyRegistered {
 				circuit_id,
@@ -575,11 +659,20 @@ pub mod pallet {
 			});
 		}
 
-		/// The rule every stored key is held to: it deserializes as a BN254 Groth16
-		/// key and its arity gives an admitted layout
+		/// Whether `hash` is in `DeniedVkHashes`.
+		pub(crate) fn is_denied(hash: &[u8; 32]) -> bool {
+			DeniedVkHashes::<T>::contains_key(hash)
+		}
+
+		/// The rule every stored key is held to: it is not denied, it deserializes as
+		/// a BN254 Groth16 key, and its arity gives an admitted layout
 		/// ([`verifier::admitted_layout`]). Ids outside the known table are only
 		/// checked to deserialize.
-		fn ensure_vk_arity(circuit_id: CircuitId, key_data: &[u8]) -> DispatchResult {
+		fn ensure_vk_admitted(circuit_id: CircuitId, key_data: &[u8]) -> DispatchResult {
+			ensure!(
+				!Self::is_denied(&sp_io::hashing::blake2_256(key_data)),
+				Error::<T>::DeniedVerificationKey
+			);
 			let arity = orbinum_zk_verifier::VerifyingKey::new(key_data.to_vec())
 				.num_public_inputs()
 				.map_err(|_| Error::<T>::InvalidVerificationKey)?;