Skip to content

Commit

Permalink
Integrate JoinSplit verifier (#3180)
Browse files Browse the repository at this point in the history 
* Integrate JoinSplit verifier with transaction verifier

* Add test with malformed Groth16 Output proof

* Use TryFrom instead of From in ItemWrapper to correctly propagate malformed proof errors

* Simplify by removing ItemWrapper and directly TryFrom into Item

* Fix existing tests to work with JoinSplit validation

* Apply suggestions from code review

Co-authored-by: Deirdre Connolly <[email protected]>

Co-authored-by: Deirdre Connolly <[email protected]>
Co-authored-by: Pili Guerra <[email protected]>
  • Loading branch information
@conradoplg @dconnolly @mpguerra
3 people authored Dec 13, 2021
1 parent 7bc2f0a commit 6ec42c6
Show file tree
Hide file tree
Showing 7 changed files with 272 additions and 149 deletions.
8 changes: 4 additions & 4 deletions zebra-chain/src/transaction.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,7 @@ impl Transaction {
}

/// Returns the `vpub_old` fields from `JoinSplit`s in this transaction,
/// regardless of version.
/// regardless of version, in the order they appear in the transaction.
///
/// These values are added to the sprout chain value pool,
/// and removed from the value pool of this transaction.
Expand Down Expand Up @@ -1067,7 +1067,7 @@ impl Transaction {
}

/// Modify the `vpub_old` fields from `JoinSplit`s in this transaction,
/// regardless of version.
/// regardless of version, in the order they appear in the transaction.
///
/// See `output_values_to_sprout` for details.
#[cfg(any(test, feature = "proptest-impl"))]
Expand Down Expand Up @@ -1116,7 +1116,7 @@ impl Transaction {
}

/// Returns the `vpub_new` fields from `JoinSplit`s in this transaction,
/// regardless of version.
/// regardless of version, in the order they appear in the transaction.
///
/// These values are removed from the value pool of this transaction.
/// and added to the sprout chain value pool.
Expand Down Expand Up @@ -1163,7 +1163,7 @@ impl Transaction {
}

/// Modify the `vpub_new` fields from `JoinSplit`s in this transaction,
/// regardless of version.
/// regardless of version, in the order they appear in the transaction.
///
/// See `input_values_from_sprout` for details.
#[cfg(any(test, feature = "proptest-impl"))]
Expand Down
13 changes: 10 additions & 3 deletions zebra-consensus/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,14 @@ pub enum TransactionError {
#[error("spend description cv and rk MUST NOT be of small order")]
SmallOrder,

// XXX change this when we align groth16 verifier errors with bellman
// and add a from annotation when the error type is more precise
// XXX: the underlying error is bellman::VerificationError, but it does not implement
// Arbitrary as required here.
#[error("spend proof MUST be valid given a primary input formed from the other fields except spendAuthSig")]
Groth16,
Groth16(String),

// XXX: the underlying error is io::Error, but it does not implement Clone as required here.
#[error("Groth16 proof is malformed")]
MalformedGroth16(String),

#[error(
"Sprout joinSplitSig MUST represent a valid signature under joinSplitPubKey of dataToBeSigned"
Expand All @@ -153,6 +157,9 @@ pub enum TransactionError {
#[error("Downcast from BoxError to redjubjub::Error failed")]
InternalDowncastError(String),

#[error("either vpub_old or vpub_new must be zero")]
BothVPubsNonZero,

#[error("adding to the sprout pool is disabled after Canopy")]
DisabledAddToSproutPool,

Expand Down
51 changes: 29 additions & 22 deletions zebra-consensus/src/primitives/groth16.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
//! Async Groth16 batch verifier service
use std::{
convert::TryInto,
convert::{TryFrom, TryInto},
error::Error,
fmt,
future::Future,
mem,
Expand All @@ -22,7 +23,7 @@ use tokio::sync::broadcast::{channel, error::RecvError, Sender};
use tower::{util::ServiceFn, Service};

use tower_batch::{Batch, BatchControl};
use tower_fallback::Fallback;
use tower_fallback::{BoxedError, Fallback};

use zebra_chain::{
primitives::{
Expand All @@ -41,6 +42,8 @@ mod vectors;

pub use params::{Groth16Parameters, GROTH16_PARAMETERS};

use crate::error::TransactionError;

/// Global batch verification context for Groth16 proofs of Spend statements.
///
/// This service transparently batches contemporaneous proof verifications,
Expand Down Expand Up @@ -115,7 +118,7 @@ pub static OUTPUT_VERIFIER: Lazy<
/// Note that making a `Service` call requires mutable access to the service, so
/// you should call `.clone()` on the global handle to create a local, mutable
/// handle.
pub static JOINSPLIT_VERIFIER: Lazy<ServiceFn<fn(Item) -> Ready<Result<(), VerificationError>>>> =
pub static JOINSPLIT_VERIFIER: Lazy<ServiceFn<fn(Item) -> Ready<Result<(), BoxedError>>>> =
Lazy::new(|| {
// We need a Service to use. The obvious way to do this would
// be to write a closure that returns an async block. But because we
Expand All @@ -126,19 +129,19 @@ pub static JOINSPLIT_VERIFIER: Lazy<ServiceFn<fn(Item) -> Ready<Result<(), Verif
// function (which is possible because it doesn't capture any state).
tower::service_fn(
(|item: Item| {
// Workaround bug in `bellman::VerificationError` fmt::Display
// implementation https://github.com/zkcrypto/bellman/pull/77
#[allow(deprecated)]
ready(
item.verify_single(&GROTH16_PARAMETERS.sprout.joinsplit_prepared_verifying_key),
item.verify_single(&GROTH16_PARAMETERS.sprout.joinsplit_prepared_verifying_key)
// When that is fixed, change to `e.to_string()`
.map_err(|e| TransactionError::Groth16(e.description().to_string()))
.map_err(tower_fallback::BoxedError::from),
)
}) as fn(_) -> _,
)
});

/// A Groth16 verification item, used as the request type of the service.
pub type Item = batch::Item<Bls12>;

/// A wrapper to workaround the missing `ServiceExt::map_err` method.
pub struct ItemWrapper(Item);

/// A Groth16 Description (JoinSplit, Spend, or Output) with a Groth16 proof
/// and its inputs encoded as scalars.
pub trait Description {
Expand Down Expand Up @@ -296,22 +299,26 @@ impl Description for (&JoinSplit<Groth16Proof>, &ed25519::VerificationKeyBytes)
}
}

impl<T> From<&T> for ItemWrapper
/// A Groth16 verification item, used as the request type of the service.
pub type Item = batch::Item<Bls12>;

/// A wrapper to allow a TryFrom blanket implementation of the [`Description`]
/// trait for the [`Item`] struct.
/// See https://github.com/rust-lang/rust/issues/50133 for more details.
pub struct DescriptionWrapper<T>(pub T);

impl<T> TryFrom<DescriptionWrapper<&T>> for Item
where
T: Description,
{
/// Convert a [`Description`] into an [`ItemWrapper`].
fn from(input: &T) -> Self {
Self(Item::from((
bellman::groth16::Proof::read(&input.proof().0[..]).unwrap(),
input.primary_inputs(),
)))
}
}
type Error = TransactionError;

impl From<ItemWrapper> for Item {
fn from(item_wrapper: ItemWrapper) -> Self {
item_wrapper.0
fn try_from(input: DescriptionWrapper<&T>) -> Result<Self, Self::Error> {
Ok(Item::from((
bellman::groth16::Proof::read(&input.0.proof().0[..])
.map_err(|e| TransactionError::MalformedGroth16(e.to_string()))?,
input.0.primary_inputs(),
)))
}
}

Expand Down
81 changes: 54 additions & 27 deletions zebra-consensus/src/primitives/groth16/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use zebra_chain::{
transaction::Transaction,
};

use crate::primitives::groth16::{self, *};
use crate::primitives::groth16::*;

async fn verify_groth16_spends_and_outputs<V>(
spend_verifier: &mut V,
Expand All @@ -37,21 +37,23 @@ where
for spend in spends {
tracing::trace!(?spend);

let spend_rsp = spend_verifier
.ready()
.await?
.call(groth16::ItemWrapper::from(&spend).into());
let spend_rsp = spend_verifier.ready().await?.call(
DescriptionWrapper(&spend)
.try_into()
.map_err(tower_fallback::BoxedError::from)?,
);

async_checks.push(spend_rsp);
}

for output in outputs {
tracing::trace!(?output);

let output_rsp = output_verifier
.ready()
.await?
.call(groth16::ItemWrapper::from(output).into());
let output_rsp = output_verifier.ready().await?.call(
DescriptionWrapper(output)
.try_into()
.map_err(tower_fallback::BoxedError::from)?,
);

async_checks.push(output_rsp);
}
Expand Down Expand Up @@ -110,9 +112,21 @@ async fn verify_sapling_groth16() {
.unwrap()
}

#[derive(Clone, Copy)]
enum Groth16OutputModification {
ZeroCMU,
ZeroProof,
}

static GROTH16_OUTPUT_MODIFICATIONS: [Groth16OutputModification; 2] = [
Groth16OutputModification::ZeroCMU,
Groth16OutputModification::ZeroProof,
];

async fn verify_invalid_groth16_output_description<V>(
output_verifier: &mut V,
transactions: Vec<std::sync::Arc<Transaction>>,
modification: Groth16OutputModification,
) -> Result<(), V::Error>
where
V: tower::Service<Item, Response = ()>,
Expand All @@ -132,14 +146,18 @@ where
// This changes the primary inputs to the proof
// verification, causing it to fail for this proof.
let mut modified_output = output.clone();
modified_output.cm_u = jubjub::Fq::zero();
match modification {
Groth16OutputModification::ZeroCMU => modified_output.cm_u = jubjub::Fq::zero(),
Groth16OutputModification::ZeroProof => modified_output.zkproof.0 = [0; 192],
}

tracing::trace!(?modified_output);

let output_rsp = output_verifier
.ready()
.await?
.call(groth16::ItemWrapper::from(&modified_output).into());
let output_rsp = output_verifier.ready().await?.call(
DescriptionWrapper(&modified_output)
.try_into()
.map_err(tower_fallback::BoxedError::from)?,
);

async_checks.push(output_rsp);
}
Expand Down Expand Up @@ -174,9 +192,15 @@ async fn correctly_err_on_invalid_output_proof() {
.zcash_deserialize_into::<Block>()
.expect("a valid block");

verify_invalid_groth16_output_description(&mut output_verifier, block.transactions)
for modification in GROTH16_OUTPUT_MODIFICATIONS {
verify_invalid_groth16_output_description(
&mut output_verifier,
block.transactions.clone(),
modification,
)
.await
.expect_err("unexpected success checking invalid groth16 inputs");
}
}

async fn verify_groth16_joinsplits<V>(
Expand Down Expand Up @@ -204,10 +228,11 @@ where
let pub_key = tx
.sprout_joinsplit_pub_key()
.expect("pub key must exist since there are joinsplits");
let joinsplit_rsp = verifier
.ready()
.await?
.call(groth16::ItemWrapper::from(&(joinsplit, &pub_key)).into());
let joinsplit_rsp = verifier.ready().await?.call(
DescriptionWrapper(&(joinsplit, &pub_key))
.try_into()
.map_err(tower_fallback::BoxedError::from)?,
);

async_checks.push(joinsplit_rsp);
}
Expand Down Expand Up @@ -268,10 +293,11 @@ where

tracing::trace!(?joinsplit);

let joinsplit_rsp = verifier
.ready()
.await?
.call(groth16::ItemWrapper::from(&(joinsplit, pub_key)).into());
let joinsplit_rsp = verifier.ready().await?.call(
DescriptionWrapper(&(joinsplit, pub_key))
.try_into()
.map_err(tower_fallback::BoxedError::from)?,
);

async_checks.push(joinsplit_rsp);

Expand Down Expand Up @@ -388,10 +414,11 @@ where
// Use an arbitrary public key which is not the correct one,
// which will make the verification fail.
let modified_pub_key = [0x42; 32].into();
let joinsplit_rsp = verifier
.ready()
.await?
.call(groth16::ItemWrapper::from(&(joinsplit, &modified_pub_key)).into());
let joinsplit_rsp = verifier.ready().await?.call(
DescriptionWrapper(&(joinsplit, &modified_pub_key))
.try_into()
.map_err(tower_fallback::BoxedError::from)?,
);

async_checks.push(joinsplit_rsp);
}
Expand Down
Loading

0 comments on commit 6ec42c6

Please sign in to comment.