From 707a2073a5309180ab521e822f10bb5078bba523 Mon Sep 17 00:00:00 2001
From: Ignotus Peverell <igno.peverell@protonmail.com>
Date: Mon, 25 Dec 2017 00:07:50 +0000
Subject: [PATCH] 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  |  2 +-
 chain/src/chain.rs   | 29 +++++++++++++++++------------
 chain/src/sumtree.rs | 22 ++++++++++++++--------
 3 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/api/src/handlers.rs b/api/src/handlers.rs
index e491ac7dc..1bd37dc8c 100644
--- a/api/src/handlers.rs
+++ b/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 {
diff --git a/chain/src/chain.rs b/chain/src/chain.rs
index 3b46e57b0..04ee2d977 100644
--- a/chain/src/chain.rs
+++ b/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
diff --git a/chain/src/sumtree.rs b/chain/src/sumtree.rs
index 872d86d3b..28ea0017a 100644
--- a/chain/src/sumtree.rs
+++ b/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),