From 5f5b1d2f135ab375d42c2efaf2477d8e180ae439 Mon Sep 17 00:00:00 2001 From: Antioch Peverell Date: Wed, 4 Mar 2020 08:36:33 +0000 Subject: [PATCH] no need to rehash with index to compare output with input spending it (#3260) * no need to rehash with index to compare output with input spending it * compare output identifier when checking is_unspent() * output identifier from cleanup --- api/src/types.rs | 2 +- chain/src/txhashset/txhashset.rs | 16 +++++++--------- chain/src/txhashset/utxo_view.rs | 9 ++++----- chain/tests/mine_simple_chain.rs | 16 ++++++++-------- core/src/core/transaction.rs | 31 ++++++++++++------------------- 5 files changed, 32 insertions(+), 42 deletions(-) diff --git a/api/src/types.rs b/api/src/types.rs index 4b82f698b5..b30e5ae074 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -288,7 +288,7 @@ impl OutputPrintable { OutputType::Transaction }; - let out_id = core::OutputIdentifier::from_output(&output); + let out_id = core::OutputIdentifier::from(output); let res = chain.is_unspent(&out_id); let (spent, block_height) = if let Ok(output_pos) = res { (false, Some(output_pos.height)) diff --git a/chain/src/txhashset/txhashset.rs b/chain/src/txhashset/txhashset.rs index a93847c5a6..caa498e582 100644 --- a/chain/src/txhashset/txhashset.rs +++ b/chain/src/txhashset/txhashset.rs @@ -20,7 +20,7 @@ use crate::core::core::hash::{Hash, Hashed}; use crate::core::core::merkle_proof::MerkleProof; use crate::core::core::pmmr::{self, Backend, ReadonlyPMMR, RewindablePMMR, PMMR}; use crate::core::core::{Block, BlockHeader, Input, Output, OutputIdentifier, TxKernel}; -use crate::core::ser::{PMMRIndexHashable, PMMRable, ProtocolVersion}; +use crate::core::ser::{PMMRable, ProtocolVersion}; use crate::error::{Error, ErrorKind}; use crate::store::{Batch, ChainStore}; use crate::txhashset::bitmap_accumulator::BitmapAccumulator; @@ -229,11 +229,11 @@ impl TxHashSet { Ok((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) { + if let Some(out) = output_pmmr.get_data(pos) { + if OutputIdentifier::from(out) == *output_id { Ok(CommitPos { pos, height }) } else { - Err(ErrorKind::TxHashSetErr("txhashset hash mismatch".to_string()).into()) + Err(ErrorKind::TxHashSetErr("txhashset mismatch".to_string()).into()) } } else { Err(ErrorKind::OutputNotFound.into()) @@ -984,11 +984,9 @@ impl<'a> Extension<'a> { let commit = input.commitment(); if let Ok((pos, height)) = batch.get_output_pos_height(&commit) { // First check this input corresponds to an existing entry in the output MMR. - if let Some(hash) = self.output_pmmr.get_hash(pos) { - if hash != input.hash_with_index(pos - 1) { - return Err( - ErrorKind::TxHashSetErr("output pmmr hash mismatch".to_string()).into(), - ); + if let Some(out) = self.output_pmmr.get_data(pos) { + if OutputIdentifier::from(input) != out { + return Err(ErrorKind::TxHashSetErr("output pmmr mismatch".to_string()).into()); } } diff --git a/chain/src/txhashset/utxo_view.rs b/chain/src/txhashset/utxo_view.rs index e239b6c0a0..8d4c5c4d87 100644 --- a/chain/src/txhashset/utxo_view.rs +++ b/chain/src/txhashset/utxo_view.rs @@ -16,9 +16,8 @@ use crate::core::core::hash::{Hash, Hashed}; use crate::core::core::pmmr::{self, ReadonlyPMMR}; -use crate::core::core::{Block, BlockHeader, Input, Output, Transaction}; +use crate::core::core::{Block, BlockHeader, Input, Output, OutputIdentifier, Transaction}; use crate::core::global; -use crate::core::ser::PMMRIndexHashable; use crate::error::{Error, ErrorKind}; use crate::store::Batch; use crate::util::secp::pedersen::RangeProof; @@ -75,11 +74,11 @@ impl<'a> UTXOView<'a> { // Input is valid if it is spending an (unspent) output // that currently exists in the output MMR. - // Compare the hash in the output MMR at the expected pos. + // Compare against the entry in output MMR at the expected pos. fn validate_input(&self, input: &Input, batch: &Batch<'_>) -> Result<(), Error> { if let Ok(pos) = batch.get_output_pos(&input.commitment()) { - if let Some(hash) = self.output_pmmr.get_hash(pos) { - if hash == input.hash_with_index(pos - 1) { + if let Some(out) = self.output_pmmr.get_data(pos) { + if OutputIdentifier::from(input) == out { return Ok(()); } } diff --git a/chain/tests/mine_simple_chain.rs b/chain/tests/mine_simple_chain.rs index 7211b72e0b..364aad657d 100644 --- a/chain/tests/mine_simple_chain.rs +++ b/chain/tests/mine_simple_chain.rs @@ -546,7 +546,7 @@ fn spend_rewind_spend() { // mine the first block and keep track of the block_hash // so we can spend the coinbase later let b = prepare_block_key_idx(&kc, &head, &chain, 2, 1); - let out_id = OutputIdentifier::from_output(&b.outputs()[0]); + let out_id = OutputIdentifier::from(&b.outputs()[0]); assert!(out_id.features.is_coinbase()); head = b.header.clone(); chain @@ -620,7 +620,7 @@ fn spend_in_fork_and_compact() { // mine the first block and keep track of the block_hash // so we can spend the coinbase later let b = prepare_block(&kc, &fork_head, &chain, 2); - let out_id = OutputIdentifier::from_output(&b.outputs()[0]); + let out_id = OutputIdentifier::from(&b.outputs()[0]); assert!(out_id.features.is_coinbase()); fork_head = b.header.clone(); chain @@ -694,10 +694,10 @@ fn spend_in_fork_and_compact() { assert_eq!(head.height, 6); assert_eq!(head.hash(), prev_main.hash()); assert!(chain - .is_unspent(&OutputIdentifier::from_output(&tx2.outputs()[0])) + .is_unspent(&OutputIdentifier::from(&tx2.outputs()[0])) .is_ok()); assert!(chain - .is_unspent(&OutputIdentifier::from_output(&tx1.outputs()[0])) + .is_unspent(&OutputIdentifier::from(&tx1.outputs()[0])) .is_err()); // make the fork win @@ -713,10 +713,10 @@ fn spend_in_fork_and_compact() { assert_eq!(head.height, 7); assert_eq!(head.hash(), prev_fork.hash()); assert!(chain - .is_unspent(&OutputIdentifier::from_output(&tx2.outputs()[0])) + .is_unspent(&OutputIdentifier::from(&tx2.outputs()[0])) .is_ok()); assert!(chain - .is_unspent(&OutputIdentifier::from_output(&tx1.outputs()[0])) + .is_unspent(&OutputIdentifier::from(&tx1.outputs()[0])) .is_err()); // add 20 blocks to go past the test horizon @@ -790,7 +790,7 @@ fn output_header_mappings() { chain.process_block(b, chain::Options::MINE).unwrap(); let header_for_output = chain - .get_header_for_output(&OutputIdentifier::from_output(&reward_outputs[n - 1])) + .get_header_for_output(&OutputIdentifier::from(&reward_outputs[n - 1])) .unwrap(); assert_eq!(header_for_output.height, n as u64); @@ -800,7 +800,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_output(&reward_outputs[n - 1])) + .get_header_for_output(&OutputIdentifier::from(&reward_outputs[n - 1])) .unwrap(); assert_eq!(header_for_output.height, n as u64); } diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index eeff00d86b..b7ee94cbe7 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -1424,7 +1424,7 @@ impl PMMRable for Output { type E = OutputIdentifier; fn as_elmt(&self) -> OutputIdentifier { - OutputIdentifier::from_output(self) + OutputIdentifier::from(self) } fn elmt_size() -> Option { @@ -1515,14 +1515,6 @@ impl OutputIdentifier { self.commit } - /// Build an output_identifier from an existing output. - pub fn from_output(output: &Output) -> OutputIdentifier { - OutputIdentifier { - features: output.features, - commit: output.commit, - } - } - /// Converts this identifier to a full output, provided a RangeProof pub fn into_output(self, proof: RangeProof) -> Output { Output { @@ -1532,14 +1524,6 @@ impl OutputIdentifier { } } - /// Build an output_identifier from an existing input. - pub fn from_input(input: &Input) -> OutputIdentifier { - OutputIdentifier { - features: input.features, - commit: input.commit, - } - } - /// convert an output_identifier to hex string format. pub fn to_hex(&self) -> String { format!( @@ -1567,8 +1551,8 @@ impl Readable for OutputIdentifier { } } -impl From for OutputIdentifier { - fn from(out: Output) -> Self { +impl From<&Output> for OutputIdentifier { + fn from(out: &Output) -> Self { OutputIdentifier { features: out.features, commit: out.commit, @@ -1576,6 +1560,15 @@ impl From for OutputIdentifier { } } +impl From<&Input> for OutputIdentifier { + fn from(input: &Input) -> Self { + OutputIdentifier { + features: input.features, + commit: input.commit, + } + } +} + #[cfg(test)] mod test { use super::*;