From 4a1615f968f5a7300ed7323148edda9da9c5ca01 Mon Sep 17 00:00:00 2001
From: Zack Youell <zackyouell@mac.com>
Date: Sun, 26 Jan 2025 15:03:23 +0000
Subject: [PATCH] Deterministic `PublicParameters` Serialisation (#437)

This PR resolves CRY-35 and is joint work with @nicholas-mainardi.

It changes the use of a `HashMap<Vec<F>, usize>` in
`recursion-framework/src/universal_verifier_gadget` to a
`BTreeMap<Vec<u64>, usize>` for deterministic iteration.

In addition to this it incorporates @nicholas-mainardi solution for
avoiding directly serialising the `empty_node_proof` in
`verifiable_db::cells_tree::PublicParamaters`.
---
 .../circuit_data_serialization.rs             |  1 +
 mp2-v1/src/api.rs                             | 25 ++++++++
 mp2-v1/src/query/planner.rs                   |  4 +-
 .../universal_verifier_gadget/circuit_set.rs  | 31 ++++++----
 .../universal_verifier_gadget/wrap_circuit.rs |  1 +
 ryhope/src/tree/scapegoat.rs                  |  2 +-
 verifiable-db/src/api.rs                      | 20 +++++++
 verifiable-db/src/cells_tree/api.rs           | 58 ++++++++++++++-----
 verifiable-db/src/ivc/api.rs                  |  7 ++-
 9 files changed, 116 insertions(+), 33 deletions(-)

diff --git a/mp2-common/src/serialization/circuit_data_serialization.rs b/mp2-common/src/serialization/circuit_data_serialization.rs
index 07895189a..cb0a0ead9 100644
--- a/mp2-common/src/serialization/circuit_data_serialization.rs
+++ b/mp2-common/src/serialization/circuit_data_serialization.rs
@@ -342,6 +342,7 @@ pub(super) mod tests {
     }
 
     #[rstest]
+    #[case(3, 0)]
     #[case(6, 0)]
     #[case(16, 0)]
     #[case(17, 2)]
diff --git a/mp2-v1/src/api.rs b/mp2-v1/src/api.rs
index 524950133..9e15d4497 100644
--- a/mp2-v1/src/api.rs
+++ b/mp2-v1/src/api.rs
@@ -248,3 +248,28 @@ pub fn metadata_hash(
     // compute final hash
     combine_digest_and_block(contract_digest + value_digest)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_deterministic_serialisation() {
+        let params_1 = build_circuits_params();
+        let serialised_1 = bincode::serialize(&params_1).unwrap();
+
+        let params_2 = build_circuits_params();
+        let serialised_2 = bincode::serialize(&params_2).unwrap();
+
+        serialised_1
+            .iter()
+            .zip(serialised_2.iter())
+            .enumerate()
+            .for_each(|(index, (&byte_1, &byte_2))| {
+                assert_eq!(
+                    byte_1, byte_2,
+                    "Parameter serialisations not the same, discrepancy occurs at index: {index}"
+                )
+            })
+    }
+}
diff --git a/mp2-v1/src/query/planner.rs b/mp2-v1/src/query/planner.rs
index 7426da087..a01fe91e7 100644
--- a/mp2-v1/src/query/planner.rs
+++ b/mp2-v1/src/query/planner.rs
@@ -217,7 +217,7 @@ pub trait TreeFetcher<K: Debug + Clone + Eq + PartialEq, V: LagrangeNode>: Sized
                 } else {
                     // we don't found the right child node in the tree, which means that the
                     // successor might be out of range, so we return None
-                    return None;
+                    None
                 }
             } else {
                 // find successor among the ancestors of current node: we go up in the path
@@ -298,7 +298,7 @@ pub trait TreeFetcher<K: Debug + Clone + Eq + PartialEq, V: LagrangeNode>: Sized
                 } else {
                     // we don't found the left child node in the tree, which means that the
                     // predecessor might be out of range, so we return None
-                    return None;
+                    None
                 }
             } else {
                 // find predecessor among the ancestors of current node: we go up in the path
diff --git a/recursion-framework/src/universal_verifier_gadget/circuit_set.rs b/recursion-framework/src/universal_verifier_gadget/circuit_set.rs
index ee5a6c189..5bbe4698c 100644
--- a/recursion-framework/src/universal_verifier_gadget/circuit_set.rs
+++ b/recursion-framework/src/universal_verifier_gadget/circuit_set.rs
@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 
 use plonky2::{
     field::extension::Extendable,
@@ -160,12 +160,8 @@ pub(crate) fn check_circuit_digest_target<
 #[serde(bound = "")]
 /// Data structure employed by the recursion framework to store and manage the set of circuits whose proofs
 /// can be verified with the universal verifier bound to such a set
-pub(crate) struct CircuitSet<
-    F: RichField + Extendable<D>,
-    C: GenericConfig<D, F = F>,
-    const D: usize,
-> {
-    circuit_digests_to_leaf_indexes: HashMap<Vec<F>, usize>,
+pub struct CircuitSet<F: RichField + Extendable<D>, C: GenericConfig<D, F = F>, const D: usize> {
+    circuit_digests_to_leaf_indexes: BTreeMap<Vec<u64>, usize>,
     #[serde(serialize_with = "serialize", deserialize_with = "deserialize")]
     mt: MerkleTree<F, C::Hasher>,
 }
@@ -175,12 +171,12 @@ where
     C::Hasher: AlgebraicHasher<F>,
 {
     pub(crate) fn build_circuit_set(circuit_digests: Vec<<C::Hasher as Hasher<F>>::Hash>) -> Self {
-        let (circuit_digests_to_leaf_indexes, mut leaves): (HashMap<Vec<F>, usize>, Vec<_>) =
+        let (circuit_digests_to_leaf_indexes, mut leaves): (BTreeMap<Vec<u64>, usize>, Vec<_>) =
             circuit_digests
                 .iter()
                 .enumerate()
                 .map(|(index, hash)| {
-                    let hash_to_fes = hash.to_vec();
+                    let hash_to_fes = hash.to_vec().iter().map(|x| x.to_canonical_u64()).collect();
                     ((hash_to_fes, index), hash.to_vec())
                 })
                 .unzip();
@@ -195,7 +191,13 @@ where
     }
 
     fn leaf_index(&self, digest: &[F]) -> Option<usize> {
-        self.circuit_digests_to_leaf_indexes.get(digest).cloned()
+        let digest_u64 = digest
+            .iter()
+            .map(|x| x.to_canonical_u64())
+            .collect::<Vec<u64>>();
+        self.circuit_digests_to_leaf_indexes
+            .get(&digest_u64)
+            .cloned()
     }
 
     /// set a `CircuitSetMembershipTargets` to prove membership of `circuit_digest` in the set of
@@ -289,7 +291,7 @@ mod tests {
 
     use super::*;
     use mp2_common::{C, D, F};
-    use plonky2::field::types::Sample;
+    use plonky2::field::types::{Field, Sample};
 
     #[test]
     fn test_circuit_set_gadgets() {
@@ -332,7 +334,12 @@ mod tests {
                 .iter()
                 .zip(circuit_membership_targets.iter().zip(elements))
                 .for_each(|(&el_t, (membership_t, el))| {
-                    let el = HashOut::from_vec(el.clone());
+                    let el_f = el
+                        .iter()
+                        .copied()
+                        .map(F::from_canonical_u64)
+                        .collect::<Vec<F>>();
+                    let el = HashOut::from_vec(el_f);
                     pw.set_hash_target(el_t, el);
                     circuit_set
                         .set_circuit_membership_target(&mut pw, membership_t, el)
diff --git a/recursion-framework/src/universal_verifier_gadget/wrap_circuit.rs b/recursion-framework/src/universal_verifier_gadget/wrap_circuit.rs
index b84650c47..5cc4e3960 100644
--- a/recursion-framework/src/universal_verifier_gadget/wrap_circuit.rs
+++ b/recursion-framework/src/universal_verifier_gadget/wrap_circuit.rs
@@ -108,6 +108,7 @@ where
                 wrap_step + 1,
                 cd.degree_bits()
             );
+
             if circuit_data.common.degree_bits() == RECURSION_THRESHOLD {
                 break;
             }
diff --git a/ryhope/src/tree/scapegoat.rs b/ryhope/src/tree/scapegoat.rs
index b97165277..4ff0cea9c 100644
--- a/ryhope/src/tree/scapegoat.rs
+++ b/ryhope/src/tree/scapegoat.rs
@@ -312,7 +312,7 @@ impl<K: Debug + Sync + Clone + Eq + Hash + Ord + Serialize + for<'a> Deserialize
                 }
             }
         } else {
-            return Err(RyhopeError::fatal("the tree is empty"));
+            Err(RyhopeError::fatal("the tree is empty"))
         }
     }
 
diff --git a/verifiable-db/src/api.rs b/verifiable-db/src/api.rs
index 03739c65e..949e8d52d 100644
--- a/verifiable-db/src/api.rs
+++ b/verifiable-db/src/api.rs
@@ -68,6 +68,26 @@ where
     pub fn empty_cell_tree_proof(&self) -> Result<Vec<u8>> {
         self.cells_tree.empty_cell_tree_proof()
     }
+
+    /// Get the cell tree params
+    pub fn get_cells_params(&self) -> &cells_tree::PublicParameters {
+        &self.cells_tree
+    }
+
+    /// Get the rows tree params
+    pub fn get_row_params(&self) -> &row_tree::PublicParameters {
+        &self.rows_tree
+    }
+
+    /// Get the block tree params
+    pub fn get_block_params(&self) -> &block_tree::PublicParameters<E> {
+        &self.block_tree
+    }
+
+    /// Get the IVC params
+    pub fn get_ivc_params(&self) -> &ivc::PublicParameters {
+        &self.ivc
+    }
 }
 
 /// Instantiate the circuits employed for the verifiable DB stage of LPN, and return their corresponding parameters.
diff --git a/verifiable-db/src/cells_tree/api.rs b/verifiable-db/src/cells_tree/api.rs
index 8cb5fb56a..db353acf3 100644
--- a/verifiable-db/src/cells_tree/api.rs
+++ b/verifiable-db/src/cells_tree/api.rs
@@ -97,14 +97,37 @@ fn new_child_input(
     }
 }
 
-/// Main struct holding the different circuit parameters for each of the circuits defined here.
 #[derive(Serialize, Deserialize)]
-pub struct PublicParameters {
+struct ParametersInner {
     leaf: CircuitWithUniversalVerifier<F, C, D, 0, LeafWires>,
     full_node: CircuitWithUniversalVerifier<F, C, D, 2, FullNodeWires>,
     partial_node: CircuitWithUniversalVerifier<F, C, D, 1, PartialNodeWires>,
     empty_node: CircuitWithUniversalVerifier<F, C, D, 0, EmptyNodeWires>,
     set: RecursiveCircuits<F, C, D>,
+}
+
+impl From<ParametersInner> for PublicParameters {
+    fn from(value: ParametersInner) -> Self {
+        // Generate the proof for the empty node. It could be reused.
+        let proof = value
+            .set
+            .generate_proof(&value.empty_node, [], [], EmptyNodeCircuit)
+            .unwrap();
+        let empty_node_proof =
+            ProofWithVK::from((proof, value.empty_node.get_verifier_data().clone()));
+        Self {
+            inner: value,
+            empty_node_proof,
+        }
+    }
+}
+
+/// Main struct holding the different circuit parameters for each of the circuits defined here.
+#[derive(Serialize, Deserialize)]
+#[serde(from = "ParametersInner")]
+pub struct PublicParameters {
+    inner: ParametersInner,
+    #[serde(skip_serializing)]
     empty_node_proof: ProofWithVK,
 }
 
@@ -127,8 +150,11 @@ impl PublicParameters {
             CircuitWithUniversalVerifierBuilder::<F, D, NUM_IO>::new::<C>(config, CIRCUIT_SET_SIZE);
 
         let leaf = builder.build_circuit(());
+
         let full_node = builder.build_circuit(());
+
         let partial_node = builder.build_circuit(());
+
         let empty_node = builder.build_circuit(());
 
         let set = RecursiveCircuits::new_from_circuit_digests(vec![
@@ -145,25 +171,27 @@ impl PublicParameters {
         let empty_node_proof = ProofWithVK::from((proof, empty_node.get_verifier_data().clone()));
 
         PublicParameters {
-            leaf,
-            full_node,
-            partial_node,
-            empty_node,
-            set,
+            inner: ParametersInner {
+                leaf,
+                full_node,
+                partial_node,
+                empty_node,
+                set,
+            },
             empty_node_proof,
         }
     }
 
     pub fn vk_set(&self) -> &RecursiveCircuits<F, C, D> {
-        &self.set
+        &self.inner.set
     }
     pub fn generate_proof(&self, circuit_type: CircuitInput) -> Result<Vec<u8>> {
-        let set = &self.set;
+        let set = &self.inner.set;
 
         let proof_with_vk = match circuit_type {
             CircuitInput::Leaf(leaf) => {
-                let proof = set.generate_proof(&self.leaf, [], [], leaf)?;
-                (proof, self.leaf.get_verifier_data().clone())
+                let proof = set.generate_proof(&self.inner.leaf, [], [], leaf)?;
+                (proof, self.inner.leaf.get_verifier_data().clone())
             }
             CircuitInput::FullNode(node) => {
                 let child_proofs = node.get_child_proofs()?;
@@ -175,12 +203,12 @@ impl PublicParameters {
                 let (child_pis, child_vks): (Vec<_>, Vec<_>) =
                     child_proofs.into_iter().map(|p| (p.proof, p.vk)).unzip();
                 let proof = set.generate_proof(
-                    &self.full_node,
+                    &self.inner.full_node,
                     child_pis.try_into().unwrap(),
                     array::from_fn(|i| &child_vks[i]),
                     node.input.into(),
                 )?;
-                (proof, self.full_node.get_verifier_data().clone())
+                (proof, self.inner.full_node.get_verifier_data().clone())
             }
             CircuitInput::PartialNode(node) => {
                 let mut child_proofs = node.get_child_proofs()?;
@@ -191,12 +219,12 @@ impl PublicParameters {
                 );
                 let (child_proof, child_vk) = child_proofs.pop().unwrap().into();
                 let proof = set.generate_proof(
-                    &self.partial_node,
+                    &self.inner.partial_node,
                     [child_proof],
                     [&child_vk],
                     node.input.into(),
                 )?;
-                (proof, self.partial_node.get_verifier_data().clone())
+                (proof, self.inner.partial_node.get_verifier_data().clone())
             }
         };
 
diff --git a/verifiable-db/src/ivc/api.rs b/verifiable-db/src/ivc/api.rs
index 0ae57d498..5d89b37a0 100644
--- a/verifiable-db/src/ivc/api.rs
+++ b/verifiable-db/src/ivc/api.rs
@@ -108,11 +108,12 @@ impl PublicParameters {
         ProofWithVK::from((proof, self.ivc.circuit_data().verifier_only.clone())).serialize()
     }
 
-    pub(crate) fn get_circuit_set(&self) -> &RecursiveCircuits<F, C, D> {
+    /// Getter for the [`RecursiveCircuits`]
+    pub fn get_circuit_set(&self) -> &RecursiveCircuits<F, C, D> {
         &self.set
     }
-
-    pub(crate) fn get_ivc_circuit_data(&self) -> &CircuitData<F, C, D> {
+    /// Getter for the IVC proof [`CircuitData`]
+    pub fn get_ivc_circuit_data(&self) -> &CircuitData<F, C, D> {
         self.ivc.circuit_data()
     }
 }