diff --git a/api/src/handlers/blocks_api.rs b/api/src/handlers/blocks_api.rs index 206f4f101..58528043a 100644 --- a/api/src/handlers/blocks_api.rs +++ b/api/src/handlers/blocks_api.rs @@ -57,7 +57,10 @@ impl HeaderHandler { } fn get_header_for_output(&self, commit_id: String) -> Result { - 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()), diff --git a/api/src/handlers/chain_api.rs b/api/src/handlers/chain_api.rs index ef0a2d746..75abd2788 100644 --- a/api/src/handlers/chain_api.rs +++ b/api/src/handlers/chain_api.rs @@ -108,21 +108,6 @@ pub struct OutputHandler { } impl OutputHandler { - fn get_output(&self, id: &str) -> Result { - 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 { - let res = get_output_v2(&self.chain, id, include_proof, include_merkle_proof)?; - Ok(res.0) - } - pub fn get_outputs_v2( &self, commits: Option>, @@ -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 = 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) diff --git a/api/src/handlers/utils.rs b/api/src/handlers/utils.rs index 0770c5e03..24097883e 100644 --- a/api/src/handlers/utils.rs +++ b/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(weak: &Weak) -> Result, 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, +/// Internal function to retrieves an output by a given commitment +fn get_unspent( + chain: &Arc, id: &str, -) -> Result<(Output, OutputIdentifier), Error> { +) -> Result, Error> { let c = util::from_hex(String::from(id)).context(ErrorKind::Argument(format!( "Not a valid commitment: {}", id @@ -49,28 +50,29 @@ 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 - ); - } + if let Some(output_pos) = chain.get_unspent(x)? { + return Ok(Some((output_pos, x.clone()))); } } - 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, + id: &str, +) -> Result, Error> { + let chain = w(chain)?; + let (output_pos, identifier) = match get_unspent(&chain, id)? { + Some(x) => x, + None => return Ok(None), + }; + + Ok(Some(( + Output::new(&identifier.commit, output_pos.height, output_pos.pos), + identifier, + ))) } /// Retrieves an output from the chain given a commit id (a tiny bit iteratively) @@ -79,63 +81,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); - - // 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), - ]; - +) -> Result, Error> { let chain = w(chain)?; + let (output_pos, identifier) = match get_unspent(&chain, id)? { + Some(x) => x, + None => return Ok(None), + }; - 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()) + 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 output_printable = OutputPrintable::from_output( + &output, + chain.clone(), + header.as_ref(), + include_proof, + include_merkle_proof, + )?; + + Ok(Some((output_printable, identifier))) } diff --git a/api/src/rest.rs b/api/src/rest.rs index 8dde2204b..a9b38ddc5 100644 --- a/api/src/rest.rs +++ b/api/src/rest.rs @@ -104,6 +104,14 @@ impl From for Error { } } +impl From 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 { diff --git a/api/src/types.rs b/api/src/types.rs index b30e5ae07..027d60ff7 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -289,8 +289,8 @@ impl OutputPrintable { }; let out_id = core::OutputIdentifier::from(output); - let res = chain.is_unspent(&out_id); - let (spent, block_height) = if let Ok(output_pos) = res { + let res = chain.get_unspent(&out_id)?; + let (spent, block_height) = if let Some(output_pos) = res { (false, Some(output_pos.height)) } else { (true, None) diff --git a/chain/src/chain.rs b/chain/src/chain.rs index 0c2f38fd1..f3846377e 100644 --- a/chain/src/chain.rs +++ b/chain/src/chain.rs @@ -504,8 +504,8 @@ 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 { - self.txhashset.read().is_unspent(output_ref) + pub fn get_unspent(&self, output_ref: &OutputIdentifier) -> Result, Error> { + self.txhashset.read().get_unspent(output_ref) } /// Retrieves an unspent output using its PMMR position @@ -1305,7 +1305,10 @@ impl Chain { ) -> Result { 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.get_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)?) } diff --git a/chain/src/store.rs b/chain/src/store.rs index 2dfe79bfc..3cfbc62d6 100644 --- a/chain/src/store.rs +++ b/chain/src/store.rs @@ -115,16 +115,19 @@ impl ChainStore { /// Get PMMR pos for the given output commitment. pub fn get_output_pos(&self, commit: &Commitment) -> Result { - self.get_output_pos_height(commit).map(|(pos, _)| pos) + match self.get_output_pos_height(commit)? { + Some((pos, _)) => Ok(pos), + None => Err(Error::NotFoundErr(format!( + "Output position for: {:?}", + commit + ))), + } } /// 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, 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. @@ -274,16 +277,19 @@ impl<'a> Batch<'a> { /// Get output_pos from index. pub fn get_output_pos(&self, commit: &Commitment) -> Result { - self.get_output_pos_height(commit).map(|(pos, _)| pos) + match self.get_output_pos_height(commit)? { + Some((pos, _)) => Ok(pos), + None => Err(Error::NotFoundErr(format!( + "Output position for: {:?}", + commit + ))), + } } /// Get output_pos and block height from index. - 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: {:?}", commit), - ) + pub fn get_output_pos_height(&self, commit: &Commitment) -> Result, Error> { + self.db + .get_ser(&to_key(OUTPUT_POS_PREFIX, &mut commit.as_ref().to_vec())) } /// Get the previous header. diff --git a/chain/src/txhashset/txhashset.rs b/chain/src/txhashset/txhashset.rs index caa498e58..00485d076 100644 --- a/chain/src/txhashset/txhashset.rs +++ b/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 { + pub fn get_unspent(&self, output_id: &OutputIdentifier) -> Result, 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(out) = output_pmmr.get_data(pos) { if OutputIdentifier::from(out) == *output_id { - Ok(CommitPos { pos, height }) + Ok(Some(CommitPos { pos, height })) } else { - Err(ErrorKind::TxHashSetErr("txhashset 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()), } } @@ -425,7 +425,12 @@ impl TxHashSet { debug!("init_output_pos_index: {} utxos", outputs_pos.len()); - outputs_pos.retain(|x| batch.get_output_pos_height(&x.0).is_err()); + outputs_pos.retain(|x| { + batch + .get_output_pos_height(&x.0) + .map(|p| p.is_none()) + .unwrap_or(true) + }); debug!( "init_output_pos_index: {} utxos with missing index entries", @@ -982,7 +987,7 @@ impl<'a> Extension<'a> { fn apply_input(&mut self, input: &Input, batch: &Batch<'_>) -> Result { let commit = input.commitment(); - if let Ok((pos, height)) = batch.get_output_pos_height(&commit) { + if let Some((pos, height)) = batch.get_output_pos_height(&commit)? { // First check this input corresponds to an existing entry in the output MMR. if let Some(out) = self.output_pmmr.get_data(pos) { if OutputIdentifier::from(input) != out { diff --git a/chain/tests/mine_simple_chain.rs b/chain/tests/mine_simple_chain.rs index 364aad657..567f110cd 100644 --- a/chain/tests/mine_simple_chain.rs +++ b/chain/tests/mine_simple_chain.rs @@ -694,11 +694,13 @@ fn spend_in_fork_and_compact() { assert_eq!(head.height, 6); assert_eq!(head.hash(), prev_main.hash()); assert!(chain - .is_unspent(&OutputIdentifier::from(&tx2.outputs()[0])) - .is_ok()); + .get_unspent(&OutputIdentifier::from(&tx2.outputs()[0])) + .unwrap() + .is_some()); assert!(chain - .is_unspent(&OutputIdentifier::from(&tx1.outputs()[0])) - .is_err()); + .get_unspent(&OutputIdentifier::from(&tx1.outputs()[0])) + .unwrap() + .is_none()); // make the fork win let fork_next = prepare_block(&kc, &prev_fork, &chain, 10); @@ -713,11 +715,13 @@ fn spend_in_fork_and_compact() { assert_eq!(head.height, 7); assert_eq!(head.hash(), prev_fork.hash()); assert!(chain - .is_unspent(&OutputIdentifier::from(&tx2.outputs()[0])) - .is_ok()); + .get_unspent(&OutputIdentifier::from(&tx2.outputs()[0])) + .unwrap() + .is_some()); assert!(chain - .is_unspent(&OutputIdentifier::from(&tx1.outputs()[0])) - .is_err()); + .get_unspent(&OutputIdentifier::from(&tx1.outputs()[0])) + .unwrap() + .is_none()); // add 20 blocks to go past the test horizon let mut prev = prev_fork;