Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

simple_hash_from_byte_slices takes a Vec instead of slice #59

Merged
merged 7 commits into from
Nov 19, 2019
Merged

simple_hash_from_byte_slices takes a Vec instead of slice #59

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6665e8e
simple_hash_from_byte_slices takes a Vec instead of slice
liamsi Nov 8, 2019
a6fcfae
cargo fmt
liamsi Nov 8, 2019
e34f1b6
WIP: pass in Vec<Vec<u8> instead of &Vec<Vec<u8>
liamsi Nov 9, 2019
d0e5e27
use Vec<Vec<u8>> with inner recursion method
liamsi Nov 9, 2019
be377fa
simplify tests:
liamsi Nov 9, 2019
b4478b8
remove obsolete comment
liamsi Nov 9, 2019
937ba67
Rename simple_hash_from_byte_slices to simple_hash_from_byte_vectors
liamsi Nov 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions tendermint/src/merkle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,21 @@ pub const HASH_SIZE: usize = 32;
/// Hash is the output of the cryptographic digest function
pub type Hash = [u8; HASH_SIZE];

/// Compute a simple Merkle root from the arbitrary sized byte slices
pub fn simple_hash_from_byte_slices(byte_slices: &[&[u8]]) -> Hash {
/// Compute a simple Merkle root from the arbitrary byte arrays
pub fn simple_hash_from_byte_slices(byte_slices: Vec<Vec<u8>>) -> Hash {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those aren't slices 😉

Copy link
Member Author

@liamsi liamsi Nov 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to simple_hash_from_byte_vectors in 937ba67.

I think a better name for the the method would rather be compute_merkle_root_from_leaves or compute_merkle_root_from_leaf_values instead of simple_hash_from_[some_type].
This would actually say what the method does instead of what type it expects as input (which the method signature clearly states). What do you think?

The current naming probably tries to as close as possible to the go implementation.
CC @ebuchman

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right about better naming happy to see this changed.

simple_hash_from_byte_slices_inner(byte_slices.as_slice())
}

// recurse into subtrees
fn simple_hash_from_byte_slices_inner(byte_slices: &[Vec<u8>]) -> Hash {
let length = byte_slices.len();
match length {
0 => [0; HASH_SIZE],
1 => leaf_hash(byte_slices[0]),
1 => leaf_hash(byte_slices[0].as_slice()),
_ => {
let k = get_split_point(length);
let left = simple_hash_from_byte_slices(&byte_slices[..k]);
let right = simple_hash_from_byte_slices(&byte_slices[k..]);
let left = simple_hash_from_byte_slices_inner(&byte_slices[..k]);
let right = simple_hash_from_byte_slices_inner(&byte_slices[k..]);
inner_hash(&left, &right)
}
}
Expand Down Expand Up @@ -90,7 +95,8 @@ mod tests {
let empty_leaf_root_hex =
"6e340b9cffb37a989ca544e6bb780a2c78901d3fb33738768511a30617afa01d";
let empty_leaf_root = &hex::decode(empty_leaf_root_hex).unwrap();
let empty_tree: &[&[u8]] = &[&[]];
let empty_tree: Vec<Vec<u8>> = vec![vec![]; 1];

let root = simple_hash_from_byte_slices(empty_tree);
assert_eq!(empty_leaf_root, &root);
}
Expand All @@ -101,7 +107,8 @@ mod tests {
let leaf_string = "L123456";

let leaf_root = &hex::decode(leaf_root_hex).unwrap();
let leaf_tree: &[&[u8]] = &[leaf_string.as_bytes()];
let leaf_tree: Vec<Vec<u8>> = vec![leaf_string.as_bytes().to_vec(); 1];

let root = simple_hash_from_byte_slices(leaf_tree);
assert_eq!(leaf_root, &root);
}
Expand Down
8 changes: 1 addition & 7 deletions tendermint/src/validator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,12 @@ impl Set {

/// Compute the Merkle root of the validator set
pub fn hash(self) -> merkle::Hash {
// We need to get from Vec<Info> to &[&[u8]] so we can call simple_hash_from_byte_slices.
// This looks like: Vec<Info> -> Vec<Vec<u8>> -> Vec<&[u8]> -> &[&[u8]]
// Can we simplify this?
// Perhaps simple_hash_from_byteslices should take Vec<Vec<u8>> directly ?
let validator_bytes: Vec<Vec<u8>> = self
.validators
.into_iter()
.map(|x| x.hash_bytes())
.collect();
let validator_byteslices: Vec<&[u8]> =
(&validator_bytes).iter().map(|x| x.as_slice()).collect();
merkle::simple_hash_from_byte_slices(validator_byteslices.as_slice())
merkle::simple_hash_from_byte_slices(validator_bytes)
}
}

Expand Down