Skip to content

Commit

Permalink
Make clone create completely independent layers structs.
Browse files Browse the repository at this point in the history
The default behavior of an Arc is to allow sharing between different
threads and different instances. So, the derived behavior for cloning
was to put the same references on the clone as on the orignal.

But, we actually want separate clones of our merkle trees to be
independent. That is, changing the data on the clone, should have no
affect on the original (and vice versa.)

The test added in this commit failed before the change and passes
afer.
  • Loading branch information
eljobe committed May 22, 2024
1 parent bd49d47 commit ef7313d
Showing 1 changed file with 24 additions and 11 deletions.
35 changes: 24 additions & 11 deletions arbitrator/prover/src/merkle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use sha3::Keccak256;
use std::{
collections::HashSet,
convert::{TryFrom, TryInto},
sync::Arc,
};

#[cfg(feature = "rayon")]
Expand Down Expand Up @@ -172,11 +171,11 @@ struct Layers {
/// and passing a minimum depth.
///
/// This structure does not contain the data itself, only the hashes.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
#[derive(Debug, Default, Serialize, Deserialize)]
pub struct Merkle {
ty: MerkleType,
#[serde(with = "arc_mutex_sedre")]
layers: Arc<Mutex<Layers>>,
#[serde(with = "mutex_sedre")]
layers: Mutex<Layers>,
min_depth: usize,
}

Expand Down Expand Up @@ -222,6 +221,12 @@ fn new_layer(ty: MerkleType, layer: &Vec<Bytes32>, empty_hash: &'static Bytes32)
new_layer
}

impl Clone for Merkle {
fn clone(&self) -> Self {
Merkle::new_advanced(self.ty, self.layers.lock().data[0].clone(), self.min_depth)
}
}

impl Merkle {
/// Creates a new Merkle tree with the given type and leaf hashes.
/// The tree is built up to the minimum depth necessary to hold all the
Expand Down Expand Up @@ -253,10 +258,10 @@ impl Merkle {
layers.push(new_layer);
layer_i += 1;
}
let layers = Arc::new(Mutex::new(Layers {
let layers = Mutex::new(Layers {
data: layers,
dirt: dirty_indices,
}));
});
Merkle {
ty,
layers,
Expand Down Expand Up @@ -416,24 +421,23 @@ impl PartialEq for Merkle {

impl Eq for Merkle {}

pub mod arc_mutex_sedre {
pub mod mutex_sedre {
use parking_lot::Mutex;
use std::sync::Arc;

pub fn serialize<S, T>(data: &Arc<Mutex<T>>, serializer: S) -> Result<S::Ok, S::Error>
pub fn serialize<S, T>(data: &Mutex<T>, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
T: serde::Serialize,
{
data.lock().serialize(serializer)
}

pub fn deserialize<'de, D, T>(deserializer: D) -> Result<Arc<Mutex<T>>, D::Error>
pub fn deserialize<'de, D, T>(deserializer: D) -> Result<Mutex<T>, D::Error>
where
D: serde::Deserializer<'de>,
T: serde::Deserialize<'de>,
{
Ok(Arc::new(Mutex::new(T::deserialize(deserializer)?)))
Ok(Mutex::new(T::deserialize(deserializer)?))
}
}

Expand Down Expand Up @@ -547,6 +551,15 @@ fn emit_memory_zerohashes() {
}
}

#[test]
fn clone_is_separate() {
let merkle = Merkle::new_advanced(MerkleType::Value, vec![Bytes32::from([1; 32])], 4);
let m2 = merkle.clone();
m2.resize(4).expect("resize failed");
m2.set(3, Bytes32::from([2; 32]));
assert_ne!(merkle, m2);
}

#[test]
fn serialization_roundtrip() {
let merkle = Merkle::new_advanced(MerkleType::Value, vec![Bytes32::from([1; 32])], 4);
Expand Down

0 comments on commit ef7313d

Please sign in to comment.