Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify api commits #3423

Merged
merged 2 commits into from
Aug 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions api/src/handlers/blocks_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
}
Expand Down Expand Up @@ -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()),
}
Expand Down Expand Up @@ -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()),
}
Expand Down
38 changes: 12 additions & 26 deletions api/src/handlers/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);

// 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<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),
identifier,
Output::new(&out.commitment(), pos.height, pos.pos),
out,
)))
}

Expand All @@ -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
};
Expand All @@ -100,5 +86,5 @@ pub fn get_output_v2(
include_merkle_proof,
)?;

Ok(Some((output_printable, identifier)))
Ok(Some((output_printable, out)))
}
8 changes: 5 additions & 3 deletions api/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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())
Expand Down
18 changes: 9 additions & 9 deletions chain/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
self.txhashset.read().get_unspent(output_ref)
pub fn get_unspent(
&self,
commit: Commitment,
) -> Result<Option<(OutputIdentifier, CommitPos)>, Error> {
self.txhashset.read().get_unspent(commit)
}

/// Retrieves an unspent output using its PMMR position
Expand Down Expand Up @@ -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<BlockHeader, Error> {
pub fn get_header_for_output(&self, commit: Commitment) -> 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)?)
}

Expand Down
10 changes: 6 additions & 4 deletions chain/src/txhashset/txhashset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
let commit = output_id.commit;
pub fn get_unspent(
&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 {
Ok(Some(pos))
if out.commitment() == commit {
Ok(Some((out, pos)))
} else {
Ok(None)
}
Expand Down
12 changes: 6 additions & 6 deletions chain/tests/mine_simple_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -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());

Expand Down Expand Up @@ -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);

Expand All @@ -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);
}
Expand Down