Fix is_unspent to consider MMR and not only index

The method `is_unspent` hadn't been fixed with the other sumtree
functions to check the MMR before deciding whether something is
really unspent. This is now fixed and also checks the output
hash is the one we expect.

api/src/handlers.rs

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

chain/src/chain.rs

@@ -23,7 +23,7 @@ use util::secp::pedersen::{Commitment, RangeProof};
 use core::core::SumCommit;
 use core::core::pmmr::{HashSum, NoSum};
 
-use core::core::{Block, BlockHeader, Output, TxKernel};
+use core::core::{Block, BlockHeader, Output, TxKernel, SwitchCommitHash};
 use core::core::target::Difficulty;
 use core::core::hash::Hash;
 use grin_store::Error::NotFoundErr;
@@ -360,21 +360,26 @@ impl Chain {
 	/// way that's consistent with the current chain state and more
 	/// specifically the current winning fork.
 	pub fn get_unspent(&self, output_ref: &Commitment) -> Result<Output, Error> {
-		let sumtrees = self.sumtrees.read().unwrap();
-		let is_unspent = sumtrees.is_unspent(output_ref)?;
-		if is_unspent {
-			self.store
-				.get_output_by_commit(output_ref)
-				.map_err(|e| Error::StoreErr(e, "chain get unspent".to_owned()))
-		} else {
-			Err(Error::OutputNotFound)
+		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())? {
+					Ok(out)
+				} else {
+					Err(Error::OutputNotFound)
+				}
+			}
+			Err(NotFoundErr) => Err(Error::OutputNotFound),
+			Err(e) => Err(Error::StoreErr(e, "chain get unspent".to_owned())),
 		}
 	}
 
 	/// Checks whether an output is unspent
-	pub fn is_unspent(&self, output_ref: &Commitment) -> Result<bool, Error> {
-		let sumtrees = self.sumtrees.read().unwrap();
-		sumtrees.is_unspent(output_ref)
+	pub fn is_unspent(&self, output_ref: &Commitment, switch: &SwitchCommitHash)
+		-> Result<bool, Error> {
+
+		let mut sumtrees = self.sumtrees.write().unwrap();
+		sumtrees.is_unspent(output_ref, switch)
 	}
 
 	/// Sets the sumtree roots on a brand new block by applying the block on the

chain/src/sumtree.rs

@@ -22,8 +22,8 @@ use std::sync::Arc;
 
 use util::secp::pedersen::{RangeProof, Commitment};
 
-use core::core::{Block, SumCommit, TxKernel};
-use core::core::pmmr::{Backend, HashSum, NoSum, Summable, PMMR};
+use core::core::{Block, SumCommit, SwitchCommitHash, TxKernel};
+use core::core::pmmr::{HashSum, NoSum, Summable, PMMR};
 use core::core::hash::Hashed;
 use grin_store;
 use grin_store::sumtree::PMMRBackend;
@@ -90,16 +90,22 @@ impl SumTrees {
 	}
 
 	/// Whether a given commitment exists in the Output MMR and it's unspent
-	pub fn is_unspent(&self, commit: &Commitment) -> Result<bool, Error> {
+	pub fn is_unspent(&mut self, commit: &Commitment, switch: &SwitchCommitHash)
+		-> Result<bool, Error> {
+
 		let rpos = self.commit_index.get_output_pos(commit);
 		match rpos {
 			Ok(pos) => {
-				// checking the position is within the MMR, the commit index could be
-				// returning rewound data
-				if pos > self.output_pmmr_h.last_pos {
-					Ok(false)
+				let output_pmmr = PMMR::at(
+					&mut self.output_pmmr_h.backend,
+					self.output_pmmr_h.last_pos
+				);
+				if let Some(hs) = output_pmmr.get(pos) {
+					let hashsum = HashSum::from_summable(
+						pos, &SumCommit{commit: commit.clone()}, Some(switch));
+					Ok(hs.hash == hashsum.hash)
 				} else {
-					Ok(self.output_pmmr_h.backend.get(pos).is_some())
+					Ok(false)
 				}
 			}
 			Err(grin_store::Error::NotFoundErr) => Ok(false),