Skip to content

Commit

Permalink
Split pmmr.get() into get_hash() and get_data() (#855)
Browse files Browse the repository at this point in the history
  • Loading branch information
@antiochp @ignopeverell
antiochp authored and ignopeverell committed Mar 23, 2018
1 parent 05d2b1d commit 7a8d614
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 120 deletions.
6 changes: 3 additions & 3 deletions chain/src/chain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -627,19 +627,19 @@ impl Chain {
}

/// returns the last n nodes inserted into the output sum tree
pub fn get_last_n_output(&self, distance: u64) -> Vec<(Hash, Option<OutputIdentifier>)> {
pub fn get_last_n_output(&self, distance: u64) -> Vec<(Hash, OutputIdentifier)> {
let mut txhashset = self.txhashset.write().unwrap();
txhashset.last_n_output(distance)
}

/// as above, for rangeproofs
pub fn get_last_n_rangeproof(&self, distance: u64) -> Vec<(Hash, Option<RangeProof>)> {
pub fn get_last_n_rangeproof(&self, distance: u64) -> Vec<(Hash, RangeProof)> {
let mut txhashset = self.txhashset.write().unwrap();
txhashset.last_n_rangeproof(distance)
}

/// as above, for kernels
pub fn get_last_n_kernel(&self, distance: u64) -> Vec<(Hash, Option<TxKernel>)> {
pub fn get_last_n_kernel(&self, distance: u64) -> Vec<(Hash, TxKernel)> {
let mut txhashset = self.txhashset.write().unwrap();
txhashset.last_n_kernel(distance)
}
Expand Down
27 changes: 14 additions & 13 deletions chain/src/txhashset.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ impl TxHashSet {
Ok(pos) => {
let output_pmmr: PMMR<OutputIdentifier, _> =
PMMR::at(&mut self.output_pmmr_h.backend, self.output_pmmr_h.last_pos);
if let Some((hash, _)) = output_pmmr.get(pos, false) {
if let Some(hash) = output_pmmr.get_hash(pos) {
if hash == output_id.hash_with_index(pos) {
Ok(hash)
} else {
Expand All @@ -163,21 +163,21 @@ impl TxHashSet {
/// returns the last N nodes inserted into the tree (i.e. the 'bottom'
/// nodes at level 0
/// TODO: These need to return the actual data from the flat-files instead of hashes now
pub fn last_n_output(&mut self, distance: u64) -> Vec<(Hash, Option<OutputIdentifier>)> {
pub fn last_n_output(&mut self, distance: u64) -> Vec<(Hash, OutputIdentifier)> {
let output_pmmr: PMMR<OutputIdentifier, _> =
PMMR::at(&mut self.output_pmmr_h.backend, self.output_pmmr_h.last_pos);
output_pmmr.get_last_n_insertions(distance)
}

/// as above, for range proofs
pub fn last_n_rangeproof(&mut self, distance: u64) -> Vec<(Hash, Option<RangeProof>)> {
pub fn last_n_rangeproof(&mut self, distance: u64) -> Vec<(Hash, RangeProof)> {
let rproof_pmmr: PMMR<RangeProof, _> =
PMMR::at(&mut self.rproof_pmmr_h.backend, self.rproof_pmmr_h.last_pos);
rproof_pmmr.get_last_n_insertions(distance)
}

/// as above, for kernels
pub fn last_n_kernel(&mut self, distance: u64) -> Vec<(Hash, Option<TxKernel>)> {
pub fn last_n_kernel(&mut self, distance: u64) -> Vec<(Hash, TxKernel)> {
let kernel_pmmr: PMMR<TxKernel, _> =
PMMR::at(&mut self.kernel_pmmr_h.backend, self.kernel_pmmr_h.last_pos);
kernel_pmmr.get_last_n_insertions(distance)
Expand Down Expand Up @@ -387,11 +387,13 @@ impl<'a> Extension<'a> {
let pos_res = self.get_output_pos(&commit);
if let Ok(pos) = pos_res {
let output_id_hash = OutputIdentifier::from_input(input).hash_with_index(pos);
if let Some((read_hash, read_elem)) = self.output_pmmr.get(pos, true) {
if let Some(read_hash) = self.output_pmmr.get_hash(pos) {
// check hash from pmmr matches hash from input (or corresponding output)
// if not then the input is not being honest about
// what it is attempting to spend...

let read_elem = self.output_pmmr.get_data(pos);

if output_id_hash != read_hash
|| output_id_hash
!= read_elem
Expand Down Expand Up @@ -435,7 +437,7 @@ impl<'a> Extension<'a> {
// (non-historical node will have a much smaller one)
// note that this doesn't show the commitment *never* existed, just
// that this is not an existing unspent commitment right now
if let Some((hash, _)) = self.output_pmmr.get(pos, false) {
if let Some(hash) = self.output_pmmr.get_hash(pos) {
// processing a new fork so we may get a position on the old
// fork that exists but matches a different node
// filtering that case out
Expand Down Expand Up @@ -635,9 +637,8 @@ impl<'a> Extension<'a> {
for n in 1..self.output_pmmr.unpruned_size() + 1 {
// non-pruned leaves only
if pmmr::bintree_postorder_height(n) == 0 {
if let Some((_, out)) = self.output_pmmr.get(n, true) {
self.commit_index
.save_output_pos(&out.expect("not a leaf node").commit, n)?;
if let Some(out) = self.output_pmmr.get_data(n) {
self.commit_index.save_output_pos(&out.commit, n)?;
}
}
}
Expand Down Expand Up @@ -708,7 +709,7 @@ impl<'a> Extension<'a> {

for n in 1..self.kernel_pmmr.unpruned_size() + 1 {
if pmmr::is_leaf(n) {
if let Some((_, Some(kernel))) = self.kernel_pmmr.get(n, true) {
if let Some(kernel) = self.kernel_pmmr.get_data(n) {
kernel.verify()?;
commitments.push(kernel.excess.clone());
}
Expand Down Expand Up @@ -736,8 +737,8 @@ impl<'a> Extension<'a> {
let mut proof_count = 0;
for n in 1..self.output_pmmr.unpruned_size() + 1 {
if pmmr::is_leaf(n) {
if let Some((_, Some(out))) = self.output_pmmr.get(n, true) {
if let Some((_, Some(rp))) = self.rproof_pmmr.get(n, true) {
if let Some(out) = self.output_pmmr.get_data(n) {
if let Some(rp) = self.rproof_pmmr.get_data(n) {
out.to_output(rp).verify_proof()?;
} else {
// TODO - rangeproof not found
Expand All @@ -764,7 +765,7 @@ impl<'a> Extension<'a> {
let mut commitments = vec![];
for n in 1..self.output_pmmr.unpruned_size() + 1 {
if pmmr::is_leaf(n) {
if let Some((_, Some(out))) = self.output_pmmr.get(n, true) {
if let Some(out) = self.output_pmmr.get_data(n) {
commitments.push(out.commit.clone());
}
}
Expand Down
93 changes: 63 additions & 30 deletions core/src/core/pmmr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,18 @@ where
/// occurred (see remove).
fn rewind(&mut self, position: u64, index: u32) -> Result<(), String>;

/// Get a Hash by insertion position. If include_data is true, will
/// also return the associated data element
fn get(&self, position: u64, include_data: bool) -> Option<(Hash, Option<T>)>;
/// Get a Hash by insertion position.
fn get_hash(&self, position: u64) -> Option<Hash>;

/// Get a Hash by original insertion position (ignoring the remove
/// list).
/// Get underlying data by insertion position.
fn get_data(&self, position: u64) -> Option<T>;

/// Get a Hash by original insertion position
/// (ignoring the remove log).
fn get_from_file(&self, position: u64) -> Option<Hash>;

/// Get a Data Element by original insertion position (ignoring the remove
/// list).
/// Get a Data Element by original insertion position
/// (ignoring the remove log).
fn get_data_from_file(&self, position: u64) -> Option<T>;

/// Remove HashSums by insertion position. An index is also provided so the
Expand Down Expand Up @@ -327,9 +329,8 @@ where

let root = self.root();

let node = self.get(pos, false)
.ok_or(format!("no element at pos {}", pos))?
.0;
let node = self.get_hash(pos)
.ok_or(format!("no element at pos {}", pos))?;

let family_branch = family_branch(pos, self.last_pos);

Expand Down Expand Up @@ -413,7 +414,7 @@ where
/// to keep an index of elements to positions in the tree. Prunes parent
/// nodes as well when they become childless.
pub fn prune(&mut self, position: u64, index: u32) -> Result<bool, String> {
if let None = self.backend.get(position, false) {
if let None = self.backend.get_hash(position) {
return Ok(false);
}
let prunable_height = bintree_postorder_height(position);
Expand All @@ -439,7 +440,7 @@ where

// if we have a pruned sibling, we can continue up the tree
// otherwise we're done
if let None = self.backend.get(sibling, false) {
if let None = self.backend.get_hash(sibling) {
current = parent;
} else {
break;
Expand All @@ -450,34 +451,47 @@ where
Ok(true)
}

/// Helper function to get a node at a given position from
/// the backend.
pub fn get(&self, position: u64, include_data: bool) -> Option<(Hash, Option<T>)> {
if position > self.last_pos {
/// Get a hash at provided position in the MMR.
pub fn get_hash(&self, pos: u64) -> Option<Hash> {
if pos > self.last_pos {
None
} else {
self.backend.get_hash(pos)
}
}

/// Get the data element at provided in the MMR.
pub fn get_data(&self, pos: u64) -> Option<T> {
if pos > self.last_pos {
None
} else {
self.backend.get(position, include_data)
self.backend.get_data(pos)
}
}

fn get_from_file(&self, position: u64) -> Option<Hash> {
if position > self.last_pos {
/// Get the hash from the underlying MMR file
/// (ignores the remove log).
fn get_from_file(&self, pos: u64) -> Option<Hash> {
if pos > self.last_pos {
None
} else {
self.backend.get_from_file(position)
self.backend.get_from_file(pos)
}
}

/// Helper function to get the last N nodes inserted, i.e. the last
/// n nodes along the bottom of the tree
pub fn get_last_n_insertions(&self, n: u64) -> Vec<(Hash, Option<T>)> {
let mut return_vec = Vec::new();
pub fn get_last_n_insertions(&self, n: u64) -> Vec<(Hash, T)> {
let mut return_vec = vec![];
let mut last_leaf = self.last_pos;
let size = self.unpruned_size();
// Special case that causes issues in bintree functions,
// just return
if size == 1 {
return_vec.push(self.backend.get(last_leaf, true).unwrap());
return_vec.push((
self.backend.get_hash(last_leaf).unwrap(),
self.backend.get_data(last_leaf).unwrap(),
));
return return_vec;
}
// if size is even, we're already at the bottom, otherwise
Expand All @@ -492,7 +506,10 @@ where
if bintree_postorder_height(last_leaf) > 0 {
last_leaf = bintree_rightmost(last_leaf);
}
return_vec.push(self.backend.get(last_leaf, true).unwrap());
return_vec.push((
self.backend.get_hash(last_leaf).unwrap(),
self.backend.get_data(last_leaf).unwrap(),
));

last_leaf = bintree_jump_left_sibling(last_leaf);
}
Expand All @@ -504,7 +521,7 @@ where
// iterate on all parent nodes
for n in 1..(self.last_pos + 1) {
if bintree_postorder_height(n) > 0 {
if let Some(hs) = self.get(n, false) {
if let Some(hash) = self.get_hash(n) {
// take the left and right children, if they exist
let left_pos = bintree_move_down_left(n).ok_or(format!("left_pos not found"))?;
let right_pos = bintree_jump_right_sibling(left_pos);
Expand All @@ -514,7 +531,7 @@ where
if let Some(right_child_hs) = self.get_from_file(right_pos) {
// hash the two child nodes together with parent_pos and compare
let (parent_pos, _) = family(left_pos);
if (left_child_hs, right_child_hs).hash_with_index(parent_pos) != hs.0 {
if (left_child_hs, right_child_hs).hash_with_index(parent_pos) != hash {
return Err(format!(
"Invalid MMR, hash of parent at {} does \
not match children.",
Expand Down Expand Up @@ -556,9 +573,9 @@ where
break;
}
idx.push_str(&format!("{:>8} ", m + 1));
let ohs = self.get(m + 1, false);
let ohs = self.get_hash(m + 1);
match ohs {
Some(hs) => hashes.push_str(&format!("{} ", hs.0)),
Some(hs) => hashes.push_str(&format!("{} ", hs)),
None => hashes.push_str(&format!("{:>8} ", "??")),
}
}
Expand Down Expand Up @@ -1015,11 +1032,27 @@ mod test {
Ok(())
}

fn get(&self, position: u64, _include_data: bool) -> Option<(Hash, Option<T>)> {
fn get_hash(&self, position: u64) -> Option<Hash> {
if self.remove_list.contains(&position) {
None
} else {
self.elems[(position - 1) as usize].clone()
if let Some(ref elem) = self.elems[(position - 1) as usize] {
Some(elem.0)
} else {
None
}
}
}

fn get_data(&self, position: u64) -> Option<T> {
if self.remove_list.contains(&position) {
None
} else {
if let Some(ref elem) = self.elems[(position - 1) as usize] {
elem.1.clone()
} else {
None
}
}
}

Expand Down
33 changes: 17 additions & 16 deletions store/src/pmmr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,26 +161,27 @@ where
}
}

/// Get a Hash by insertion position
fn get(&self, position: u64, include_data: bool) -> Option<(Hash, Option<T>)> {
/// Get the hash at pos.
/// Return None if it has been removed.
fn get_hash(&self, pos: u64) -> Option<(Hash)> {
// Check if this position has been pruned in the remove log...
if self.rm_log.includes(position) {
return None;
}

let hash_val = self.get_from_file(position);
if !include_data {
return hash_val.map(|hash| (hash, None));
if self.rm_log.includes(pos) {
None
} else {
self.get_from_file(pos)
}
}

// if this is not a leaf then we have no data
if !pmmr::is_leaf(position) {
return hash_val.map(|hash| (hash, None));
/// Get the data at pos.
/// Return None if it has been removed or if pos is not a leaf node.
fn get_data(&self, pos: u64) -> Option<(T)> {
if self.rm_log.includes(pos) {
None
} else if !pmmr::is_leaf(pos) {
None
} else {
self.get_data_from_file(pos)
}

let data = self.get_data_from_file(position);

hash_val.map(|x| (x, data))
}

fn rewind(&mut self, position: u64, index: u32) -> Result<(), String> {
Expand Down
Loading

0 comments on commit 7a8d614

Please sign in to comment.