Security/zk core hardening

.github/workflows/test.yml

@@ -75,6 +75,13 @@ jobs:
       - name: Check workspace
         run: cargo check --release --workspace
 
+      # zk-core must build in a no_std/WASM context. Guard against a consumer or
+      # feature change that leaks `std` into the runtime build.
+      - name: Check zk-core no_std feature matrix
+        run: |
+          cargo check -p orbinum-zk-core --no-default-features
+          cargo check -p orbinum-zk-core --no-default-features --features poseidon-native
+
   # Step 3: Build workspace - compiles once for all subsequent jobs to reuse
   build:
     name: Build

primitives/zk-core/CHANGELOG.md

@@ -4,6 +4,61 @@ All notable changes to this crate are documented here.
 
 ---
 
+## [1.1.0] — 2026-07-02
+
+### Security
+- Poseidon hashing no longer uses `.expect()` / `panic!`. All arities
+  (`hash_2`, `hash_4`, `hash_5`, `poseidon_hash_1`) now route through a single
+  internal `poseidon_circom` helper that returns a deterministic value instead of
+  panicking. This closes a consensus-divergence risk: a native panic aborts the
+  node process while a WASM panic is a recoverable trap, so the same malformed
+  input could split the network. The failure path is provably unreachable (constant
+  arity, reduced `Fr` inputs) and is now documented as an invariant. The fallback
+  value is a fixed non-zero marker, never zero, so it cannot be mistaken for the
+  all-zero dummy sentinel (a zero nullifier is skipped rather than marked spent).
+- Poseidon host functions (`poseidon_hash_2`, `poseidon_hash_4`) no longer
+  `assert_eq!` input length. A failed assert in a native host function aborts the
+  node process. Inputs are now read through a `read_32_le` helper that zero-pads or
+  truncates to 32 bytes deterministically and never panics.
+- The 32-byte field↔bytes conversions (`field_to_bytes`, `bytes_to_field`, host
+  output) now copy by length instead of slicing `[..32]`, removing a latent panic
+  if a bigint ever serialized to fewer than 32 bytes.
+
+### Changed
+- `Note::is_zero` now matches the system-wide dummy definition (`value == 0`)
+  instead of also requiring `asset_id`/`owner_pubkey`/`blinding` to be zero. The
+  circuit and shielded-pool treat any value-zero input as dummy; the previous
+  stricter check could disagree. Documented on both `is_zero` and `zero`.
+
+### Documentation
+- Documented in `Cargo.toml` that runtime/WASM consumers must depend on this crate
+  with `default-features = false` (else `std` leaks into the `no_std` build), and
+  added a CI feature-matrix check to enforce it.
+
+### Added
+- `FieldElement::is_canonical_le` and `FieldElement::from_canonical_le` — reject
+  non-canonical little-endian byte encodings (values `>= p`). Trust boundaries that
+  ingest raw bytes (nullifiers, commitments, external addresses) should use these so
+  that `x` and `x + p` cannot masquerade as distinct values while hashing/proving
+  identically.
+- Documented the consensus invariant `Native(x) == Wasm(x) == Circuit(x)` in the
+  crate docs.
+
+### Tests
+- Added `hash_no_panic_on_boundary_inputs`, `hash_boundary_inputs_are_deterministic`,
+  and `poseidon_circom_core_matches_trait` covering boundary field elements
+  (`0` and `p-1`) and native/WASM path equivalence.
+- Added `read_32_le_pads_and_truncates`, `hash_2_no_panic_on_short_input`,
+  `hash_4_no_panic_on_short_input`, and `hash_2_still_correct_for_32_bytes`
+  covering non-32-byte host-function inputs.
+- Added `tests/poseidon_vectors.rs`: known-answer vectors against canonical
+  circomlib BN254 Poseidon outputs (arities 1/2/4/5) and native≡WASM equivalence
+  checks. The vectors catch any parameter drift between `light-poseidon-nostd` and
+  the compiled circuit.
+- Added canonical-encoding tests including the `n` vs `n+p` collision vector.
+
+---
+
 ## [1.0.1] — 2026-05-14
 
 ### Changed

