From 599bf22cfc50dfb8650a92ba0deeed09e75ec57d Mon Sep 17 00:00:00 2001 From: Antioch Peverell Date: Mon, 17 Aug 2020 18:19:29 +0100 Subject: [PATCH] Simplify api commits (#3423) * simplify api with unspent by commitment * fix chain tests --- api/src/handlers/blocks_api.rs | 6 ++--- api/src/handlers/utils.rs | 38 ++++++++++---------------------- api/src/types.rs | 8 ++++--- chain/src/chain.rs | 18 +++++++-------- chain/src/txhashset/txhashset.rs | 10 +++++---- chain/tests/mine_simple_chain.rs | 12 +++++----- 6 files changed, 41 insertions(+), 51 deletions(-) diff --git a/api/src/handlers/blocks_api.rs b/api/src/handlers/blocks_api.rs index 7ddb2274c..0860149e4 100644 --- a/api/src/handlers/blocks_api.rs +++ b/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()), } diff --git a/api/src/handlers/utils.rs b/api/src/handlers/utils.rs index 96ed152f5..cb308640e 100644 --- a/api/src/handlers/utils.rs +++ b/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(weak: &Weak) -> Result, Error> { fn get_unspent( chain: &Arc, id: &str, -) -> Result, Error> { +) -> Result, Error> { let c = util::from_hex(id) .map_err(|_| 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), - ]; - - for x in outputs.iter() { - if let Some(output_pos) = chain.get_unspent(x)? { - return Ok(Some((output_pos, x.clone()))); - } - } - Ok(None) + let res = chain.get_unspent(commit)?; + Ok(res) } -/// 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, id: &str, ) -> Result, 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), - identifier, + Output::new(&out.commitment(), pos.height, pos.pos), + out, ))) } @@ -80,14 +66,14 @@ pub fn get_output_v2( include_merkle_proof: bool, ) -> Result, 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))) } diff --git a/api/src/types.rs b/api/src/types.rs index c212ae4de..8c13422f7 100644 --- a/api/src/types.rs +++ b/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 block_height = pos.map(|x| x.height).or(block_header.map(|x| x.height)); + 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 proof = if include_proof { Some(output.proof_bytes().to_hex()) diff --git a/chain/src/chain.rs b/chain/src/chain.rs index 917decb3c..1e85052d7 100644 --- a/chain/src/chain.rs +++ b/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, Error> { - self.txhashset.read().get_unspent(output_ref) + pub fn get_unspent( + &self, + commit: Commitment, + ) -> Result, 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( - &self, - output_ref: &OutputIdentifier, - ) -> Result { + pub fn get_header_for_output(&self, commit: Commitment) -> Result { 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)?) } diff --git a/chain/src/txhashset/txhashset.rs b/chain/src/txhashset/txhashset.rs index f9222dd6c..a5a91ee56 100644 --- a/chain/src/txhashset/txhashset.rs +++ b/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, Error> { - let commit = output_id.commit; + pub fn get_unspent( + &self, + commit: Commitment, + ) -> Result, 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 { - Ok(Some(pos)) + if out.commitment() == commit { + Ok(Some((out, pos))) } else { Ok(None) } diff --git a/chain/tests/mine_simple_chain.rs b/chain/tests/mine_simple_chain.rs index d5783e67d..a55e99707 100644 --- a/chain/tests/mine_simple_chain.rs +++ b/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); }