From ef7313d8af33d93002ceb95334c461bbc43e8d71 Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Wed, 22 May 2024 11:20:44 +0200 Subject: [PATCH] Make clone create completely independent layers structs. 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. --- arbitrator/prover/src/merkle.rs | 35 ++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/arbitrator/prover/src/merkle.rs b/arbitrator/prover/src/merkle.rs index 70884da096..be1eef8b69 100644 --- a/arbitrator/prover/src/merkle.rs +++ b/arbitrator/prover/src/merkle.rs @@ -32,7 +32,6 @@ use sha3::Keccak256; use std::{ collections::HashSet, convert::{TryFrom, TryInto}, - sync::Arc, }; #[cfg(feature = "rayon")] @@ -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>, + #[serde(with = "mutex_sedre")] + layers: Mutex, min_depth: usize, } @@ -222,6 +221,12 @@ fn new_layer(ty: MerkleType, layer: &Vec, 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 @@ -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, @@ -416,11 +421,10 @@ 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(data: &Arc>, serializer: S) -> Result + pub fn serialize(data: &Mutex, serializer: S) -> Result where S: serde::Serializer, T: serde::Serialize, @@ -428,12 +432,12 @@ pub mod arc_mutex_sedre { data.lock().serialize(serializer) } - pub fn deserialize<'de, D, T>(deserializer: D) -> Result>, D::Error> + pub fn deserialize<'de, D, T>(deserializer: D) -> Result, 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)?)) } } @@ -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);