First iteration of circuits naming convention

In this commit I remove all `*Gadget` types and instead implement the naming convention defined here https://github.com/0xPARC/pod2/issues/181#issuecomment-3051954321

The biggest changes can be summarized by:
- a) Removal of `*Gadget` types and their `eval_*` methods in favour of `verb_object_circuit` functions.
- b) The above functions don't create targets that need to be witness-assigned later.  Instead they receive those as arguments.  This clearly shows what's the circuit input and output.

I'm specially happy about the changes from b), I think they make the flow of data in the circuit more clear.

Missing things that I did not address in this PR
- The RecursiveCircuit still uses some old naming conventions like `build`.
- We have some `*Target` types that have methods that define constraints.  I think we can keep those as they are convenient and I don't see them as strongly breaking the new convention: I see them as the object-oriented way to apply the convention.  In those cases the `object` can be omitted from the method when it's implied by the type name, and the `_circuit` suffix doesn't appear because it's implied by the fact that the type is a `*Target`.  Examples are: `SignatureTarget::verify -> BoolTarget`, `StatementTarget::has_native_type -> BoolTarget` or `OperationTypeTarget::as_custom -> (BoolTarget, HashOutTarget, Target)`.

book/src/anchoredkeys.md

@@ -6,6 +6,8 @@ type AnchoredKey = (Origin, Key)
 type Key = String
 ```
 
+FIXME: This description is incorrect.  We don't have *gadget ID*.  And we don't think of *origin* as a triple, we just see it as a single value that encodes the *pod ID* or SELF.
+
 An *origin* is a triple consisting of a numeric identifier called the *origin ID*, a string called the *origin name* (omitted in the backend) and another numeric identifier called the *gadget ID*, which identifies the means by which the value corresponding to a given key is produced.
 
 The origin ID is defined to be 0 for 'no origin' and 1 for 'self origin', otherwise it is the content ID[^content-id] of the POD to which it refers. The origin name is not cryptographically significant and is merely a convenience for the frontend.
@@ -19,4 +21,4 @@ The gadget ID takes on the values in the following table:
 | 2         | `MainPOD` gadget: The key-value pair was produced in the construction of a `MainPOD`.     |
 
 For example, a gadget ID of 1 implies that the key-value pair in question was produced in the process of constructing a `SignedPOD`.
-[^content-id]: <font color="red">TODO</font> Refer to this when it is documented.
+[^content-id]: <font color="red">TODO</font> Refer to this when it is documented.

src/backends/plonky2/circuits/common.rs

@@ -579,10 +579,22 @@ pub struct CustomPredicateVerifyEntryTarget {
     pub custom_predicate_table_index: Target,
     pub custom_predicate: CustomPredicateEntryTarget,
     pub args: Vec<ValueTarget>,
-    pub query: CustomPredicateVerifyQueryTarget,
+    pub op_args: Vec<StatementTarget>,
 }
 
 impl CustomPredicateVerifyEntryTarget {
+    pub fn new_virtual(params: &Params, builder: &mut CircuitBuilder) -> Self {
+        CustomPredicateVerifyEntryTarget {
+            custom_predicate_table_index: builder.add_virtual_target(),
+            custom_predicate: builder.add_virtual_custom_predicate_entry(params),
+            args: (0..params.max_custom_predicate_wildcards)
+                .map(|_| builder.add_virtual_value())
+                .collect(),
+            op_args: (0..params.max_operation_args)
+                .map(|_| builder.add_virtual_statement(params))
+                .collect(),
+        }
+    }
     pub fn set_targets(
         &self,
         pw: &mut PartialWitness<F>,
@@ -606,7 +618,7 @@ impl CustomPredicateVerifyEntryTarget {
             arg_target.set_targets(pw, &Value::from(arg.raw()))?;
         }
         let pad_op_arg = Statement(Predicate::Native(NativePredicate::None), vec![]);
-        for (op_arg_target, op_arg) in self.query.op_args.iter().zip_eq(
+        for (op_arg_target, op_arg) in self.op_args.iter().zip_eq(
             cpv.op_args
                 .iter()
                 .chain(iter::repeat(&pad_op_arg))

src/backends/plonky2/circuits/signedpod.rs

@@ -16,9 +16,10 @@ use crate::{
         error::Result,
         primitives::{
             merkletree::{
-                MerkleClaimAndProof, MerkleProofExistenceGadget, MerkleProofExistenceTarget,
+                verify_merkle_proof_existence_circuit, MerkleClaimAndProof,
+                MerkleProofExistenceTarget,
             },
-            signature::{SignatureVerifyGadget, SignatureVerifyTarget},
+            signature::{verify_signature_circuit, SignatureVerifyTarget},
         },
         signedpod::SignedPod,
     },
@@ -29,53 +30,45 @@ use crate::{
     },
 };
 
-pub struct SignedPodVerifyGadget {
+pub fn verify_signed_pod_circuit(
-    pub params: Params,
+    builder: &mut CircuitBuilder<F, D>,
-}
+    signed_pod: &SignedPodVerifyTarget,
-
+) -> Result<()> {
-impl SignedPodVerifyGadget {
+    let params = &signed_pod.params;
-    pub fn eval(&self, builder: &mut CircuitBuilder<F, D>) -> Result<SignedPodVerifyTarget> {
+    let measure = measure_gates_begin!(builder, "SignedPodVerify");
-        let measure = measure_gates_begin!(builder, "SignedPodVerify");
+    // 1. Verify id
-        // 1. Verify id
+    assert_eq!(params.max_signed_pod_values, signed_pod.mt_proofs.len());
-        let id = builder.add_virtual_hash();
+    for mt_proof in &signed_pod.mt_proofs {
-        let mut mt_proofs = Vec::new();
+        verify_merkle_proof_existence_circuit(builder, mt_proof);
-        for _ in 0..self.params.max_signed_pod_values {
+        builder.connect_hashes(signed_pod.id, mt_proof.root);
-            let mt_proof = MerkleProofExistenceGadget {
+        // mt_proofs.push(mt_proof);
-                max_depth: self.params.max_depth_mt_containers,
-            }
-            .eval(builder)?;
-            builder.connect_hashes(id, mt_proof.root);
-            mt_proofs.push(mt_proof);
-        }
-
-        // 2. Verify type
-        let type_mt_proof = &mt_proofs[0];
-        let key_type = builder.constant_value(hash_str(KEY_TYPE).into());
-        builder.connect_values(type_mt_proof.key, key_type);
-        let value_type = builder.constant_value(Value::from(PodType::Signed).raw());
-        builder.connect_values(type_mt_proof.value, value_type);
-
-        // 3.a. Verify signature
-        let signature = SignatureVerifyGadget {}.eval(builder)?;
-
-        // 3.b. Verify signer (ie. hash(signature.pk) == merkletree.signer_leaf)
-        let signer_mt_proof = &mt_proofs[1];
-        let key_signer = builder.constant_value(Key::from(KEY_SIGNER).raw());
-        let pk_hash = signature.pk.to_value(builder);
-        builder.connect_values(signer_mt_proof.key, key_signer);
-        builder.connect_values(signer_mt_proof.value, pk_hash);
-
-        // 3.c. connect signed message to pod.id
-        builder.connect_values(ValueTarget::from_slice(&id.elements), signature.msg);
-
-        measure_gates_end!(builder, measure);
-        Ok(SignedPodVerifyTarget {
-            params: self.params.clone(),
-            id,
-            mt_proofs,
-            signature,
-        })
     }
+
+    // 2. Verify type
+    let type_mt_proof = &signed_pod.mt_proofs[0];
+    let key_type = builder.constant_value(hash_str(KEY_TYPE).into());
+    builder.connect_values(type_mt_proof.key, key_type);
+    let value_type = builder.constant_value(Value::from(PodType::Signed).raw());
+    builder.connect_values(type_mt_proof.value, value_type);
+
+    // 3.a. Verify signature
+    verify_signature_circuit(builder, &signed_pod.signature);
+
+    // 3.b. Verify signer (ie. hash(signature.pk) == merkletree.signer_leaf)
+    let signer_mt_proof = &signed_pod.mt_proofs[1];
+    let key_signer = builder.constant_value(Key::from(KEY_SIGNER).raw());
+    let pk_hash = signed_pod.signature.pk.to_value(builder);
+    builder.connect_values(signer_mt_proof.key, key_signer);
+    builder.connect_values(signer_mt_proof.value, pk_hash);
+
+    // 3.c. connect signed message to pod.id
+    builder.connect_values(
+        ValueTarget::from_slice(&signed_pod.id.elements),
+        signed_pod.signature.msg,
+    );
+
+    measure_gates_end!(builder, measure);
+    Ok(())
 }
 
 pub struct SignedPodVerifyTarget {
@@ -88,6 +81,18 @@ pub struct SignedPodVerifyTarget {
 }
 
 impl SignedPodVerifyTarget {
+    pub fn new_virtual(params: &Params, builder: &mut CircuitBuilder<F, D>) -> Self {
+        SignedPodVerifyTarget {
+            params: params.clone(),
+            id: builder.add_virtual_hash(),
+            mt_proofs: (0..params.max_signed_pod_values)
+                .map(|_| {
+                    MerkleProofExistenceTarget::new_virtual(params.max_depth_mt_containers, builder)
+                })
+                .collect(),
+            signature: SignatureVerifyTarget::new_virtual(builder),
+        }
+    }
     pub fn pub_statements(
         &self,
         builder: &mut CircuitBuilder<F, D>,
@@ -229,7 +234,8 @@ pub mod tests {
         let mut pw = PartialWitness::<F>::new();
 
         // build the circuit logic
-        let signed_pod_verify = SignedPodVerifyGadget { params }.eval(&mut builder)?;
+        let signed_pod_verify = SignedPodVerifyTarget::new_virtual(&params, &mut builder);
+        verify_signed_pod_circuit(&mut builder, &signed_pod_verify)?;
 
         // set the signed_pod as target values for the circuit
         signed_pod_verify.set_targets(&mut pw, &signed_pod)?;

src/backends/plonky2/emptypod.rs

@@ -20,7 +20,7 @@ use crate::{
         basetypes::{Proof, C, D},
         circuits::{
             common::{Flattenable, StatementTarget},
-            mainpod::{CalculateIdGadget, PI_OFFSET_ID},
+            mainpod::{calculate_id_circuit, PI_OFFSET_ID},
         },
         deserialize_proof,
         error::{Error, Result},
@@ -52,10 +52,7 @@ impl EmptyPodVerifyCircuit {
             &self.params,
             &builder.constants(&type_statement().to_fields(&self.params)),
         );
-        let id = CalculateIdGadget {
+        let id = calculate_id_circuit(&self.params, builder, &[type_statement]);
-            params: self.params.clone(),
-        }
-        .eval(builder, &[type_statement]);
         let vds_root = builder.add_virtual_hash();
         builder.register_public_inputs(&id.elements);
         builder.register_public_inputs(&vds_root.elements);

src/backends/plonky2/primitives/ec/schnorr.rs

@@ -133,6 +133,10 @@ pub trait WitnessWriteSchnorr: WitnessWrite<GoldilocksField> + WitnessWriteCurve
 
 impl<W: WitnessWrite<GoldilocksField>> WitnessWriteSchnorr for W {}
 
+// TODO: Rename this to a function `verify_signature_circuit`?  I think this convention is also
+// nice as an object-oriented alternative to `verb_object_circuit` methods.  It's clear that this
+// is constraining vs native operation because the type is `*Target`.  But of course this
+// convention doesn't work for all situations.
 impl SignatureTarget {
     pub fn verify(
         &self,

src/backends/plonky2/primitives/merkletree/circuit.rs

@@ -34,14 +34,6 @@ use crate::{
     middleware::{EMPTY_HASH, EMPTY_VALUE, F, HASH_SIZE},
 };
 
-/// `MerkleProofGadget` allows to verify both proofs of existence and proofs
-/// non-existence with the same circuit.
-/// If only proofs of existence are needed, use `MerkleProofExistenceGadget`,
-/// which requires less amount of constraints.
-pub struct MerkleProofGadget {
-    pub max_depth: usize,
-}
-
 #[derive(Clone, Debug)]
 pub struct MerkleClaimAndProofTarget {
     pub(crate) max_depth: usize,
@@ -57,108 +49,103 @@ pub struct MerkleClaimAndProofTarget {
     pub(crate) other_value: ValueTarget,
 }
 
-impl MerkleProofGadget {
+/// Allows to verify both proofs of existence and proofs non-existence with the same circuit. If
-    /// creates the targets and defines the logic of the circuit
+/// only proofs of existence are needed, use `verify_merkle_proof_existence_circuit`, which
-    pub fn eval(&self, builder: &mut CircuitBuilder<F, D>) -> MerkleClaimAndProofTarget {
+/// requires less amount of constraints.
-        let measure = measure_gates_begin!(builder, format!("MerkleProof_{}", self.max_depth));
+pub fn verify_merkle_proof_circuit(
-        let enabled = builder.add_virtual_bool_target_safe();
+    builder: &mut CircuitBuilder<F, D>,
-        let root = builder.add_virtual_hash();
+    proof: &MerkleClaimAndProofTarget,
-        let key = builder.add_virtual_value();
+) {
-        let value = builder.add_virtual_value();
+    let max_depth = proof.max_depth;
-        // from proof struct:
+    let measure = measure_gates_begin!(builder, format!("MerkleProof_{}", max_depth));
-        let existence = builder.add_virtual_bool_target_safe();
-        // siblings are padded till max_depth length
-        let siblings = builder.add_virtual_hashes(self.max_depth);
 
-        let case_ii_selector = builder.add_virtual_bool_target_safe();
+    // We have 3 cases for when computing the Leaf's hash:
-        let other_key = builder.add_virtual_value();
+    // - existence: leaf contains the given key & value
-        let other_value = builder.add_virtual_value();
+    // - non-existence:
+    //      - case i) expected leaf does not exist
+    //      - case ii) expected leaf does exist but it has a different key
+    //
+    // The following table expresses the options with their in-circuit
+    // selectors:
+    // | existence   | case_ii   | leaf_hash                    |
+    // | ----------- | --------- | ---------------------------- |
+    // | 1           | 0         | H(key, value, 1)             |
+    // | 0           | 0         | EMPTY_HASH                   |
+    // | 0           | 1         | H(other_key, other_value, 1) |
+    // | 1           | 1         | invalid combination          |
 
-        // We have 3 cases for when computing the Leaf's hash:
+    // First, ensure that both existence & case_ii are not true at the same
-        // - existence: leaf contains the given key & value
+    // time:
-        // - non-existence:
+    // 1. sum = existence + case_ii_selector
-        //      - case i) expected leaf does not exist
+    let sum = builder.add(proof.existence.target, proof.case_ii_selector.target);
-        //      - case ii) expected leaf does exist but it has a different key
+    // 2. sum * (sum-1) == 0
-        //
+    builder.assert_bool(BoolTarget::new_unsafe(sum));
-        // The following table expresses the options with their in-circuit
-        // selectors:
-        // | existence   | case_ii   | leaf_hash                    |
-        // | ----------- | --------- | ---------------------------- |
-        // | 1           | 0         | H(key, value, 1)             |
-        // | 0           | 0         | EMPTY_HASH                   |
-        // | 0           | 1         | H(other_key, other_value, 1) |
-        // | 1           | 1         | invalid combination          |
 
-        // First, ensure that both existence & case_ii are not true at the same
+    // define the case_i_selector as true when both existence and
-        // time:
+    // case_ii_selector are false:
-        // 1. sum = existence + case_ii_selector
+    let not_existence = builder.not(proof.existence);
-        let sum = builder.add(existence.target, case_ii_selector.target);
+    let not_case_ii_selector = builder.not(proof.case_ii_selector);
-        // 2. sum * (sum-1) == 0
+    let case_i_selector = builder.and(not_existence, not_case_ii_selector);
-        builder.assert_bool(BoolTarget::new_unsafe(sum));
 
-        // define the case_i_selector as true when both existence and
+    // use (key,value) or (other_key, other_value) depending if it's a proof
-        // case_ii_selector are false:
+    // of existence or of non-existence, ie:
-        let not_existence = builder.not(existence);
+    // k = key * existence + other_key * (1-existence)
-        let not_case_ii_selector = builder.not(case_ii_selector);
+    // v = value * existence + other_value * (1-existence)
-        let case_i_selector = builder.and(not_existence, not_case_ii_selector);
+    let k = builder.select_value(proof.existence, proof.key, proof.other_key);
+    let v = builder.select_value(proof.existence, proof.value, proof.other_value);
 
-        // use (key,value) or (other_key, other_value) depending if it's a proof
+    // get leaf's hash for the selected k & v
-        // of existence or of non-existence, ie:
+    let h = kv_hash_target(builder, &k, &v);
-        // k = key * existence + other_key * (1-existence)
-        // v = value * existence + other_value * (1-existence)
-        let k = builder.select_value(existence, key, other_key);
-        let v = builder.select_value(existence, value, other_value);
 
-        // get leaf's hash for the selected k & v
+    // if we're in the case i), use leaf_hash=EMPTY_HASH, else use the
-        let h = kv_hash_target(builder, &k, &v);
+    // previously computed hash h.
+    let empty_hash = builder.constant_hash(HashOut::from(EMPTY_HASH.0));
+    let leaf_hash = HashOutTarget::from_vec(
+        (0..HASH_SIZE)
+            .map(|j| builder.select(case_i_selector, empty_hash.elements[j], h.elements[j]))
+            .collect(),
+    );
 
-        // if we're in the case i), use leaf_hash=EMPTY_HASH, else use the
+    // get key's path
-        // previously computed hash h.
+    let path = keypath_target(max_depth, builder, &proof.key);
-        let empty_hash = builder.constant_hash(HashOut::from(EMPTY_HASH.0));
-        let leaf_hash = HashOutTarget::from_vec(
-            (0..HASH_SIZE)
-                .map(|j| builder.select(case_i_selector, empty_hash.elements[j], h.elements[j]))
-                .collect(),
-        );
 
-        // get key's path
+    // compute the root for the given siblings and the computed leaf_hash
-        let path = keypath_target(self.max_depth, builder, &key);
+    // (this is for the three cases (existence, non-existence case i, and
+    // non-existence case ii).
+    let obtained_root =
+        compute_root_from_leaf(max_depth, builder, &path, &leaf_hash, &proof.siblings);
 
-        // compute the root for the given siblings and the computed leaf_hash
+    // check that obtained_root==root (from inputs), when enabled==true
-        // (this is for the three cases (existence, non-existence case i, and
+    let zero = builder.zero();
-        // non-existence case ii).
+    let expected_root: Vec<Target> = (0..HASH_SIZE)
-        let obtained_root =
+        .map(|j| builder.select(proof.enabled, proof.root.elements[j], zero))
-            compute_root_from_leaf(self.max_depth, builder, &path, &leaf_hash, &siblings);
+        .collect();
-
+    let computed_root: Vec<Target> = (0..HASH_SIZE)
-        // check that obtained_root==root (from inputs), when enabled==true
+        .map(|j| builder.select(proof.enabled, obtained_root.elements[j], zero))
-        let zero = builder.zero();
+        .collect();
-        let expected_root: Vec<Target> = (0..HASH_SIZE)
+    for j in 0..HASH_SIZE {
-            .map(|j| builder.select(enabled, root.elements[j], zero))
+        builder.connect(computed_root[j], expected_root[j]);
-            .collect();
-        let computed_root: Vec<Target> = (0..HASH_SIZE)
-            .map(|j| builder.select(enabled, obtained_root.elements[j], zero))
-            .collect();
-        for j in 0..HASH_SIZE {
-            builder.connect(computed_root[j], expected_root[j]);
-        }
-        measure_gates_end!(builder, measure);
-
-        MerkleClaimAndProofTarget {
-            max_depth: self.max_depth,
-            enabled,
-            existence,
-            root,
-            siblings,
-            key,
-            value,
-            case_ii_selector,
-            other_key,
-            other_value,
-        }
     }
+    measure_gates_end!(builder, measure);
 }
 
 impl MerkleClaimAndProofTarget {
+    pub fn new_virtual(max_depth: usize, builder: &mut CircuitBuilder<F, D>) -> Self {
+        MerkleClaimAndProofTarget {
+            max_depth,
+            enabled: builder.add_virtual_bool_target_safe(),
+            root: builder.add_virtual_hash(),
+            key: builder.add_virtual_value(),
+            value: builder.add_virtual_value(),
+            // from proof struct:
+            existence: builder.add_virtual_bool_target_safe(),
+            // siblings are padded till max_depth length
+            siblings: builder.add_virtual_hashes(max_depth),
+            case_ii_selector: builder.add_virtual_bool_target_safe(),
+            other_key: builder.add_virtual_value(),
+            other_value: builder.add_virtual_value(),
+        }
+    }
     /// assigns the given values to the targets
     #[allow(clippy::too_many_arguments)]
     pub fn set_targets(
@@ -205,12 +192,6 @@ impl MerkleClaimAndProofTarget {
     }
 }
 
-/// `MerkleProofExistenceCircuit` allows to verify proofs of existence only. If
-/// proofs of non-existence are needed, use `MerkleProofCircuit`.
-pub struct MerkleProofExistenceGadget {
-    pub max_depth: usize,
-}
-
 pub struct MerkleProofExistenceTarget {
     max_depth: usize,
     // `enabled` determines if the merkleproof verification is enabled
@@ -221,52 +202,51 @@ pub struct MerkleProofExistenceTarget {
     pub(crate) siblings: Vec<HashOutTarget>,
 }
 
-impl MerkleProofExistenceGadget {
+/// Allows to verify proofs of existence only. If proofs of non-existence are needed, use
-    /// creates the targets and defines the logic of the circuit
+/// `verify_merkle_proof_circuit`.
-    pub fn eval(&self, builder: &mut CircuitBuilder<F, D>) -> Result<MerkleProofExistenceTarget> {
+pub fn verify_merkle_proof_existence_circuit(
-        let measure = measure_gates_begin!(builder, format!("MerkleProofExist_{}", self.max_depth));
+    builder: &mut CircuitBuilder<F, D>,
-        let enabled = builder.add_virtual_bool_target_safe();
+    proof: &MerkleProofExistenceTarget,
-        let root = builder.add_virtual_hash();
+) {
-        let key = builder.add_virtual_value();
+    let max_depth = proof.max_depth;
-        let value = builder.add_virtual_value();
+    let measure = measure_gates_begin!(builder, format!("MerkleProofExist_{}", max_depth));
-        // siblings are padded till max_depth length
-        let siblings = builder.add_virtual_hashes(self.max_depth);
 
-        // get leaf's hash for the selected k & v
+    // get leaf's hash for the selected k & v
-        let leaf_hash = kv_hash_target(builder, &key, &value);
+    let leaf_hash = kv_hash_target(builder, &proof.key, &proof.value);
 
-        // get key's path
+    // get key's path
-        let path = keypath_target(self.max_depth, builder, &key);
+    let path = keypath_target(max_depth, builder, &proof.key);
 
-        // compute the root for the given siblings and the computed leaf_hash.
+    // compute the root for the given siblings and the computed leaf_hash.
-        let obtained_root =
+    let obtained_root =
-            compute_root_from_leaf(self.max_depth, builder, &path, &leaf_hash, &siblings);
+        compute_root_from_leaf(max_depth, builder, &path, &leaf_hash, &proof.siblings);
 
-        // check that obtained_root==root (from inputs), when enabled==true
+    // check that obtained_root==root (from inputs), when enabled==true
-        let zero = builder.zero();
+    let zero = builder.zero();
-        let expected_root: Vec<Target> = (0..HASH_SIZE)
+    let expected_root: Vec<Target> = (0..HASH_SIZE)
-            .map(|j| builder.select(enabled, root.elements[j], zero))
+        .map(|j| builder.select(proof.enabled, proof.root.elements[j], zero))
-            .collect();
+        .collect();
-        let computed_root: Vec<Target> = (0..HASH_SIZE)
+    let computed_root: Vec<Target> = (0..HASH_SIZE)
-            .map(|j| builder.select(enabled, obtained_root.elements[j], zero))
+        .map(|j| builder.select(proof.enabled, obtained_root.elements[j], zero))
-            .collect();
+        .collect();
-        for j in 0..HASH_SIZE {
+    for j in 0..HASH_SIZE {
-            builder.connect(computed_root[j], expected_root[j]);
+        builder.connect(computed_root[j], expected_root[j]);
-        }
-        measure_gates_end!(builder, measure);
-
-        Ok(MerkleProofExistenceTarget {
-            max_depth: self.max_depth,
-            enabled,
-            root,
-            siblings,
-            key,
-            value,
-        })
     }
+    measure_gates_end!(builder, measure);
 }
 
 impl MerkleProofExistenceTarget {
+    pub fn new_virtual(max_depth: usize, builder: &mut CircuitBuilder<F, D>) -> Self {
+        MerkleProofExistenceTarget {
+            max_depth,
+            enabled: builder.add_virtual_bool_target_safe(),
+            root: builder.add_virtual_hash(),
+            key: builder.add_virtual_value(),
+            value: builder.add_virtual_value(),
+            // siblings are padded till max_depth length
+            siblings: builder.add_virtual_hashes(max_depth),
+        }
+    }
     /// assigns the given values to the targets
     pub fn set_targets(
         &self,
@@ -545,7 +525,8 @@ pub mod tests {
         let mut builder = CircuitBuilder::<F, D>::new(config);
         let mut pw = PartialWitness::<F>::new();
 
-        let targets = MerkleProofGadget { max_depth }.eval(&mut builder);
+        let targets = MerkleClaimAndProofTarget::new_virtual(max_depth, &mut builder);
+        verify_merkle_proof_circuit(&mut builder, &targets);
         targets.set_targets(
             &mut pw,
             true,
@@ -591,7 +572,8 @@ pub mod tests {
         let mut builder = CircuitBuilder::<F, D>::new(config);
         let mut pw = PartialWitness::<F>::new();
 
-        let targets = MerkleProofExistenceGadget { max_depth }.eval(&mut builder)?;
+        let targets = MerkleClaimAndProofTarget::new_virtual(max_depth, &mut builder);
+        verify_merkle_proof_circuit(&mut builder, &targets);
         targets.set_targets(
             &mut pw,
             true,
@@ -666,7 +648,8 @@ pub mod tests {
         let mut builder = CircuitBuilder::<F, D>::new(config);
         let mut pw = PartialWitness::<F>::new();
 
-        let targets = MerkleProofGadget { max_depth }.eval(&mut builder);
+        let targets = MerkleClaimAndProofTarget::new_virtual(max_depth, &mut builder);
+        verify_merkle_proof_circuit(&mut builder, &targets);
         targets.set_targets(
             &mut pw,
             true,
@@ -713,7 +696,8 @@ pub mod tests {
         let mut builder = CircuitBuilder::<F, D>::new(config);
         let mut pw = PartialWitness::<F>::new();
 
-        let targets = MerkleProofGadget { max_depth }.eval(&mut builder);
+        let targets = MerkleClaimAndProofTarget::new_virtual(max_depth, &mut builder);
+        verify_merkle_proof_circuit(&mut builder, &targets);
         // verification enabled & proof of existence
         let mp = MerkleClaimAndProof::new(tree2.root(), key, Some(value), proof);
         targets.set_targets(&mut pw, true, &mp)?;
@@ -729,7 +713,8 @@ pub mod tests {
         let mut builder = CircuitBuilder::<F, D>::new(config);
         let mut pw = PartialWitness::<F>::new();
 
-        let targets = MerkleProofGadget { max_depth }.eval(&mut builder);
+        let targets = MerkleClaimAndProofTarget::new_virtual(max_depth, &mut builder);
+        verify_merkle_proof_circuit(&mut builder, &targets);
         // verification disabled & proof of existence
         targets.set_targets(&mut pw, false, &mp)?;
 

src/backends/plonky2/primitives/signature/circuit.rs

@@ -35,8 +35,9 @@ use crate::{
     middleware::{Hash, Proof, RawValue, EMPTY_HASH, EMPTY_VALUE, F, VALUE_SIZE},
 };
 
-pub struct SignatureVerifyGadget;
+// TODO: This is a very simple wrapper over the signature verification implemented on
-
+// `SignatureTarget`.  I think we can remove this and use it directly.  Also we're not using the
+// `enabled` flag, so it should be straight-forward to remove this.
 pub struct SignatureVerifyTarget {
     // `enabled` determines if the signature verification is enabled
     pub(crate) enabled: BoolTarget,
@@ -46,32 +47,34 @@ pub struct SignatureVerifyTarget {
     pub(crate) sig: SignatureTarget,
 }
 
-impl SignatureVerifyGadget {
+pub fn verify_signature_circuit(
-    /// creates the targets and defines the logic of the circuit
+    builder: &mut CircuitBuilder<F, D>,
-    pub fn eval(&self, builder: &mut CircuitBuilder<F, D>) -> Result<SignatureVerifyTarget> {
+    signature: &SignatureVerifyTarget,
-        let measure = measure_gates_begin!(builder, "SignatureVerify");
+) {
-        let enabled = builder.add_virtual_bool_target_safe();
+    let measure = measure_gates_begin!(builder, "SignatureVerify");
-        let pk = builder.add_virtual_point_target();
+    let verified = signature.sig.verify(
-        let msg = builder.add_virtual_value();
+        builder,
-        let sig = builder.add_virtual_schnorr_signature_target();
+        HashOutTarget::from(signature.msg.elements),
-
+        &signature.pk,
-        let verified = sig.verify(builder, HashOutTarget::from(msg.elements), &pk);
+    );
-
+    let result = builder.mul_sub(
-        let result = builder.mul_sub(enabled.target, verified.target, enabled.target);
+        signature.enabled.target,
-
+        verified.target,
-        builder.assert_zero(result);
+        signature.enabled.target,
-
+    );
-        measure_gates_end!(builder, measure);
+    builder.assert_zero(result);
-        Ok(SignatureVerifyTarget {
+    measure_gates_end!(builder, measure);
-            enabled,
-            pk,
-            msg,
-            sig,
-        })
-    }
 }
 
 impl SignatureVerifyTarget {
+    pub fn new_virtual(builder: &mut CircuitBuilder<F, D>) -> Self {
+        SignatureVerifyTarget {
+            enabled: builder.add_virtual_bool_target_safe(),
+            pk: builder.add_virtual_point_target(),
+            msg: builder.add_virtual_value(),
+            sig: builder.add_virtual_schnorr_signature_target(),
+        }
+    }
     /// assigns the given values to the targets
     pub fn set_targets(
         &self,
@@ -115,7 +118,8 @@ pub mod tests {
         let mut builder = CircuitBuilder::<F, D>::new(config);
         let mut pw = PartialWitness::<F>::new();
 
-        let targets = SignatureVerifyGadget {}.eval(&mut builder)?;
+        let targets = SignatureVerifyTarget::new_virtual(&mut builder);
+        verify_signature_circuit(&mut builder, &targets);
         targets.set_targets(&mut pw, true, pk, msg, sig)?;
 
         // generate & verify proof
@@ -147,8 +151,9 @@ pub mod tests {
         // circuit
         let config = CircuitConfig::standard_recursion_zk_config();
         let mut builder = CircuitBuilder::<F, D>::new(config);
+        let targets = SignatureVerifyTarget::new_virtual(&mut builder);
+        verify_signature_circuit(&mut builder, &targets);
         let mut pw = PartialWitness::<F>::new();
-        let targets = SignatureVerifyGadget {}.eval(&mut builder)?;
         targets.set_targets(&mut pw, true, pk, msg, sig.clone())?; // enabled=true
 
         // generate proof, and expect it to fail
@@ -162,7 +167,8 @@ pub mod tests {
         let mut builder = CircuitBuilder::<F, D>::new(config);
         let mut pw = PartialWitness::<F>::new();
 
-        let targets = SignatureVerifyGadget {}.eval(&mut builder)?;
+        let targets = SignatureVerifyTarget::new_virtual(&mut builder);
+        verify_signature_circuit(&mut builder, &targets);
         targets.set_targets(&mut pw, false, pk, msg, sig)?; // enabled=false
 
         // generate & verify proof