Skip to content

Commit

Permalink
[accumulator] Fix accumulator get_leaves return order and append result.
Browse files Browse the repository at this point in the history 
1. Accumulator get_leaves return reverse order when get with reverse = true.
2. Remove leaf_index from Accumulator append return result, only return root_hash.

accumulator] Fix accumulator append result.
  • Loading branch information
@jolestar
jolestar committed Nov 3, 2020
1 parent b3a93d1 commit 6e8c239
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 47 deletions.
2 changes: 1 addition & 1 deletion chain/open-block/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ impl OpenedBlock {
gas_used,
status,
);
let (accumulator_root, _) = self.txn_accumulator.append(&[txn_info.id()])?;
let accumulator_root = self.txn_accumulator.append(&[txn_info.id()])?;
Ok((txn_state_root, accumulator_root))
}

Expand Down
4 changes: 1 addition & 3 deletions chain/src/chain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -945,9 +945,7 @@ impl BlockChain {
let executed_accumulator_root = {
let included_txn_info_hashes: Vec<_> =
vec_transaction_info.iter().map(|info| info.id()).collect();
let (accumulator_root, _first_leaf_idx) =
self.txn_accumulator.append(&included_txn_info_hashes)?;
accumulator_root
self.txn_accumulator.append(&included_txn_info_hashes)?
};

verify_block!(
Expand Down
3 changes: 1 addition & 2 deletions chain/src/tests/block_test_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,7 @@ fn gen_root_hashes(
.iter()
.map(|info| info.id())
.collect();
let (accumulator_root, _first_leaf_idx) =
txn_accumulator.append(&included_txn_info_hashes).unwrap();
let accumulator_root = txn_accumulator.append(&included_txn_info_hashes).unwrap();
(accumulator_root, executed_data.state_root)
}
// Err(err) => {
Expand Down
51 changes: 24 additions & 27 deletions core/accumulator/src/accumulator_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,19 @@ fn test_get_leaves() {
0,
Arc::new(mock_store),
);
let (root_hash, index) = accumulator.append(leaves.as_slice()).unwrap();
let root_hash = accumulator.append(leaves.as_slice()).unwrap();
let new_num_leaves = accumulator.num_leaves();
dbg!(root_hash);
let mut i = index;
let len = leaves.len() as u64;
let begin = SystemTime::now();
loop {
if i < len {
let _leaf = accumulator.get_leaf(i).unwrap().unwrap();
} else {
break;
}
i += 1;
}
(0..new_num_leaves).for_each(|idx| {
let _leaf = accumulator.get_leaf(idx).unwrap().unwrap();
});
let use_time = SystemTime::now().duration_since(begin).unwrap();
println!(
"test accumulator get leaves, leaves count: {:?} use time: {:?} average time:{:?}",
len,
new_num_leaves,
use_time,
use_time.as_nanos() / len as u128,
use_time.as_nanos() / new_num_leaves as u128,
);
}

Expand Down Expand Up @@ -102,7 +96,7 @@ fn test_multiple_chain() {
0,
mock_store.clone(),
);
let (root_hash, _index1) = accumulator.append(&leaves).unwrap();
let root_hash = accumulator.append(&leaves).unwrap();
accumulator.flush().unwrap();
proof_verify(&accumulator, root_hash, &leaves, 0);
let frozen_node = accumulator.get_frozen_subtree_roots();
Expand All @@ -123,9 +117,9 @@ fn test_multiple_chain() {
let leaves2 = create_leaves(54..58);
let leaves3 = create_leaves(60..64);

let (_root_hash2, _) = accumulator.append(&leaves2).unwrap();
let _root_hash2 = accumulator.append(&leaves2).unwrap();
accumulator.flush().unwrap();
let (_root_hash3, _) = accumulator2.append(&leaves3).unwrap();
let _root_hash3 = accumulator2.append(&leaves3).unwrap();
accumulator2.flush().unwrap();
assert_eq!(
accumulator.get_node_by_position(1).unwrap().unwrap(),
Expand All @@ -150,11 +144,11 @@ fn test_one_leaf() {
0,
Arc::new(mock_store),
);
let (root_hash, _) = accumulator.append(&[hash]).unwrap();
let root_hash = accumulator.append(&[hash]).unwrap();
assert_eq!(hash, root_hash);
proof_verify(&accumulator, root_hash, &[hash], 0);
let new_hash = HashValue::random();
let (new_root_hash, _) = accumulator.append(&[new_hash]).unwrap();
let new_root_hash = accumulator.append(&[new_hash]).unwrap();
proof_verify(&accumulator, new_root_hash, &[new_hash], 1);
let vec = vec![hash, new_hash];
proof_verify(&accumulator, new_root_hash, &vec, 0);
Expand All @@ -171,7 +165,7 @@ fn test_proof() {
Arc::new(mock_store),
);
let batch1 = create_leaves(500..600);
let (root_hash1, _) = accumulator.append(&batch1).unwrap();
let root_hash1 = accumulator.append(&batch1).unwrap();
accumulator.flush().unwrap();
proof_verify(&accumulator, root_hash1, &batch1, 0);
}
Expand All @@ -187,10 +181,10 @@ fn test_multiple_leaves() {
0,
Arc::new(mock_store),
);
let (root_hash1, _) = accumulator.append(&batch1).unwrap();
let root_hash1 = accumulator.append(&batch1).unwrap();
proof_verify(&accumulator, root_hash1, &batch1, 0);
let batch2 = create_leaves(609..613);
let (root_hash2, _) = accumulator.append(&batch2).unwrap();
let root_hash2 = accumulator.append(&batch2).unwrap();
batch1.extend_from_slice(&batch2);
proof_verify(&accumulator, root_hash2, &batch1, 0);
}
Expand All @@ -207,7 +201,7 @@ fn test_multiple_tree() {
0,
arc_store.clone(),
);
let (root_hash1, _) = accumulator.append(&batch1).unwrap();
let root_hash1 = accumulator.append(&batch1).unwrap();
accumulator.flush().unwrap();
proof_verify(&accumulator, root_hash1, &batch1, 0);
let frozen_hash = accumulator.get_frozen_subtree_roots();
Expand All @@ -229,7 +223,7 @@ fn test_update_left_leaf() {
0,
Arc::new(mock_store),
);
let (root_hash, _) = accumulator.append(&leaves).unwrap();
let root_hash = accumulator.append(&leaves).unwrap();
proof_verify(&accumulator, root_hash, &leaves, 0);
}
#[test]
Expand All @@ -244,7 +238,7 @@ fn test_update_right_leaf() {
0,
Arc::new(mock_store),
);
let (root_hash, _) = accumulator.append(&leaves).unwrap();
let root_hash = accumulator.append(&leaves).unwrap();
proof_verify(&accumulator, root_hash, &leaves, 0);
}
#[test]
Expand All @@ -258,7 +252,7 @@ fn test_flush() {
0,
Arc::new(mock_store),
);
let (_root_hash, _) = accumulator.append(&leaves).unwrap();
let _root_hash = accumulator.append(&leaves).unwrap();
accumulator.flush().unwrap();
//get from storage
for node_hash in leaves.clone() {
Expand All @@ -278,7 +272,7 @@ fn test_get_leaves_batch() {
Arc::new(mock_store),
);
let leaves: Vec<HashValue> = (0..100).map(|_| HashValue::random()).collect();
let (_root_hash, _) = accumulator.append(leaves.as_slice()).unwrap();
let _root_hash = accumulator.append(leaves.as_slice()).unwrap();
accumulator.flush().unwrap();

let leaves0 = accumulator.get_leaves(0, false, 100).unwrap();
Expand All @@ -287,7 +281,10 @@ fn test_get_leaves_batch() {

let leaves1 = accumulator.get_leaves(5, true, 100).unwrap();
assert_eq!(leaves1.len(), 6);
assert_eq!(&leaves[0..6], leaves1.as_slice());
assert_eq!(
(0..6usize).rev().map(|i| leaves[i]).collect::<Vec<_>>(),
leaves1
);

let leaves2 = accumulator.get_leaves(5, false, 90).unwrap();
assert_eq!(leaves2.len(), 90);
Expand Down
35 changes: 22 additions & 13 deletions core/accumulator/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub const MAC_CACHE_SIZE: usize = 65535;
/// accumulator method define
pub trait Accumulator {
/// Append leaves and return new root
fn append(&self, leaves: &[HashValue]) -> Result<(HashValue, u64)>;
fn append(&self, leaves: &[HashValue]) -> Result<HashValue>;
/// Get leaf node by index.
fn get_leaf(&self, leaf_index: u64) -> Result<Option<HashValue>>;
/// Batch get leaves by index.
Expand Down Expand Up @@ -109,11 +109,10 @@ impl MerkleAccumulator {
}

impl Accumulator for MerkleAccumulator {
fn append(&self, new_leaves: &[HashValue]) -> Result<(HashValue, u64)> {
fn append(&self, new_leaves: &[HashValue]) -> Result<HashValue> {
let mut tree_guard = self.tree.lock();
let first_index_leaf = tree_guard.num_leaves;
let root_hash = tree_guard.append(new_leaves)?;
Ok((root_hash, first_index_leaf))
Ok(root_hash)
}

fn get_leaf(&self, leaf_index: u64) -> Result<Option<HashValue>> {
Expand All @@ -130,26 +129,36 @@ impl Accumulator for MerkleAccumulator {
) -> Result<Vec<HashValue>> {
let mut tree = self.tree.lock();
let max_size_u64 = max_size as u64;
let seq = if reverse {
if reverse {
let end = start_index + 1;
let begin = if end > max_size_u64 {
end - max_size_u64
} else {
0
};
begin..end
(begin..end)
.rev()
.map(|idx| {
tree.get_node_hash(NodeIndex::from_leaf_index(idx))?
.ok_or_else(|| {
format_err!("Can not find accumulator leaf by index: {}", idx)
})
})
.collect()
} else {
let mut end = start_index + max_size_u64;
if end > tree.num_leaves {
end = tree.num_leaves;
}
start_index..end
};
seq.map(|idx| {
tree.get_node_hash(NodeIndex::from_leaf_index(idx))?
.ok_or_else(|| format_err!("Can not find accumulator leaf by index: {}", idx))
})
.collect()
(start_index..end)
.map(|idx| {
tree.get_node_hash(NodeIndex::from_leaf_index(idx))?
.ok_or_else(|| {
format_err!("Can not find accumulator leaf by index: {}", idx)
})
})
.collect()
}
}

fn get_node_by_position(&self, position: u64) -> Result<Option<HashValue>> {
Expand Down
2 changes: 1 addition & 1 deletion core/genesis/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl Genesis {
);
let txn_info_hash = transaction_info.id();

let (accumulator_root, _) = accumulator.append(vec![txn_info_hash].as_slice())?;
let accumulator_root = accumulator.append(vec![txn_info_hash].as_slice())?;
accumulator.flush()?;
Ok(Block::genesis_block(
genesis_config.parent_hash,
Expand Down

0 comments on commit 6e8c239

Please sign in to comment.