pmmr should not depend on switch_commit_hash to identify utxo (#576)

* pmmr does not depend on switch_commit_hash
for a given chain a commitment shold be sufficient to identify a utxo

* fix the tests

api/src/handlers.rs

@@ -119,11 +119,7 @@ impl UtxoHandler {
 		let outputs = block
 			.outputs
 			.iter()
-			.filter(|c| {
+			.filter(|c| self.chain.is_unspent(&c.commit).unwrap())
-				self.chain
-					.is_unspent(&c.commit, &c.switch_commit_hash)
-					.unwrap()
-			})
 			.map(|k| OutputSwitch::from_output(k, &header))
 			.collect();
 		BlockOutputs {

chain/src/chain.rs

@@ -24,7 +24,7 @@ use util::secp::pedersen::{Commitment, RangeProof};
 use core::core::SumCommit;
 use core::core::pmmr::{HashSum, NoSum};
 
-use core::core::{Block, BlockHeader, Output, TxKernel, SwitchCommitHash};
+use core::core::{Block, BlockHeader, Output, TxKernel};
 use core::core::target::Difficulty;
 use core::core::hash::Hash;
 use grin_store::Error::NotFoundErr;
@@ -339,7 +339,7 @@ impl Chain {
 		match self.store.get_output_by_commit(output_ref) {
 			Ok(out) => {
 				let mut sumtrees = self.sumtrees.write().unwrap();
-				if sumtrees.is_unspent(output_ref, &out.switch_commit_hash())? {
+				if sumtrees.is_unspent(output_ref)? {
 					Ok(out)
 				} else {
 					Err(Error::OutputNotFound)
@@ -351,11 +351,9 @@ impl Chain {
 	}
 
 	/// Checks whether an output is unspent
-	pub fn is_unspent(&self, output_ref: &Commitment, switch: &SwitchCommitHash)
+	pub fn is_unspent(&self, output_ref: &Commitment) -> Result<bool, Error> {
-		-> Result<bool, Error> {
-
 		let mut sumtrees = self.sumtrees.write().unwrap();
-		sumtrees.is_unspent(output_ref, switch)
+		sumtrees.is_unspent(output_ref)
 	}
 
 	/// Sets the sumtree roots on a brand new block by applying the block on the

chain/src/sumtree.rs

@@ -22,7 +22,7 @@ use std::sync::Arc;
 
 use util::secp::pedersen::{RangeProof, Commitment};
 
-use core::core::{Block, SumCommit, SwitchCommitHash, TxKernel};
+use core::core::{Block, SumCommit, TxKernel};
 use core::core::pmmr::{HashSum, NoSum, Summable, PMMR};
 use core::core::hash::Hashed;
 use grin_store;
@@ -90,9 +90,7 @@ impl SumTrees {
 	}
 
 	/// Whether a given commitment exists in the Output MMR and it's unspent
-	pub fn is_unspent(&mut self, commit: &Commitment, switch: &SwitchCommitHash)
+	pub fn is_unspent(&mut self, commit: &Commitment) -> Result<bool, Error> {
-		-> Result<bool, Error> {
-
 		let rpos = self.commit_index.get_output_pos(commit);
 		match rpos {
 			Ok(pos) => {
@@ -102,7 +100,7 @@ impl SumTrees {
 				);
 				if let Some(hs) = output_pmmr.get(pos) {
 					let hashsum = HashSum::from_summable(
-						pos, &SumCommit{commit: commit.clone()}, Some(switch));
+						pos, &SumCommit{commit: commit.clone()});
 					Ok(hs.hash == hashsum.hash)
 				} else {
 					Ok(false)
@@ -273,11 +271,10 @@ impl<'a> Extension<'a> {
 				// note that this doesn't show the commitment *never* existed, just
 				// that this is not an existing unspent commitment right now
 				if let Some(c) = self.output_pmmr.get(pos) {
-					let hashsum = HashSum::from_summable(
+					let hashsum = HashSum::from_summable(pos, &SumCommit{commit});
-						pos, &SumCommit{commit}, Some(out.switch_commit_hash));
+					// processing a new fork so we may get a position on the old
-					// as we're processing a new fork, we may get a position on the old
+					// fork that exists but matches a different node
-					// fork that exists but matches a different node, filtering that
+					// filtering that case out
-					// case out
 					if c.hash == hashsum.hash {
 						return Err(Error::DuplicateCommitment(commit));
 					}
@@ -289,7 +286,6 @@ impl<'a> Extension<'a> {
 					SumCommit {
 						commit: out.commitment(),
 					},
-					Some(out.switch_commit_hash()),
 				)
 				.map_err(&Error::SumTreeErr)?;
 
@@ -297,7 +293,7 @@ impl<'a> Extension<'a> {
 
 			// push range proofs in their MMR
 			self.rproof_pmmr
-				.push(NoSum(out.proof), None::<RangeProof>)
+				.push(NoSum(out.proof))
 				.map_err(&Error::SumTreeErr)?;
 		}
 
@@ -305,8 +301,7 @@ impl<'a> Extension<'a> {
 			if let Ok(pos) = self.get_kernel_pos(&kernel.excess) {
 				// same as outputs
 				if let Some(k) = self.kernel_pmmr.get(pos) {
-					let hashsum = HashSum::from_summable(
+					let hashsum = HashSum::from_summable(pos, &NoSum(kernel));
-						pos, &NoSum(kernel), None::<RangeProof>);
 					if k.hash == hashsum.hash {
 						return Err(Error::DuplicateKernel(kernel.excess.clone()));
 					}
@@ -314,7 +309,7 @@ impl<'a> Extension<'a> {
 			}
 			// push kernels in their MMR
 			let pos = self.kernel_pmmr
-				.push(NoSum(kernel.clone()), None::<RangeProof>)
+				.push(NoSum(kernel.clone()))
 				.map_err(&Error::SumTreeErr)?;
 			self.new_kernel_excesses.insert(kernel.excess, pos);
 		}

core/src/core/pmmr.rs

@@ -119,11 +119,8 @@ where
 	T: Summable + Hashed,
 {
 	/// Create a hash sum from a summable
-	pub fn from_summable<W: Writeable>(idx: u64, elmt: &T, hash_with: Option<W>) -> HashSum<T> {
+	pub fn from_summable(idx: u64, elmt: &T) -> HashSum<T> {
-		let hash = match hash_with {
+		let hash = elmt.hash();
-			Some(h) => elmt.hash_with(h),
-			None => elmt.hash(),
-		};
 		let sum = elmt.sum();
 		let node_hash = (idx, &sum, hash).hash();
 		HashSum {
@@ -259,9 +256,9 @@ where
 
 	/// Push a new Summable element in the MMR. Computes new related peaks at
 	/// the same time if applicable.
-	pub fn push<W: Writeable>(&mut self, elmt: T, hash_with: Option<W>) -> Result<u64, String> {
+	pub fn push(&mut self, elmt: T) -> Result<u64, String> {
 		let elmt_pos = self.last_pos + 1;
-		let mut current_hashsum = HashSum::from_summable(elmt_pos, &elmt, hash_with);
+		let mut current_hashsum = HashSum::from_summable(elmt_pos, &elmt);
 		let mut to_append = vec![current_hashsum.clone()];
 		let mut height = 0;
 		let mut pos = elmt_pos;
@@ -884,7 +881,7 @@ mod test {
 		let mut pmmr = PMMR::new(&mut ba);
 
 		// one element
-		pmmr.push(elems[0], None::<TestElem>).unwrap();
+		pmmr.push(elems[0]).unwrap();
 		let hash = Hashed::hash(&elems[0]);
 		let sum = elems[0].sum();
 		let node_hash = (1 as u64, &sum, hash).hash();
@@ -898,59 +895,59 @@ mod test {
 		assert_eq!(pmmr.unpruned_size(), 1);
 
 		// two elements
-		pmmr.push(elems[1], None::<TestElem>).unwrap();
+		pmmr.push(elems[1]).unwrap();
-		let sum2 = HashSum::from_summable(1, &elems[0], None::<TestElem>) +
+		let sum2 = HashSum::from_summable(1, &elems[0]) +
-			HashSum::from_summable(2, &elems[1], None::<TestElem>);
+			HashSum::from_summable(2, &elems[1]);
 		assert_eq!(pmmr.root(), sum2);
 		assert_eq!(pmmr.unpruned_size(), 3);
 
 		// three elements
-		pmmr.push(elems[2], None::<TestElem>).unwrap();
+		pmmr.push(elems[2]).unwrap();
-		let sum3 = sum2.clone() + HashSum::from_summable(4, &elems[2], None::<TestElem>);
+		let sum3 = sum2.clone() + HashSum::from_summable(4, &elems[2]);
 		assert_eq!(pmmr.root(), sum3);
 		assert_eq!(pmmr.unpruned_size(), 4);
 
 		// four elements
-		pmmr.push(elems[3], None::<TestElem>).unwrap();
+		pmmr.push(elems[3]).unwrap();
 		let sum4 = sum2 +
-			(HashSum::from_summable(4, &elems[2], None::<TestElem>) +
+			(HashSum::from_summable(4, &elems[2]) +
-				 HashSum::from_summable(5, &elems[3], None::<TestElem>));
+				 HashSum::from_summable(5, &elems[3]));
 		assert_eq!(pmmr.root(), sum4);
 		assert_eq!(pmmr.unpruned_size(), 7);
 
 		// five elements
-		pmmr.push(elems[4], None::<TestElem>).unwrap();
+		pmmr.push(elems[4]).unwrap();
-		let sum5 = sum4.clone() + HashSum::from_summable(8, &elems[4], None::<TestElem>);
+		let sum5 = sum4.clone() + HashSum::from_summable(8, &elems[4]);
 		assert_eq!(pmmr.root(), sum5);
 		assert_eq!(pmmr.unpruned_size(), 8);
 
 		// six elements
-		pmmr.push(elems[5], None::<TestElem>).unwrap();
+		pmmr.push(elems[5]).unwrap();
 		let sum6 = sum4.clone() +
-			(HashSum::from_summable(8, &elems[4], None::<TestElem>) +
+			(HashSum::from_summable(8, &elems[4]) +
-				 HashSum::from_summable(9, &elems[5], None::<TestElem>));
+				 HashSum::from_summable(9, &elems[5]));
 		assert_eq!(pmmr.root(), sum6.clone());
 		assert_eq!(pmmr.unpruned_size(), 10);
 
 		// seven elements
-		pmmr.push(elems[6], None::<TestElem>).unwrap();
+		pmmr.push(elems[6]).unwrap();
-		let sum7 = sum6 + HashSum::from_summable(11, &elems[6], None::<TestElem>);
+		let sum7 = sum6 + HashSum::from_summable(11, &elems[6]);
 		assert_eq!(pmmr.root(), sum7);
 		assert_eq!(pmmr.unpruned_size(), 11);
 
 		// eight elements
-		pmmr.push(elems[7], None::<TestElem>).unwrap();
+		pmmr.push(elems[7]).unwrap();
 		let sum8 = sum4 +
-			((HashSum::from_summable(8, &elems[4], None::<TestElem>) +
+			((HashSum::from_summable(8, &elems[4]) +
-				  HashSum::from_summable(9, &elems[5], None::<TestElem>)) +
+				  HashSum::from_summable(9, &elems[5])) +
-				 (HashSum::from_summable(11, &elems[6], None::<TestElem>) +
+				 (HashSum::from_summable(11, &elems[6]) +
-					  HashSum::from_summable(12, &elems[7], None::<TestElem>)));
+					  HashSum::from_summable(12, &elems[7])));
 		assert_eq!(pmmr.root(), sum8);
 		assert_eq!(pmmr.unpruned_size(), 15);
 
 		// nine elements
-		pmmr.push(elems[8], None::<TestElem>).unwrap();
+		pmmr.push(elems[8]).unwrap();
-		let sum9 = sum8 + HashSum::from_summable(16, &elems[8], None::<TestElem>);
+		let sum9 = sum8 + HashSum::from_summable(16, &elems[8]);
 		assert_eq!(pmmr.root(), sum9);
 		assert_eq!(pmmr.unpruned_size(), 16);
 	}
@@ -975,31 +972,31 @@ mod test {
 		let res = pmmr.get_last_n_insertions(19);
 		assert!(res.len() == 0);
 
-		pmmr.push(elems[0], None::<TestElem>).unwrap();
+		pmmr.push(elems[0]).unwrap();
 		let res = pmmr.get_last_n_insertions(19);
 		assert!(res.len() == 1 && res[0].sum == 1);
 
-		pmmr.push(elems[1], None::<TestElem>).unwrap();
+		pmmr.push(elems[1]).unwrap();
 
 		let res = pmmr.get_last_n_insertions(12);
 		assert!(res[0].sum == 2 && res[1].sum == 1);
 
-		pmmr.push(elems[2], None::<TestElem>).unwrap();
+		pmmr.push(elems[2]).unwrap();
 
 		let res = pmmr.get_last_n_insertions(2);
 		assert!(res[0].sum == 3 && res[1].sum == 2);
 
-		pmmr.push(elems[3], None::<TestElem>).unwrap();
+		pmmr.push(elems[3]).unwrap();
 
 		let res = pmmr.get_last_n_insertions(19);
 		assert!(
 			res[0].sum == 4 && res[1].sum == 3 && res[2].sum == 2 && res[3].sum == 1 && res.len() == 4
 		);
 
-		pmmr.push(elems[5], None::<TestElem>).unwrap();
+		pmmr.push(elems[5]).unwrap();
-		pmmr.push(elems[6], None::<TestElem>).unwrap();
+		pmmr.push(elems[6]).unwrap();
-		pmmr.push(elems[7], None::<TestElem>).unwrap();
+		pmmr.push(elems[7]).unwrap();
-		pmmr.push(elems[8], None::<TestElem>).unwrap();
+		pmmr.push(elems[8]).unwrap();
 
 		let res = pmmr.get_last_n_insertions(7);
 		assert!(
@@ -1028,7 +1025,7 @@ mod test {
 		{
 			let mut pmmr = PMMR::new(&mut ba);
 			for elem in &elems[..] {
-				pmmr.push(*elem, None::<TestElem>).unwrap();
+				pmmr.push(*elem).unwrap();
 			}
 			orig_root = pmmr.root();
 			sz = pmmr.unpruned_size();

store/tests/sumtree.rs

@@ -48,17 +48,17 @@ fn sumtree_append() {
 		})
 	);
 
-	let sum2 = HashSum::from_summable(1, &elems[0], None::<TestElem>)
+	let sum2 = HashSum::from_summable(1, &elems[0])
-		+ HashSum::from_summable(2, &elems[1], None::<TestElem>);
+		+ HashSum::from_summable(2, &elems[1]);
 	let sum4 = sum2
-		+ (HashSum::from_summable(4, &elems[2], None::<TestElem>)
+		+ (HashSum::from_summable(4, &elems[2])
-			+ HashSum::from_summable(5, &elems[3], None::<TestElem>));
+			+ HashSum::from_summable(5, &elems[3]));
 	let sum8 = sum4
-		+ ((HashSum::from_summable(8, &elems[4], None::<TestElem>)
+		+ ((HashSum::from_summable(8, &elems[4])
-			+ HashSum::from_summable(9, &elems[5], None::<TestElem>))
+			+ HashSum::from_summable(9, &elems[5]))
-			+ (HashSum::from_summable(11, &elems[6], None::<TestElem>)
+			+ (HashSum::from_summable(11, &elems[6])
-				+ HashSum::from_summable(12, &elems[7], None::<TestElem>)));
+				+ HashSum::from_summable(12, &elems[7])));
-	let sum9 = sum8 + HashSum::from_summable(16, &elems[8], None::<TestElem>);
+	let sum9 = sum8 + HashSum::from_summable(16, &elems[8]);
 
 	{
 		let pmmr = PMMR::at(&mut backend, mmr_size);
@@ -233,7 +233,7 @@ fn setup() -> (String, Vec<TestElem>) {
 fn load(pos: u64, elems: &[TestElem], backend: &mut store::sumtree::PMMRBackend<TestElem>) -> u64 {
 	let mut pmmr = PMMR::at(backend, pos);
 	for elem in elems {
-		pmmr.push(elem.clone(), None::<TestElem>).unwrap();
+		pmmr.push(elem.clone()).unwrap();
 	}
 	pmmr.unpruned_size()
 }