diff --git a/CHANGELOG.md b/CHANGELOG.md index 7800e03..22fc595 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,13 @@ All changes are relative to 0.2.0, the last published version. - **Structured `Error` enum** ([#54](https://github.com/paritytech/verifiable/pull/54)) - Fallible trait methods return `Error` instead of `()` - **Use uncompressed-unchecked codec for trusted domain types** ([#34](https://github.com/paritytech/verifiable/pull/34)) +- **`MembersCommitment` holds only the ring commitment** ([#62](https://github.com/paritytech/verifiable/pull/62)) + - Wraps `ark_vrf::ring::RingCommitment` instead of `RingVerifierKey`, dropping the + embedded KZG verifier key; `MEMBERS_COMMITMENT_SIZE` shrinks from 768 to 288 bytes + - Verification rebuilds the verifier key from the commitment and the suite's canonical + KZG key via `ark_vrf::ring::verifier_key_from_commitment`, so the trusted setup can no + longer be influenced by a decoded member set (the runtime verifier-key pinning check is + removed; a commitment built under a foreign SRS now simply fails the pairing check) ### Added - **Multi-context proof creation and validation** ([#37](https://github.com/paritytech/verifiable/pull/37), @@ -40,6 +47,10 @@ All changes are relative to 0.2.0, the last published version. - **Batch proof validation** ([#26](https://github.com/paritytech/verifiable/pull/26), [#61](https://github.com/paritytech/verifiable/pull/61)) - Added `batch_validate` method and `BatchProofItem` type + - Added `batch_validate_per_item`, returning one outcome per item for callers batching + proofs from untrusted submitters; the ring implementation keeps the single combined + check as the fast path and bisects only on failure to attribute it to the offending + items (#TODO) - **Pluggable verifier/prover caches.** `RingSuiteExt` carries `VerifierCache` and `ProverCache` associated types (with a `NullCache` no-op impl). The Bandersnatch suite ships static caches so verification does not recompute `PiopParams` on every call diff --git a/README.md b/README.md index f398021..9294906 100644 --- a/README.md +++ b/README.md @@ -21,9 +21,11 @@ The [`Verifiable`] trait defines the full API: and alias(es). - **Proof validation**: `validate` / `validate_multi_context` verify a proof and return the alias(es). `batch_validate` verifies multiple independent proofs - efficiently in a single batched check; each `BatchProofItem` carries its own - config and members, so a batch may mix proofs from different rings, even rings - of different sizes. + efficiently in a single all-or-nothing batched check; `batch_validate_per_item` + additionally attributes failures, reporting a per-item outcome so one invalid + proof does not affect the others. Each `BatchProofItem` carries its own config + and members, so a batch may mix proofs from different rings, even rings of + different sizes. - **Plain signatures**: `sign` / `verify_signature` for non-anonymous signatures attributable to a specific member. diff --git a/src/lib.rs b/src/lib.rs index a16f4f3..0029df2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -114,8 +114,8 @@ pub enum Error { VerificationFailed, /// The operation does not support the supplied input. /// - /// Currently used by `batch_validate` when given a multi-context proof. - /// Only single-context proofs are batchable today. + /// Currently used by the batch validation methods when given a + /// multi-context proof. Only single-context proofs are batchable today. Unsupported, } @@ -378,6 +378,14 @@ pub trait GenerateVerifiable { /// proofs in a batch may come from different rings, even rings of different /// sizes. /// + /// This is all-or-nothing: a single invalid item rejects the whole batch with + /// an error that does not identify the offending item, and the worst-case cost + /// stays that of one batched check. Appropriate when the batch has a single + /// responsible producer (e.g. the proofs of an already-authored block). When + /// batching proofs from untrusted submitters, use + /// [`Self::batch_validate_per_item`] instead, so one junk submission cannot + /// suppress the honest proofs grouped with it. + /// /// Currently only supports single-context proofs. Multi-context proofs should be /// validated individually via [`Self::validate_multi_context`]. fn batch_validate(proofs: &[BatchProofItemFor]) -> Result, Error> { @@ -395,6 +403,33 @@ pub trait GenerateVerifiable { .collect() } + /// Like [`Self::batch_validate`], but returns one outcome per item, in input + /// order, attributing each failure to the item responsible: an invalid, + /// malformed, or unsupported item never affects the outcome of the others. + /// + /// Implementations may verify the whole batch in a single combined check and + /// pay an extra attribution cost only when that check fails, so a batch with + /// invalid items can cost more than [`Self::batch_validate`] on the same + /// input. Prefer `batch_validate` when an all-or-nothing verdict suffices and + /// the worst-case cost matters. + /// + /// Currently only supports single-context proofs; multi-context proofs are + /// reported as [`Error::Unsupported`]. + fn batch_validate_per_item(proofs: &[BatchProofItemFor]) -> Vec> { + proofs + .iter() + .map(|item| { + Self::validate( + item.config, + &item.proof, + &item.members, + &item.context, + &item.message, + ) + }) + .collect() + } + /// Check whether `member` is a valid encoded public key for this scheme. fn is_member_valid(member: &Self::Member) -> bool; diff --git a/src/ring/bandersnatch.rs b/src/ring/bandersnatch.rs index 92352ec..35b0233 100644 --- a/src/ring/bandersnatch.rs +++ b/src/ring/bandersnatch.rs @@ -729,10 +729,15 @@ mod builder_tests { println!("* Batch validate {} proofs: {} ms", num_proofs, batch_ms); // Verify aliases match - assert_eq!(aliases.len(), num_proofs); - for (alias, expected) in aliases.iter().zip(expected_aliases.iter()) { - assert_eq!(alias, expected); - } + assert_eq!(aliases, expected_aliases); + + // The per-item variant agrees on an all-valid batch. + let results = BandersnatchVrfVerifiable::batch_validate_per_item(&batch_items); + assert_eq!(results, aliases.into_iter().map(Ok).collect::>()); + + // An empty batch is vacuously valid for both variants. + assert_eq!(BandersnatchVrfVerifiable::batch_validate(&[]), Ok(vec![])); + assert!(BandersnatchVrfVerifiable::batch_validate_per_item(&[]).is_empty()); println!( "* Speedup: {:.2}x", @@ -740,7 +745,11 @@ mod builder_tests { ); }); - test_for_all_domains!(batch_validate_rejects_invalid_proof, |domain_size| { + // A junk item is free to produce, so the two batch semantics must both hold: + // `batch_validate` rejects the whole batch (all-or-nothing), while + // `batch_validate_per_item` attributes the failure to the offending item + // without affecting the others. + test_for_all_domains!(batch_validate_invalid_items, |domain_size| { use crate::BatchProofItem; let context = b"Context"; @@ -767,6 +776,7 @@ mod builder_tests { // Create 3 valid proofs let mut batch_items = Vec::new(); + let mut expected = Vec::new(); for i in 0..3 { let member = member_keys[i]; let message = format!("Message from member {}", i); @@ -776,13 +786,14 @@ mod builder_tests { member_keys.clone().into_iter(), ) .unwrap(); - let (proof, _alias) = BandersnatchVrfVerifiable::create( + let (proof, alias) = BandersnatchVrfVerifiable::create( commitment, &secrets[i], context, message.as_bytes(), ) .unwrap(); + expected.push(alias); batch_items.push(BatchProofItem { proof, config: domain_size, @@ -791,21 +802,58 @@ mod builder_tests { message: message.into_bytes(), }); } + let all_valid: Vec<_> = expected.iter().copied().map(Ok).collect(); // Sanity: batch with all valid proofs should pass - assert!(BandersnatchVrfVerifiable::batch_validate(&batch_items,).is_ok()); + assert_eq!( + BandersnatchVrfVerifiable::batch_validate(&batch_items), + Ok(expected.clone()) + ); // Corrupt the proof of the second item by flipping a byte in the proof data let mut corrupted_items = batch_items.clone(); corrupted_items[1].proof[10] ^= 0xFF; - assert!(BandersnatchVrfVerifiable::batch_validate(&corrupted_items,).is_err()); + assert!(BandersnatchVrfVerifiable::batch_validate(&corrupted_items).is_err()); + let results = BandersnatchVrfVerifiable::batch_validate_per_item(&corrupted_items); + assert!(results[1].is_err()); + assert_eq!(results[0], all_valid[0]); + assert_eq!(results[2], all_valid[2]); - // Also test with a wrong message on a valid proof + // A wrong message on a valid proof decodes fine and only fails the combined + // check, so this exercises the bisecting attribution path. let mut wrong_message_items = batch_items.clone(); wrong_message_items[2].message = b"tampered message".to_vec(); - assert!(BandersnatchVrfVerifiable::batch_validate(&wrong_message_items,).is_err()); + assert_eq!( + BandersnatchVrfVerifiable::batch_validate(&wrong_message_items), + Err(crate::Error::VerificationFailed) + ); + let results = BandersnatchVrfVerifiable::batch_validate_per_item(&wrong_message_items); + assert_eq!( + results, + vec![ + all_valid[0], + all_valid[1], + Err(crate::Error::VerificationFailed) + ] + ); + + // Multiple bad items: bisection must descend into both halves. + let mut two_bad_items = batch_items.clone(); + two_bad_items[0].message = b"tampered message".to_vec(); + two_bad_items[2].message = b"tampered message".to_vec(); + + assert!(BandersnatchVrfVerifiable::batch_validate(&two_bad_items).is_err()); + let results = BandersnatchVrfVerifiable::batch_validate_per_item(&two_bad_items); + assert_eq!( + results, + vec![ + Err(crate::Error::VerificationFailed), + all_valid[1], + Err(crate::Error::VerificationFailed) + ] + ); // Test with a proof from a non-member key. // Generate a new key that is NOT in the ring, create a proof using a ring @@ -834,7 +882,10 @@ mod builder_tests { message: b"outsider message".to_vec(), }); - assert!(BandersnatchVrfVerifiable::batch_validate(&outsider_items,).is_err()); + assert!(BandersnatchVrfVerifiable::batch_validate(&outsider_items).is_err()); + let results = BandersnatchVrfVerifiable::batch_validate_per_item(&outsider_items); + assert_eq!(results[..3], all_valid); + assert_eq!(results[3], Err(crate::Error::VerificationFailed)); // Test with a proof created under a different context let wrong_context = b"WrongContext"; @@ -858,7 +909,10 @@ mod builder_tests { message: b"some message".to_vec(), }); - assert!(BandersnatchVrfVerifiable::batch_validate(&wrong_ctx_items,).is_err()); + assert!(BandersnatchVrfVerifiable::batch_validate(&wrong_ctx_items).is_err()); + let results = BandersnatchVrfVerifiable::batch_validate_per_item(&wrong_ctx_items); + assert_eq!(results[..3], all_valid); + assert_eq!(results[3], Err(crate::Error::VerificationFailed)); }); // Each `BatchProofItem` carries its own ring, so a batch may mix proofs from @@ -923,11 +977,18 @@ mod builder_tests { assert_eq!(aliases, vec![alias_a, alias_b]); // Swapping the per-item rings verifies each proof against the wrong ring, - // which must fail. + // which must fail; the per-item variant attributes both items. let mut swapped = batch.clone(); swapped[0].members = members_b; swapped[1].members = members_a; assert!(BandersnatchVrfVerifiable::batch_validate(&swapped).is_err()); + assert_eq!( + BandersnatchVrfVerifiable::batch_validate_per_item(&swapped), + vec![ + Err(crate::Error::VerificationFailed), + Err(crate::Error::VerificationFailed) + ] + ); }); // A single batch may mix proofs from rings of *different* domain sizes: the @@ -974,13 +1035,96 @@ mod builder_tests { // Sanity: the batch really spans all three domain sizes. assert_eq!(batch.len(), 3); - let aliases = BandersnatchVrfVerifiable::batch_validate(&batch).unwrap(); - assert_eq!(aliases, expected); + assert_eq!( + BandersnatchVrfVerifiable::batch_validate(&batch), + Ok(expected.clone()) + ); - // Verifying one item under the wrong domain size must fail the whole batch. + // Verifying one item under the wrong domain size fails the whole batch; + // the per-item variant pins the failure on that item alone. let mut wrong = batch.clone(); wrong[0].config = wrong[1].config; assert!(BandersnatchVrfVerifiable::batch_validate(&wrong).is_err()); + let all_valid: Vec<_> = expected.iter().copied().map(Ok).collect(); + let results = BandersnatchVrfVerifiable::batch_validate_per_item(&wrong); + assert!(results[0].is_err()); + assert_eq!(results[1..], all_valid[1..]); + } + + // Items rejected before entering the combined check (malformed bytes, + // unsupported multi-context proofs) short-circuit `batch_validate`, while + // `batch_validate_per_item` reports them at their own index with their own + // error, leaving the other items' outcomes untouched. + #[test] + fn batch_validate_per_item_reports_errors() { + use crate::BatchProofItem; + use bounded_collections::BoundedVec; + + let domain_size = RingDomainSize::Domain11; + let context = b"Context"; + let message = b"msg"; + let _ = bandersnatch_ring_setup(domain_size); + + let secrets: Vec<_> = (0..3) + .map(|i| BandersnatchVrfVerifiable::new_secret([i as u8; 32])) + .collect(); + let member_keys: Vec<_> = secrets + .iter() + .map(BandersnatchVrfVerifiable::member_from_secret) + .collect(); + let members = build_members(member_keys.iter().copied(), domain_size); + + let commitment = BandersnatchVrfVerifiable::open( + domain_size, + &member_keys[0], + member_keys.iter().copied(), + ) + .unwrap(); + let (valid_proof, alias) = + BandersnatchVrfVerifiable::create(commitment.clone(), &secrets[0], context, message) + .unwrap(); + + // Valid on its own, but not batchable. + let (multi_ctx_proof, _) = BandersnatchVrfVerifiable::create_multi_context( + commitment, + &secrets[0], + &[context.as_slice(), b"other context".as_slice()], + message, + ) + .unwrap(); + + // Random bytes of the right length, the cheapest junk a submitter can produce. + let garbage_proof = BoundedVec::try_from(vec![0xAA; valid_proof.len()]).unwrap(); + + let make_item = |proof| BatchProofItem { + proof, + config: domain_size, + members: members.clone(), + context: context.to_vec(), + message: message.to_vec(), + }; + let batch = vec![ + make_item(garbage_proof), + make_item(valid_proof), + make_item(multi_ctx_proof), + ]; + + assert_eq!( + BandersnatchVrfVerifiable::batch_validate(&batch), + Err(crate::Error::DecodeError), + ); + assert_eq!( + BandersnatchVrfVerifiable::batch_validate(&batch[1..]), + Err(crate::Error::Unsupported), + ); + assert_eq!( + BandersnatchVrfVerifiable::batch_validate_per_item(&batch), + vec![ + Err(crate::Error::DecodeError), + Ok(alias), + Err(crate::Error::Unsupported), + ], + ); } test_for_all_domains!(open_validate_single_vs_multiple_keys, |domain_size| { @@ -1235,7 +1379,10 @@ mod builder_tests { context: context.to_vec(), message: message.to_vec(), }]; - assert!(BandersnatchVrfVerifiable::batch_validate(&batch_items).is_err()); + assert_eq!( + BandersnatchVrfVerifiable::batch_validate(&batch_items), + Err(crate::Error::DecodeError), + ); } // The neutral element passes the subgroup check, so `is_member_valid` @@ -1377,6 +1524,10 @@ mod builder_tests { BandersnatchVrfVerifiable::batch_validate(&batch), Err(crate::Error::VerificationFailed), ); + assert_eq!( + BandersnatchVrfVerifiable::batch_validate_per_item(&batch), + vec![Err(crate::Error::VerificationFailed)], + ); } fn chunk_lookup( diff --git a/src/ring/mod.rs b/src/ring/mod.rs index c5a3880..cb96d87 100644 --- a/src/ring/mod.rs +++ b/src/ring/mod.rs @@ -1,4 +1,4 @@ -use alloc::{borrow::Cow, vec}; +use alloc::{borrow::Cow, rc::Rc, vec}; use core::{marker::PhantomData, ops::Range}; pub use ark_vrf; @@ -627,6 +627,138 @@ fn make_ring_verifier( S::VerifierCache::ring_context(config).ring_verifier(verifier_key) } +/// Per-item state retained during batch accumulation, so that a failed +/// combined check can be attributed by re-verifying sub-batches without +/// re-parsing the proofs. `verifier` is a handle to the ring verifier of the +/// item's run, shared with the other items of the same ring; `index` is the +/// item's position in the caller's slice (and in the results). +struct BatchEntry<'a, S: RingSuiteExt> { + index: usize, + verifier: Rc>, + io: VrfIo, + message: &'a [u8], + proof: ark_vrf::ring::Proof, +} + +fn verify_sub_batch(entries: &[BatchEntry<'_, S>]) -> bool { + let mut batch = ark_vrf::ring::BatchVerifier::::new(&entries[0].verifier); + for entry in entries { + batch + .push(&entry.verifier, entry.io, entry.message, &entry.proof) + .expect("push succeeded for the same arguments in the full batch"); + } + batch.verify().is_ok() +} + +/// Attribute a failed combined check to the responsible items. +/// +/// The combined check is a single randomized linear combination and cannot say +/// which item broke it, so `entries` (known to fail as a batch) is bisected, +/// descending only into failing halves: `k` bad items among `n` cost +/// `O(k*log(n/k))` sub-batch checks instead of `n` individual verifications. +fn attribute_batch_failure( + entries: &[BatchEntry<'_, S>], + results: &mut [Result], +) { + // Work list of sub-batches known to fail as a batch. + let mut failing = vec![entries]; + while let Some(entries) = failing.pop() { + if let [entry] = entries { + results[entry.index] = Err(Error::VerificationFailed); + continue; + } + let (left, right) = entries.split_at(entries.len() / 2); + let left_ok = verify_sub_batch(left); + if !left_ok { + failing.push(left); + } + // A batch of valid proofs always passes (completeness is deterministic), + // so a failing set with a passing left half pins the failure on the + // right half: checking it would be redundant. + if left_ok || !verify_sub_batch(right) { + failing.push(right); + } + } +} + +// Multi-ring batch accumulation shared by `batch_validate` and +// `batch_validate_per_item`: each proof enters a single combined check via a +// `RingVerifier` built from its own ring, the KZG SRS being the only shared +// requirement. Returns the combined verifier, `None` when nothing entered it. +// +// `sink` gets one outcome per item, in input order: `Ok` carries the alias +// (provisional until the combined check passes) and the item's `BatchEntry`; +// `Err` reports an item that never entered the batch. `exit_on_first_error` +// stops accumulation at the first `Err`. +fn accumulate_batch<'a, S: RingSuiteExt>( + proofs: &'a [BatchProofItemFor>], + exit_on_first_error: bool, + mut sink: impl FnMut(Result<(Alias, BatchEntry<'a, S>), Error>), +) -> Option> { + let mut verifier: Option>> = None; + let mut last_ring: Option<(RingDomainSize, &MembersCommitment)> = None; + // Seeded lazily from the first item's verifier; the KZG verifier key it + // extracts is shared by every ring regardless of domain size. + let mut batch_verifier: Option> = None; + let mut process_item = |index, item: &'a BatchProofItemFor>| { + let BatchProofItem { + proof, + config, + members, + context, + message, + } = item; + + let signature = RingVrfSignature::::deserialize_canonical(proof.as_slice())?; + + let outputs = signature.outputs.0.as_slice(); + if outputs.len() != 1 { + // The batch verifier only supports single-context proofs. + return Err(Error::Unsupported); + } + let output = outputs[0]; + + if last_ring != Some((*config, members)) { + verifier = Some(Rc::new(make_ring_verifier::(*config, members))); + last_ring = Some((*config, members)); + } + let verifier = verifier + .as_ref() + .expect("set on the first item and on each ring change"); + + let input_msg = [S::VRF_INPUT_DOMAIN, context.as_slice()].concat(); + let input = ark_vrf::Input::::new(&input_msg[..]).expect("H2C can't fail here"); + let io = VrfIo { input, output }; + + let batch_verifier = + batch_verifier.get_or_insert_with(|| ark_vrf::ring::BatchVerifier::::new(verifier)); + batch_verifier + .push(verifier, io, message, &signature.proof) + .map_err(|_| Error::VerificationFailed)?; + + Ok(( + make_alias(&output), + BatchEntry { + index, + verifier: verifier.clone(), + io, + message, + proof: signature.proof, + }, + )) + }; + + for (index, item) in proofs.iter().enumerate() { + let outcome = process_item(index, item); + let failed = outcome.is_err(); + sink(outcome); + if failed && exit_on_first_error { + break; + } + } + batch_verifier +} + /// Generic ring VRF implementation parameterized over the ring suite. /// /// The curve params provider is obtained from `S::CurveParams`. @@ -735,75 +867,51 @@ impl GenerateVerifiable for RingVrfVerifiable { Ok(aliases) } - // Multi-ring batch verification. Each item carries its own `config` and - // `members`, so the proofs may come from different rings, even rings of - // different sizes. The only shared requirement is the KZG SRS, which is the - // same across all domain sizes for a suite. `ark_vrf`'s `BatchVerifier` - // accumulates the proofs into a single pairing check seeded once with that - // shared KZG verifier key; each proof is pushed with a per-item `RingVerifier` - // built from the item's own domain size and ring commitment, so the PIOP - // domain baked into each accumulated item is the one its proof was made under. + // All-or-nothing: any per-item error or a failed combined check rejects the + // whole batch, so accumulation exits at the first error; the aliases are + // collected directly and nothing is retained for attribution, whose cost is + // never paid. See `accumulate_batch`. fn batch_validate(proofs: &[BatchProofItemFor]) -> Result, Error> { - struct Cache<'a, S: RingSuiteExt> { - config: RingDomainSize, - members: &'a MembersCommitment, - verifier: ark_vrf::ring::RingVerifier, - } - // Reused across consecutive items that share a ring; see `CachedRingVerifier`. - let mut cache: Option> = None; - let mut aliases = Vec::with_capacity(proofs.len()); - // Seeded lazily from the first item's verifier; the KZG verifier key it - // extracts is shared by every ring regardless of domain size. - let mut batch_verifier: Option> = None; - for BatchProofItem { - proof, - config, - members, - context, - message, - } in proofs - { - if !matches!(&cache, Some(c) if c.config == *config && c.members == members) { - cache = Some(Cache { - config: *config, - members, - verifier: make_ring_verifier::(*config, members), - }); - } - let verifier = &cache - .as_ref() - .expect("populated on the first item and on each ring change") - .verifier; - - let input_msg = [S::VRF_INPUT_DOMAIN, context.as_slice()].concat(); - let input = ark_vrf::Input::::new(&input_msg[..]).expect("H2C can't fail here"); - let signature = RingVrfSignature::::deserialize_canonical(proof.as_slice())?; - - let outputs = signature.outputs.0.as_slice(); - if outputs.len() != 1 { - // The current batch verifier only supports single-context proofs. - return Err(Error::Unsupported); - } - let output = outputs[0]; - - aliases.push(make_alias(&output)); - - let io = VrfIo { input, output }; - let batch_verifier = batch_verifier - .get_or_insert_with(|| ark_vrf::ring::BatchVerifier::::new(verifier)); + let mut first_error = None; + let batch_verifier = accumulate_batch::(proofs, true, |outcome| match outcome { + Ok((alias, _)) => aliases.push(alias), + Err(error) => first_error = Some(error), + }); + if let Some(error) = first_error { + return Err(error); + } + if let Some(batch_verifier) = batch_verifier { batch_verifier - .push(verifier, [io], message, &signature.proof) + .verify() .map_err(|_| Error::VerificationFailed)?; } - match batch_verifier { - Some(batch_verifier) => batch_verifier + Ok(aliases) + } + + // Per-item: entries are captured during accumulation so that, when the + // combined check fails, `attribute_batch_failure` can bisect the batch + // without re-parsing the proofs; the all-valid fast path stays a single + // combined check. See `accumulate_batch`. + fn batch_validate_per_item(proofs: &[BatchProofItemFor]) -> Vec> { + let mut results = Vec::with_capacity(proofs.len()); + let mut entries = Vec::with_capacity(proofs.len()); + let batch_verifier = accumulate_batch::(proofs, false, |outcome| match outcome { + Ok((alias, entry)) => { + results.push(Ok(alias)); + entries.push(entry); + } + Err(error) => results.push(Err(error)), + }); + if !entries.is_empty() + && batch_verifier + .expect("initialized when the first entry was pushed") .verify() - .map(|_| aliases) - .map_err(|_| Error::VerificationFailed), - // Empty batch: vacuously valid, no aliases. - None => Ok(aliases), + .is_err() + { + attribute_batch_failure(&entries, &mut results); } + results } #[cfg(feature = "prover")]