API: don't error on missing output

api/src/handlers/blocks_api.rs

@@ -57,7 +57,10 @@ impl HeaderHandler {
 	}
 
 	fn get_header_for_output(&self, commit_id: String) -> Result<BlockHeaderPrintable, Error> {
-		let oid = get_output(&self.chain, &commit_id)?.1;
+		let oid = match get_output(&self.chain, &commit_id)? {
+			Some((_, o)) => o,
+			None => return Err(ErrorKind::NotFound.into()),
+		};
 		match w(&self.chain)?.get_header_for_output(&oid) {
 			Ok(header) => Ok(BlockHeaderPrintable::from_header(&header)),
 			Err(_) => Err(ErrorKind::NotFound.into()),
@@ -87,7 +90,10 @@ impl HeaderHandler {
 			return Ok(hash);
 		}
 		if let Some(commit) = commit {
-			let oid = get_output_v2(&self.chain, &commit, false, false)?.1;
+			let oid = match get_output_v2(&self.chain, &commit, false, false)? {
+				Some((_, o)) => o,
+				None => return Err(ErrorKind::NotFound.into()),
+			};
 			match w(&self.chain)?.get_header_for_output(&oid) {
 				Ok(header) => return Ok(header.hash()),
 				Err(_) => return Err(ErrorKind::NotFound.into()),
@@ -169,7 +175,10 @@ impl BlockHandler {
 			return Ok(hash);
 		}
 		if let Some(commit) = commit {
-			let oid = get_output_v2(&self.chain, &commit, false, false)?.1;
+			let oid = match get_output_v2(&self.chain, &commit, false, false)? {
+				Some((_, o)) => o,
+				None => return Err(ErrorKind::NotFound.into()),
+			};
 			match w(&self.chain)?.get_header_for_output(&oid) {
 				Ok(header) => return Ok(header.hash()),
 				Err(_) => return Err(ErrorKind::NotFound.into()),

api/src/handlers/chain_api.rs

@@ -108,21 +108,6 @@ pub struct OutputHandler {
 }
 
 impl OutputHandler {
-	fn get_output(&self, id: &str) -> Result<Output, Error> {
-		let res = get_output(&self.chain, id)?;
-		Ok(res.0)
-	}
-
-	fn get_output_v2(
-		&self,
-		id: &str,
-		include_proof: bool,
-		include_merkle_proof: bool,
-	) -> Result<OutputPrintable, Error> {
-		let res = get_output_v2(&self.chain, id, include_proof, include_merkle_proof)?;
-		Ok(res.0)
-	}
-
 	pub fn get_outputs_v2(
 		&self,
 		commits: Option<Vec<String>>,
@@ -144,17 +129,23 @@ impl OutputHandler {
 				}
 			}
 			for commit in commits {
-				match self.get_output_v2(
+				match get_output_v2(
+					&self.chain,
 					&commit,
 					include_proof.unwrap_or(false),
 					include_merkle_proof.unwrap_or(false),
 				) {
-					Ok(output) => outputs.push(output),
-					// do not crash here simply do not retrieve this output
-					Err(e) => error!(
-						"Failure to get output for commitment {} with error {}",
-						commit, e
-					),
+					Ok(Some((output, _))) => outputs.push(output),
+					Ok(None) => {
+						// Ignore outputs that are not found
+					}
+					Err(e) => {
+						error!(
+							"Failure to get output for commitment {} with error {}",
+							commit, e
+						);
+						return Err(e.into());
+					}
 				};
 			}
 		}
@@ -219,12 +210,18 @@ impl OutputHandler {
 
 		let mut outputs: Vec<Output> = vec![];
 		for x in commitments {
-			match self.get_output(&x) {
-				Ok(output) => outputs.push(output),
-				Err(e) => error!(
-					"Failure to get output for commitment {} with error {}",
-					x, e
-				),
+			match get_output(&self.chain, &x) {
+				Ok(Some((output, _))) => outputs.push(output),
+				Ok(None) => {
+					// Ignore outputs that are not found
+				}
+				Err(e) => {
+					error!(
+						"Failure to get output for commitment {} with error {}",
+						x, e
+					);
+					return Err(e.into());
+				}
 			};
 		}
 		Ok(outputs)

api/src/handlers/utils.rs

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 use crate::chain;
+use crate::chain::types::CommitPos;
 use crate::core::core::{OutputFeatures, OutputIdentifier};
 use crate::rest::*;
 use crate::types::*;
@@ -29,11 +30,11 @@ pub fn w<T>(weak: &Weak<T>) -> Result<Arc<T>, Error> {
 		.ok_or_else(|| ErrorKind::Internal("failed to upgrade weak refernce".to_owned()).into())
 }
 
-/// Retrieves an output from the chain given a commit id (a tiny bit iteratively)
-pub fn get_output(
-	chain: &Weak<chain::Chain>,
+/// Internal function to retrieves an output by a given commitment
+fn retrieve_output(
+	chain: &Arc<chain::Chain>,
 	id: &str,
-) -> Result<(Output, OutputIdentifier), Error> {
+) -> Result<Option<(Commitment, CommitPos, OutputIdentifier)>, Error> {
 	let c = util::from_hex(String::from(id)).context(ErrorKind::Argument(format!(
 		"Not a valid commitment: {}",
 		id
@@ -49,28 +50,30 @@ pub fn get_output(
 		OutputIdentifier::new(OutputFeatures::Coinbase, &commit),
 	];
 
-	let chain = w(chain)?;
-
 	for x in outputs.iter() {
-		let res = chain.is_unspent(x);
-		match res {
-			Ok(output_pos) => {
-				return Ok((
-					Output::new(&commit, output_pos.height, output_pos.pos),
-					x.clone(),
-				));
-			}
-			Err(e) => {
-				trace!(
-					"get_output: err: {} for commit: {:?} with feature: {:?}",
-					e.to_string(),
-					x.commit,
-					x.features
-				);
-			}
+		match chain.is_unspent(x)? {
+			Some(output_pos) => return Ok(Some((commit, output_pos, x.clone()))),
+			None => {}
 		}
 	}
-	Err(ErrorKind::NotFound.into())
+	Ok(None)
+}
+
+/// Retrieves an output from the chain given a commit id (a tiny bit iteratively)
+pub fn get_output(
+	chain: &Weak<chain::Chain>,
+	id: &str,
+) -> Result<Option<(Output, OutputIdentifier)>, Error> {
+	let chain = w(chain)?;
+	let (commit, output_pos, identifier) = match retrieve_output(&chain, id)? {
+		Some(x) => x,
+		None => return Ok(None),
+	};
+
+	Ok(Some((
+		Output::new(&commit, output_pos.height, output_pos.pos),
+		identifier,
+	)))
 }
 
 /// Retrieves an output from the chain given a commit id (a tiny bit iteratively)
@@ -79,63 +82,27 @@ pub fn get_output_v2(
 	id: &str,
 	include_proof: bool,
 	include_merkle_proof: bool,
-) -> Result<(OutputPrintable, OutputIdentifier), Error> {
-	let c = util::from_hex(String::from(id)).context(ErrorKind::Argument(format!(
-		"Not a valid commitment: {}",
-		id
-	)))?;
-	let commit = Commitment::from_vec(c);
+) -> Result<Option<(OutputPrintable, OutputIdentifier)>, Error> {
+	let chain = w(chain)?;
+	let (_, output_pos, identifier) = match retrieve_output(&chain, id)? {
+		Some(x) => x,
+		None => return Ok(None),
+	};
 
-	// We need the features here to be able to generate the necessary hash
-	// 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),
-	];
+	let output = chain.get_unspent_output_at(output_pos.pos)?;
+	let header = if include_merkle_proof && output.is_coinbase() {
+		chain.get_header_by_height(output_pos.height).ok()
+	} else {
+		None
+	};
 
-	let chain = w(chain)?;
+	let output_printable = OutputPrintable::from_output(
+		&output,
+		chain.clone(),
+		header.as_ref(),
+		include_proof,
+		include_merkle_proof,
+	)?;
 
-	for x in outputs.iter() {
-		let res = chain.is_unspent(x);
-		match res {
-			Ok(output_pos) => match chain.get_unspent_output_at(output_pos.pos) {
-				Ok(output) => {
-					let header = if include_merkle_proof && output.is_coinbase() {
-						chain.get_header_by_height(output_pos.height).ok()
-					} else {
-						None
-					};
-					match OutputPrintable::from_output(
-						&output,
-						chain.clone(),
-						header.as_ref(),
-						include_proof,
-						include_merkle_proof,
-					) {
-						Ok(output_printable) => return Ok((output_printable, x.clone())),
-						Err(e) => {
-							trace!(
-								"get_output: err: {} for commit: {:?} with feature: {:?}",
-								e.to_string(),
-								x.commit,
-								x.features
-							);
-						}
-					}
-				}
-				Err(_) => return Err(ErrorKind::NotFound.into()),
-			},
-			Err(e) => {
-				trace!(
-					"get_output: err: {} for commit: {:?} with feature: {:?}",
-					e.to_string(),
-					x.commit,
-					x.features
-				);
-			}
-		}
-	}
-	Err(ErrorKind::NotFound.into())
+	Ok(Some((output_printable, identifier)))
 }

api/src/rest.rs

@@ -104,6 +104,14 @@ impl From<RouterError> for Error {
 	}
 }
 
+impl From<crate::chain::Error> for Error {
+	fn from(error: crate::chain::Error) -> Error {
+		Error {
+			inner: Context::new(ErrorKind::Internal(error.to_string())),
+		}
+	}
+}
+
 /// TLS config
 #[derive(Clone)]
 pub struct TLSConfig {

api/src/types.rs

@@ -289,8 +289,8 @@ impl OutputPrintable {
 		};
 
 		let out_id = core::OutputIdentifier::from_output(&output);
-		let res = chain.is_unspent(&out_id);
-		let (spent, block_height) = if let Ok(output_pos) = res {
+		let res = chain.is_unspent(&out_id)?;
+		let (spent, block_height) = if let Some(output_pos) = res {
 			(false, Some(output_pos.height))
 		} else {
 			(true, None)

chain/src/chain.rs

@@ -504,7 +504,7 @@ impl Chain {
 	/// spent. This querying is done in a way that is consistent with the
 	/// current chain state, specifically the current winning (valid, most
 	/// work) fork.
-	pub fn is_unspent(&self, output_ref: &OutputIdentifier) -> Result<CommitPos, Error> {
+	pub fn is_unspent(&self, output_ref: &OutputIdentifier) -> Result<Option<CommitPos>, Error> {
 		self.txhashset.read().is_unspent(output_ref)
 	}
 
@@ -1305,7 +1305,10 @@ impl Chain {
 	) -> Result<BlockHeader, Error> {
 		let header_pmmr = self.header_pmmr.read();
 		let txhashset = self.txhashset.read();
-		let output_pos = txhashset.is_unspent(output_ref)?;
+		let output_pos = match txhashset.is_unspent(output_ref)? {
+			Some(o) => o,
+			None => return Err(ErrorKind::OutputNotFound.into()),
+		};
 		let hash = header_pmmr.get_header_hash_by_height(output_pos.height)?;
 		Ok(self.get_block_header(&hash)?)
 	}

chain/src/store.rs

@@ -115,16 +115,20 @@ impl ChainStore {
 
 	/// Get PMMR pos for the given output commitment.
 	pub fn get_output_pos(&self, commit: &Commitment) -> Result<u64, Error> {
-		self.get_output_pos_height(commit).map(|(pos, _)| pos)
+		match self.get_output_pos_height(commit) {
+			Ok(Some((pos, _))) => Ok(pos),
+			Ok(None) => Err(Error::NotFoundErr(format!(
+				"Output position for: {:?}",
+				commit
+			))),
+			Err(e) => Err(e),
+		}
 	}
 
 	/// Get PMMR pos and block height for the given output commitment.
-	pub fn get_output_pos_height(&self, commit: &Commitment) -> Result<(u64, u64), Error> {
-		option_to_not_found(
-			self.db
-				.get_ser(&to_key(OUTPUT_POS_PREFIX, &mut commit.as_ref().to_vec())),
-			|| format!("Output position for: {:?}", commit),
-		)
+	pub fn get_output_pos_height(&self, commit: &Commitment) -> Result<Option<(u64, u64)>, Error> {
+		self.db
+			.get_ser(&to_key(OUTPUT_POS_PREFIX, &mut commit.as_ref().to_vec()))
 	}
 
 	/// Builds a new batch to be used with this store.

chain/src/txhashset/txhashset.rs

@@ -223,23 +223,23 @@ 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 is_unspent(&self, output_id: &OutputIdentifier) -> Result<CommitPos, Error> {
+	pub fn is_unspent(&self, output_id: &OutputIdentifier) -> Result<Option<CommitPos>, Error> {
 		let commit = output_id.commit;
 		match self.commit_index.get_output_pos_height(&commit) {
-			Ok((pos, height)) => {
+			Ok(Some((pos, height))) => {
 				let output_pmmr: ReadonlyPMMR<'_, Output, _> =
 					ReadonlyPMMR::at(&self.output_pmmr_h.backend, self.output_pmmr_h.last_pos);
 				if let Some(hash) = output_pmmr.get_hash(pos) {
 					if hash == output_id.hash_with_index(pos - 1) {
-						Ok(CommitPos { pos, height })
+						Ok(Some(CommitPos { pos, height }))
 					} else {
-						Err(ErrorKind::TxHashSetErr("txhashset hash mismatch".to_string()).into())
+						Ok(None)
 					}
 				} else {
-					Err(ErrorKind::OutputNotFound.into())
+					Ok(None)
 				}
 			}
-			Err(grin_store::Error::NotFoundErr(_)) => Err(ErrorKind::OutputNotFound.into()),
+			Ok(None) => Ok(None),
 			Err(e) => Err(ErrorKind::StoreErr(e, "txhashset unspent check".to_string()).into()),
 		}
 	}