primitives/zk-core/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "orbinum-zk-core"
-version = "1.0.1"
+version = "1.1.0"
 authors = ["Orbinum Network <contact@orbinum.net>"]
 edition = "2021"
 license = "Apache-2.0 OR GPL-3.0-or-later"
@@ -40,6 +40,14 @@ ark-std = { version = "0.5.0", default-features = false, features = ["std"] }
 serde_json = "1.0"
 
 [features]
+# `default` enables `std` + native host functions for host tooling and tests.
+#
+# IMPORTANT: runtime/WASM consumers MUST depend on this crate with
+# `default-features = false` and opt into `poseidon-native` explicitly, otherwise
+# they pull in `std` (breaks the `no_std` WASM build). All in-tree consumers do
+# this correctly; verify with:
+#   cargo build -p orbinum-zk-core --no-default-features
+#   cargo build -p orbinum-zk-core --no-default-features --features poseidon-native
 default = ["std", "poseidon-native"]
 std = [
 	"ark-bn254/std",
@@ -50,5 +58,6 @@ std = [
 	"sp-runtime-interface/std",
 ]
 
-# Native Poseidon hashing - enabled by default for optimal performance
+# Native Poseidon host functions. `no_std`-compatible: generates the WASM-side
+# import plus the native implementation. Enable explicitly in runtime consumers.
 poseidon-native = ["sp-runtime-interface"]

primitives/zk-core/src/hash.rs

@@ -9,6 +9,63 @@ use crate::types::FieldElement;
 use ark_bn254::Fr;
 use light_poseidon_nostd::{Poseidon, PoseidonHasher as LightHasher};
 
+// ─── Circom Poseidon core ───────────────────────────────────────────────────────
+
+/// Compute the circomlib Poseidon hash for a fixed arity.
+///
+/// # Consensus determinism
+///
+/// This is the ONLY place that invokes `light-poseidon-nostd`. Both the WASM path
+/// ([`LightPoseidonHasher`]) and the native path ([`NativePoseidonHasher`], via the
+/// host function that internally calls `LightPoseidonHasher`) go through here, so
+/// their behavior is identical by construction.
+///
+/// # Invariant (why it cannot fail)
+///
+/// - `new_circom(nr_inputs)` only fails with `InvalidWidthCircom` when
+///   `nr_inputs + 1 > MAX_X5_LEN (= 13)`. The arities in use (1, 2, 4, 5) yield
+///   width 2/3/5/6, always valid.
+/// - `hash(&inputs)` only fails with `InvalidNumberOfInputs` when
+///   `inputs.len() != nr_inputs`. Callers pass fixed-length arrays matching the
+///   arity.
+///
+/// Inputs are already-reduced `Fr` (not raw bytes), so no parsing/range error
+/// variant applies.
+///
+/// # Failure behavior (unreachable)
+///
+/// If the invariant were ever violated (a programming bug), we return a
+/// **deterministic** value instead of `panic!`. This guarantees native and WASM
+/// never diverge in behavior: a native `panic!` aborts the node process while in
+/// WASM it is a recoverable trap — a divergence that would break consensus. Never
+/// introduce `.expect()`/`panic!` in this path.
+///
+/// The fallback is deliberately NOT zero: a zero commitment/nullifier is the dummy
+/// sentinel elsewhere in the system, so a zero fallback could be silently skipped
+/// (e.g. an unspent nullifier). We return a fixed non-zero domain-separated marker
+/// so any downstream use of a fallback value stands out rather than masquerading as
+/// a dummy. This value is never produced by a real Poseidon hash of valid inputs in
+/// practice, and the branch is unreachable regardless.
+#[inline]
+fn poseidon_circom(nr_inputs: usize, inputs: &[Fr]) -> Fr {
+	debug_assert_eq!(
+		inputs.len(),
+		nr_inputs,
+		"poseidon_circom: arity mismatch (broken invariant)"
+	);
+	match Poseidon::<Fr>::new_circom(nr_inputs).and_then(|mut p| p.hash(inputs)) {
+		Ok(result) => result,
+		// Unreachable with constant arity and `Fr` inputs (see invariant above).
+		// Non-zero deterministic marker: identical on native and WASM, never panics,
+		// and never collides with the all-zero dummy sentinel.
+		Err(_) => Fr::from(POSEIDON_FALLBACK_MARKER_U64),
+	}
+}
+
+/// Non-zero, deterministic fallback for the unreachable Poseidon failure branch.
+/// Chosen so it cannot be confused with the all-zero dummy sentinel.
+const POSEIDON_FALLBACK_MARKER_U64: u64 = 0xDEAD_BEEF_DEAD_BEEF;
+
 // ─── Trait ────────────────────────────────────────────────────────────────────
 
 /// Abstraction over Poseidon hash implementations.
@@ -33,37 +90,34 @@ pub struct LightPoseidonHasher;
 
 impl PoseidonHasher for LightPoseidonHasher {
 	fn hash_2(&self, inputs: [FieldElement; 2]) -> FieldElement {
-		let result = Poseidon::<Fr>::new_circom(2)
-			.expect("Poseidon init (2 inputs) failed")
-			.hash(&[inputs[0].inner(), inputs[1].inner()])
-			.expect("Poseidon hash_2 failed");
+		let result = poseidon_circom(2, &[inputs[0].inner(), inputs[1].inner()]);
 		FieldElement::new(result)
 	}
 
 	fn hash_4(&self, inputs: [FieldElement; 4]) -> FieldElement {
-		let result = Poseidon::<Fr>::new_circom(4)
-			.expect("Poseidon init (4 inputs) failed")
-			.hash(&[
+		let result = poseidon_circom(
+			4,
+			&[
 				inputs[0].inner(),
 				inputs[1].inner(),
 				inputs[2].inner(),
 				inputs[3].inner(),
-			])
-			.expect("Poseidon hash_4 failed");
+			],
+		);
 		FieldElement::new(result)
 	}
 
 	fn hash_5(&self, inputs: [FieldElement; 5]) -> FieldElement {
-		let result = Poseidon::<Fr>::new_circom(5)
-			.expect("Poseidon init (5 inputs) failed")
-			.hash(&[
+		let result = poseidon_circom(
+			5,
+			&[
 				inputs[0].inner(),
 				inputs[1].inner(),
 				inputs[2].inner(),
 				inputs[3].inner(),
 				inputs[4].inner(),
-			])
-			.expect("Poseidon hash_5 failed");
+			],
+		);
 		FieldElement::new(result)
 	}
 }
@@ -109,15 +163,17 @@ fn field_to_bytes(field: Fr) -> [u8; 32] {
 	use ark_ff::{BigInteger, PrimeField};
 	let bytes = field.into_bigint().to_bytes_le();
 	let mut result = [0u8; 32];
-	result.copy_from_slice(&bytes[..32]);
+	let n = bytes.len().min(32);
+	result[..n].copy_from_slice(&bytes[..n]);
 	result
 }
 
 #[cfg(feature = "poseidon-native")]
 fn bytes_to_field(bytes: &[u8]) -> FieldElement {
 	use ark_ff::PrimeField;
 	let mut arr = [0u8; 32];
-	arr.copy_from_slice(&bytes[..32]);
+	let n = bytes.len().min(32);
+	arr[..n].copy_from_slice(&bytes[..n]);
 	FieldElement::new(Fr::from_le_bytes_mod_order(&arr))
 }
 
@@ -131,10 +187,7 @@ fn bytes_to_field(bytes: &[u8]) -> FieldElement {
 /// Single-input Poseidon is intentionally not part of [`PoseidonHasher`] because
 /// it is only needed for the value_proof circuit.
 pub fn poseidon_hash_1(input: FieldElement) -> FieldElement {
-	let result = Poseidon::<Fr>::new_circom(1)
-		.expect("Poseidon init (1 input) failed")
-		.hash(&[input.inner()])
-		.expect("Poseidon hash_1 failed");
+	let result = poseidon_circom(1, &[input.inner()]);
 	FieldElement::new(result)
 }
 
@@ -216,4 +269,79 @@ mod tests {
 		let h2 = LightPoseidonHasher.hash_2([input, FieldElement::from_u64(0)]);
 		assert_ne!(h1, h2);
 	}
+
+	// ─── No panic on boundary inputs, deterministic behavior ───────────────────
+
+	/// `Fr = p - 1`, the largest canonical field element. Must not panic or diverge.
+	fn field_max() -> FieldElement {
+		use ark_ff::Field;
+		// -1 in the field == p - 1.
+		FieldElement::new(-Fr::ONE)
+	}
+
+	#[test]
+	fn hash_no_panic_on_boundary_inputs() {
+		let h = LightPoseidonHasher;
+		let zero = FieldElement::zero();
+		let max = field_max();
+		// None of these calls must panic (they previously used `.expect()`).
+		let _ = h.hash_2([zero, max]);
+		let _ = h.hash_2([max, max]);
+		let _ = h.hash_4([zero, max, zero, max]);
+		let _ = h.hash_5([max, max, max, max, max]);
+		let _ = poseidon_hash_1(max);
+	}
+
+	#[test]
+	fn hash_boundary_inputs_are_deterministic() {
+		let h = LightPoseidonHasher;
+		let max = field_max();
+		assert_eq!(h.hash_2([max, max]), h.hash_2([max, max]));
+		assert_eq!(
+			h.hash_4([max, max, max, max]),
+			h.hash_4([max, max, max, max])
+		);
+		assert_eq!(poseidon_hash_1(max), poseidon_hash_1(max));
+	}
+
+	#[test]
+	fn poseidon_circom_core_matches_trait() {
+		// The internal core and the trait produce the same result (shared path).
+		let a = FieldElement::from_u64(7);
+		let b = FieldElement::from_u64(11);
+		let via_trait = LightPoseidonHasher.hash_2([a, b]);
+		let via_core = FieldElement::new(poseidon_circom(2, &[a.inner(), b.inner()]));
+		assert_eq!(via_trait, via_core);
+	}
+
+	// ─── Defensive 32-byte conversion (no panic on short/long) ─────────────────
+
+	#[cfg(feature = "poseidon-native")]
+	#[test]
+	fn field_to_bytes_roundtrips_boundary_values() {
+		use ark_ff::Field;
+		// 0, 1, and p-1 must all round-trip through the 32-byte conversion.
+		for f in [Fr::from(0u64), Fr::from(1u64), -Fr::ONE] {
+			let bytes = field_to_bytes(f);
+			assert_eq!(bytes.len(), 32);
+			let back = bytes_to_field(&bytes);
+			assert_eq!(back, FieldElement::new(f));
+		}
+	}
+
+	#[cfg(feature = "poseidon-native")]
+	#[test]
+	fn bytes_to_field_no_panic_on_short_or_long() {
+		// Shorter and longer inputs must not panic (previously copy_from_slice would).
+		let _ = bytes_to_field(&[1u8, 2, 3]);
+		let _ = bytes_to_field(&[]);
+		let _ = bytes_to_field(&[0xABu8; 40]);
+	}
+
+	#[test]
+	fn fallback_marker_is_non_zero() {
+		// The unreachable failure branch must never yield the all-zero dummy sentinel.
+		assert_ne!(super::POSEIDON_FALLBACK_MARKER_U64, 0);
+		assert!(!FieldElement::new(Fr::from(super::POSEIDON_FALLBACK_MARKER_U64)).is_zero());
+	}
 }