Skip to content

Commit

Permalink
fix: impl suggestions (#1064)
Browse files Browse the repository at this point in the history
1. HashSet::find_element_index function, the probe index iteration can be limited to 0..20 similar to insertion, avoiding iteration up to capacity. Use a constant instead of the hardcoded value 20.
2. In HashSet::find_element_iter function, it would be better to iterate from start_iter to (`start_iter + num_iterations`) as per the parameter name.
3. In IndexedMerkleTree::add_highest_element function, simplify indexed_array.append(&init_value) with indexed_array.init().
4. In IndexedArray::hash_element, simplify hashing by using element.hash(next_element.value).
5. In ConcurrentMerkleTree::append_batch, use self.next_index instead of first_leaf_index + leaf_i.
  • Loading branch information
ananas-block authored Aug 9, 2024
1 parent a91cc3d commit 0ccb5b0
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 18 deletions.
8 changes: 3 additions & 5 deletions merkle-tree/concurrent/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,18 +553,16 @@ where
));
}

let first_leaf_index = self.next_index();
let first_changelog_index = (self.changelog.last_index() + 1) % self.changelog.capacity();
let first_sequence_number = self.sequence_number() + 1;

for (leaf_i, leaf) in leaves.iter().enumerate() {
let mut current_index = self.next_index();

self.changelog
.push(ChangelogEntry::<HEIGHT>::default_with_index(
first_leaf_index + leaf_i,
));
.push(ChangelogEntry::<HEIGHT>::default_with_index(current_index));
let changelog_index = self.changelog_index();

let mut current_index = self.next_index();
let mut current_node = **leaf;

self.changelog[changelog_index].path[0] = **leaf;
Expand Down
8 changes: 5 additions & 3 deletions merkle-tree/hash-set/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use thiserror::Error;

pub mod zero_copy;

pub const ITERATIONS: usize = 20;

#[derive(Debug, Error, PartialEq)]
pub enum HashSetError {
#[error("The hash set is full, cannot add any new elements")]
Expand Down Expand Up @@ -309,7 +311,7 @@ impl HashSet {
value: &BigUint,
current_sequence_number: usize,
) -> Result<usize, HashSetError> {
let index_bucket = self.find_element_iter(value, current_sequence_number, 0, 20)?;
let index_bucket = self.find_element_iter(value, current_sequence_number, 0, ITERATIONS)?;
let (index, is_new) = match index_bucket {
Some(index) => index,
None => {
Expand Down Expand Up @@ -347,7 +349,7 @@ impl HashSet {
value: &BigUint,
current_sequence_number: Option<usize>,
) -> Result<Option<usize>, HashSetError> {
for i in 0..self.capacity {
for i in 0..ITERATIONS {
let probe_index = self.probe_index(value, i);
// PANICS: `probe_index()` ensures the bounds.
let bucket = self.get_bucket(probe_index).unwrap();
Expand Down Expand Up @@ -428,7 +430,7 @@ impl HashSet {
num_iterations: usize,
) -> Result<Option<(usize, bool)>, HashSetError> {
let mut first_free_element: Option<(usize, bool)> = None;
for i in start_iter..num_iterations {
for i in start_iter..start_iter + num_iterations {
let probe_index = self.probe_index(value, i);
let bucket = self.get_bucket(probe_index).unwrap();

Expand Down
7 changes: 1 addition & 6 deletions merkle-tree/indexed/src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,12 +290,7 @@ where
.elements
.get(usize::from(element.next_index))
.ok_or(IndexedMerkleTreeError::IndexHigherThanMax)?;
let hash = H::hashv(&[
bigint_to_be_bytes_array::<32>(&element.value)?.as_ref(),
element.next_index.to_be_bytes().as_ref(),
bigint_to_be_bytes_array::<32>(&next_element.value)?.as_ref(),
])?;
Ok(hash)
element.hash::<H>(&next_element.value)
}

/// Returns an updated low element and a new element, created based on the
Expand Down
6 changes: 2 additions & 4 deletions merkle-tree/indexed/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use light_concurrent_merkle_tree::{
};
use light_utils::bigint::bigint_to_be_bytes_array;
use num_bigint::BigUint;
use num_traits::{CheckedAdd, CheckedSub, Num, ToBytes, Unsigned};
use num_traits::{CheckedAdd, CheckedSub, ToBytes, Unsigned};

pub mod array;
pub mod changelog;
Expand Down Expand Up @@ -154,10 +154,8 @@ where
/// However, it comes with a tradeoff - the space available in the tree
/// becomes lower by 1.
pub fn add_highest_element(&mut self) -> Result<(), IndexedMerkleTreeError> {
let init_value = BigUint::from_str_radix(HIGHEST_ADDRESS_PLUS_ONE, 10).unwrap();

let mut indexed_array = IndexedArray::<H, I>::default();
let element_bundle = indexed_array.append(&init_value)?;
let element_bundle = indexed_array.init()?;
let new_low_leaf = element_bundle
.new_low_element
.hash::<H>(&element_bundle.new_element.value)?;
Expand Down

0 comments on commit 0ccb5b0

Please sign in to comment.