Simplify api commits (#3423)

* simplify api with unspent by commitment

* fix chain tests

api/src/handlers/blocks_api.rs

@@ -61,7 +61,7 @@ impl HeaderHandler {
 			Some((_, o)) => o,
 			None => return Err(ErrorKind::NotFound.into()),
 		};
-		match w(&self.chain)?.get_header_for_output(&oid) {
+		match w(&self.chain)?.get_header_for_output(oid.commitment()) {
 			Ok(header) => Ok(BlockHeaderPrintable::from_header(&header)),
 			Err(_) => Err(ErrorKind::NotFound.into()),
 		}
@@ -94,7 +94,7 @@ impl HeaderHandler {
 				Some((_, o)) => o,
 				None => return Err(ErrorKind::NotFound.into()),
 			};
-			match w(&self.chain)?.get_header_for_output(&oid) {
+			match w(&self.chain)?.get_header_for_output(oid.commitment()) {
 				Ok(header) => return Ok(header.hash()),
 				Err(_) => return Err(ErrorKind::NotFound.into()),
 			}
@@ -179,7 +179,7 @@ impl BlockHandler {
 				Some((_, o)) => o,
 				None => return Err(ErrorKind::NotFound.into()),
 			};
-			match w(&self.chain)?.get_header_for_output(&oid) {
+			match w(&self.chain)?.get_header_for_output(oid.commitment()) {
 				Ok(header) => return Ok(header.hash()),
 				Err(_) => return Err(ErrorKind::NotFound.into()),
 			}

api/src/handlers/utils.rs

@@ -14,7 +14,7 @@
 
 use crate::chain;
 use crate::chain::types::CommitPos;
-use crate::core::core::{OutputFeatures, OutputIdentifier};
+use crate::core::core::OutputIdentifier;
 use crate::rest::*;
 use crate::types::*;
 use crate::util;
@@ -33,42 +33,28 @@ pub fn w<T>(weak: &Weak<T>) -> Result<Arc<T>, Error> {
 fn get_unspent(
 	chain: &Arc<chain::Chain>,
 	id: &str,
-) -> Result<Option<(CommitPos, OutputIdentifier)>, Error> {
+) -> Result<Option<(OutputIdentifier, CommitPos)>, Error> {
 	let c = util::from_hex(id)
 		.map_err(|_| ErrorKind::Argument(format!("Not a valid commitment: {}", id)))?;
 	let commit = Commitment::from_vec(c);
-
+	let res = chain.get_unspent(commit)?;
-	// We need the features here to be able to generate the necessary hash
+	Ok(res)
-	// to compare against the hash in the output MMR.
-	// For now we can just try both (but this probably needs to be part of the api
-	// params)
-	let outputs = [
-		OutputIdentifier::new(OutputFeatures::Plain, &commit),
-		OutputIdentifier::new(OutputFeatures::Coinbase, &commit),
-	];
-
-	for x in outputs.iter() {
-		if let Some(output_pos) = chain.get_unspent(x)? {
-			return Ok(Some((output_pos, x.clone())));
-		}
-	}
-	Ok(None)
 }
 
-/// Retrieves an output from the chain given a commit id (a tiny bit iteratively)
+/// Retrieves an output from the chain given a commitment.
 pub fn get_output(
 	chain: &Weak<chain::Chain>,
 	id: &str,
 ) -> Result<Option<(Output, OutputIdentifier)>, Error> {
 	let chain = w(chain)?;
-	let (output_pos, identifier) = match get_unspent(&chain, id)? {
+	let (out, pos) = match get_unspent(&chain, id)? {
 		Some(x) => x,
 		None => return Ok(None),
 	};
 
 	Ok(Some((
-		Output::new(&identifier.commit, output_pos.height, output_pos.pos),
+		Output::new(&out.commitment(), pos.height, pos.pos),
-		identifier,
+		out,
 	)))
 }
 
@@ -80,14 +66,14 @@ pub fn get_output_v2(
 	include_merkle_proof: bool,
 ) -> Result<Option<(OutputPrintable, OutputIdentifier)>, Error> {
 	let chain = w(chain)?;
-	let (output_pos, identifier) = match get_unspent(&chain, id)? {
+	let (out, pos) = match get_unspent(&chain, id)? {
 		Some(x) => x,
 		None => return Ok(None),
 	};
 
-	let output = chain.get_unspent_output_at(output_pos.pos)?;
+	let output = chain.get_unspent_output_at(pos.pos)?;
 	let header = if include_merkle_proof && output.is_coinbase() {
-		chain.get_header_by_height(output_pos.height).ok()
+		chain.get_header_by_height(pos.height).ok()
 	} else {
 		None
 	};
@@ -100,5 +86,5 @@ pub fn get_output_v2(
 		include_merkle_proof,
 	)?;
 
-	Ok(Some((output_printable, identifier)))
+	Ok(Some((output_printable, out)))
 }

api/src/types.rs

@@ -291,7 +291,7 @@ impl OutputPrintable {
 		};
 
 		let out_id = core::OutputIdentifier::from(output);
-		let pos = chain.get_unspent(&out_id)?;
+		let pos = chain.get_unspent(out_id.commitment())?;
 
 		let spent = pos.is_none();
 
@@ -301,8 +301,10 @@ impl OutputPrintable {
 		// api is currently doing the right thing here:
 		// An output can be spent and then subsequently reused and the new instance unspent.
 		// This would result in a height that differs from the provided block height.
-		let output_pos = pos.map(|x| x.pos).unwrap_or(0);
+		let output_pos = pos.map(|(_, x)| x.pos).unwrap_or(0);
-		let block_height = pos.map(|x| x.height).or(block_header.map(|x| x.height));
+		let block_height = pos
+			.map(|(_, x)| x.height)
+			.or(block_header.map(|x| x.height));
 
 		let proof = if include_proof {
 			Some(output.proof_bytes().to_hex())

chain/src/chain.rs

@@ -568,11 +568,14 @@ impl Chain {
 		}
 	}
 
-	/// Returns Ok(Some(pos)) if output is unspent.
+	/// Returns Ok(Some((out, pos))) if output is unspent.
 	/// Returns Ok(None) if output is spent.
 	/// Returns Err if something went wrong beyond not finding the output.
-	pub fn get_unspent(&self, output_ref: &OutputIdentifier) -> Result<Option<CommitPos>, Error> {
+	pub fn get_unspent(
-		self.txhashset.read().get_unspent(output_ref)
+		&self,
+		commit: Commitment,
+	) -> Result<Option<(OutputIdentifier, CommitPos)>, Error> {
+		self.txhashset.read().get_unspent(commit)
 	}
 
 	/// Retrieves an unspent output using its PMMR position
@@ -1387,17 +1390,14 @@ impl Chain {
 	}
 
 	/// Gets the block header in which a given output appears in the txhashset.
-	pub fn get_header_for_output(
+	pub fn get_header_for_output(&self, commit: Commitment) -> Result<BlockHeader, Error> {
-		&self,
-		output_ref: &OutputIdentifier,
-	) -> Result<BlockHeader, Error> {
 		let header_pmmr = self.header_pmmr.read();
 		let txhashset = self.txhashset.read();
-		let output_pos = match txhashset.get_unspent(output_ref)? {
+		let (_, pos) = match txhashset.get_unspent(commit)? {
 			Some(o) => o,
 			None => return Err(ErrorKind::OutputNotFound.into()),
 		};
-		let hash = header_pmmr.get_header_hash_by_height(output_pos.height)?;
+		let hash = header_pmmr.get_header_hash_by_height(pos.height)?;
 		Ok(self.get_block_header(&hash)?)
 	}
 

chain/src/txhashset/txhashset.rs

@@ -255,15 +255,17 @@ impl TxHashSet {
 	/// Check if an output is unspent.
 	/// We look in the index to find the output MMR pos.
 	/// Then we check the entry in the output MMR and confirm the hash matches.
-	pub fn get_unspent(&self, output_id: &OutputIdentifier) -> Result<Option<CommitPos>, Error> {
+	pub fn get_unspent(
-		let commit = output_id.commit;
+		&self,
+		commit: Commitment,
+	) -> Result<Option<(OutputIdentifier, CommitPos)>, Error> {
 		match self.commit_index.get_output_pos_height(&commit) {
 			Ok(Some(pos)) => {
 				let output_pmmr: ReadonlyPMMR<'_, Output, _> =
 					ReadonlyPMMR::at(&self.output_pmmr_h.backend, self.output_pmmr_h.last_pos);
 				if let Some(out) = output_pmmr.get_data(pos.pos) {
-					if out == *output_id {
+					if out.commitment() == commit {
-						Ok(Some(pos))
+						Ok(Some((out, pos)))
 					} else {
 						Ok(None)
 					}

chain/tests/mine_simple_chain.rs

@@ -696,11 +696,11 @@ fn spend_in_fork_and_compact() {
 		assert_eq!(head.height, 6);
 		assert_eq!(head.hash(), prev_main.hash());
 		assert!(chain
-			.get_unspent(&OutputIdentifier::from(&tx2.outputs()[0]))
+			.get_unspent(tx2.outputs()[0].commitment())
 			.unwrap()
 			.is_some());
 		assert!(chain
-			.get_unspent(&OutputIdentifier::from(&tx1.outputs()[0]))
+			.get_unspent(tx1.outputs()[0].commitment())
 			.unwrap()
 			.is_none());
 
@@ -717,11 +717,11 @@ fn spend_in_fork_and_compact() {
 		assert_eq!(head.height, 7);
 		assert_eq!(head.hash(), prev_fork.hash());
 		assert!(chain
-			.get_unspent(&OutputIdentifier::from(&tx2.outputs()[0]))
+			.get_unspent(tx2.outputs()[0].commitment())
 			.unwrap()
 			.is_some());
 		assert!(chain
-			.get_unspent(&OutputIdentifier::from(&tx1.outputs()[0]))
+			.get_unspent(tx1.outputs()[0].commitment())
 			.unwrap()
 			.is_none());
 
@@ -796,7 +796,7 @@ fn output_header_mappings() {
 			chain.process_block(b, chain::Options::MINE).unwrap();
 
 			let header_for_output = chain
-				.get_header_for_output(&OutputIdentifier::from(&reward_outputs[n - 1]))
+				.get_header_for_output(reward_outputs[n - 1].commitment())
 				.unwrap();
 			assert_eq!(header_for_output.height, n as u64);
 
@@ -806,7 +806,7 @@ fn output_header_mappings() {
 		// Check all output positions are as expected
 		for n in 1..15 {
 			let header_for_output = chain
-				.get_header_for_output(&OutputIdentifier::from(&reward_outputs[n - 1]))
+				.get_header_for_output(reward_outputs[n - 1].commitment())
 				.unwrap();
 			assert_eq!(header_for_output.height, n as u64);
 		}