From fc3b2a13bf71ef6d9fa9f3892d40c7474b6a17ea Mon Sep 17 00:00:00 2001 From: debris Date: Thu, 6 Apr 2017 22:37:57 +0700 Subject: [PATCH 1/3] removed redundant StorageTransactionOutputProvider, clarified bip30 --- .../utils/memory_pool_transaction_provider.rs | 7 ++-- sync/src/utils/mod.rs | 4 +-- .../src/utils/storage_transaction_provider.rs | 31 ------------------ verification/src/accept_transaction.rs | 32 +++++++------------ 4 files changed, 15 insertions(+), 59 deletions(-) delete mode 100644 sync/src/utils/storage_transaction_provider.rs diff --git a/sync/src/utils/memory_pool_transaction_provider.rs b/sync/src/utils/memory_pool_transaction_provider.rs index 8f16615e..1cc39eab 100644 --- a/sync/src/utils/memory_pool_transaction_provider.rs +++ b/sync/src/utils/memory_pool_transaction_provider.rs @@ -4,13 +4,12 @@ use db::{PreviousTransactionOutputProvider, TransactionOutputObserver}; use miner::{DoubleSpendCheckResult, HashedOutPoint, NonFinalDoubleSpendSet}; use verification::TransactionError; use super::super::types::{MemoryPoolRef, StorageRef}; -use super::StorageTransactionOutputProvider; /// Transaction output observer, which looks into both storage && into memory pool. /// It also allows to replace non-final transactions in the memory pool. pub struct MemoryPoolTransactionOutputProvider { /// Storage provider - storage_provider: StorageTransactionOutputProvider, + storage_provider: StorageRef, /// Transaction inputs from memory pool transactions mempool_inputs: HashMap>, /// Previous outputs, for which we should return 'Not spent' value. @@ -29,7 +28,7 @@ impl MemoryPoolTransactionOutputProvider { DoubleSpendCheckResult::DoubleSpend(_, hash, index) => Err(TransactionError::UsingSpentOutput(hash, index)), // there are no transactions, which are spending same inputs in memory pool DoubleSpendCheckResult::NoDoubleSpend => Ok(MemoryPoolTransactionOutputProvider { - storage_provider: StorageTransactionOutputProvider::with_storage(storage), + storage_provider: storage, mempool_inputs: transaction.inputs.iter() .map(|input| ( input.previous_output.clone().into(), @@ -39,7 +38,7 @@ impl MemoryPoolTransactionOutputProvider { }), // there are non-final transactions, which are spending same inputs in memory pool DoubleSpendCheckResult::NonFinalDoubleSpend(nonfinal_spends) => Ok(MemoryPoolTransactionOutputProvider { - storage_provider: StorageTransactionOutputProvider::with_storage(storage), + storage_provider: storage, mempool_inputs: transaction.inputs.iter() .map(|input| ( input.previous_output.clone().into(), diff --git a/sync/src/utils/mod.rs b/sync/src/utils/mod.rs index 5c4b7385..fb279791 100644 --- a/sync/src/utils/mod.rs +++ b/sync/src/utils/mod.rs @@ -11,7 +11,6 @@ mod message_block_headers_provider; mod orphan_blocks_pool; mod orphan_transactions_pool; mod partial_merkle_tree; -mod storage_transaction_provider; mod synchronization_state; pub use self::average_speed_meter::AverageSpeedMeter; @@ -27,8 +26,7 @@ pub use self::message_block_headers_provider::MessageBlockHeadersProvider; pub use self::orphan_blocks_pool::OrphanBlocksPool; pub use self::orphan_transactions_pool::{OrphanTransactionsPool, OrphanTransaction}; pub use self::partial_merkle_tree::{PartialMerkleTree, build_partial_merkle_tree}; -pub use self::storage_transaction_provider::StorageTransactionOutputProvider; pub use self::synchronization_state::SynchronizationState; /// Block height type -pub type BlockHeight = u32; \ No newline at end of file +pub type BlockHeight = u32; diff --git a/sync/src/utils/storage_transaction_provider.rs b/sync/src/utils/storage_transaction_provider.rs deleted file mode 100644 index 9d9720da..00000000 --- a/sync/src/utils/storage_transaction_provider.rs +++ /dev/null @@ -1,31 +0,0 @@ -use chain::{TransactionOutput, OutPoint}; -use db::{PreviousTransactionOutputProvider, TransactionOutputObserver}; -use super::super::types::StorageRef; - -/// Transaction output observer, which looks into storage -pub struct StorageTransactionOutputProvider { - /// Storage reference - storage: StorageRef, -} - -impl StorageTransactionOutputProvider { - pub fn with_storage(storage: StorageRef) -> Self { - StorageTransactionOutputProvider { - storage: storage, - } - } -} - -impl TransactionOutputObserver for StorageTransactionOutputProvider { - fn is_spent(&self, prevout: &OutPoint) -> Option { - self.storage - .transaction_meta(&prevout.hash) - .and_then(|tm| tm.is_spent(prevout.index as usize)) - } -} - -impl PreviousTransactionOutputProvider for StorageTransactionOutputProvider { - fn previous_transaction_output(&self, prevout: &OutPoint, transaction_index: usize) -> Option { - self.storage.as_previous_transaction_output_provider().previous_transaction_output(prevout, transaction_index) - } -} diff --git a/verification/src/accept_transaction.rs b/verification/src/accept_transaction.rs index 151f70d3..bdc3a0c2 100644 --- a/verification/src/accept_transaction.rs +++ b/verification/src/accept_transaction.rs @@ -58,7 +58,6 @@ impl<'a> TransactionAcceptor<'a> { } pub struct MemoryPoolTransactionAcceptor<'a> { - pub bip30: TransactionBip30<'a>, pub missing_inputs: TransactionMissingInputs<'a>, pub maturity: TransactionMaturity<'a>, pub overspent: TransactionOverspent<'a>, @@ -85,7 +84,6 @@ impl<'a> MemoryPoolTransactionAcceptor<'a> { let params = network.consensus_params(); let transaction_index = 0; MemoryPoolTransactionAcceptor { - bip30: TransactionBip30::new_for_mempool(transaction, meta_store), missing_inputs: TransactionMissingInputs::new(transaction, prevout_store, transaction_index), maturity: TransactionMaturity::new(transaction, meta_store, height), overspent: TransactionOverspent::new(transaction, prevout_store), @@ -96,8 +94,8 @@ impl<'a> MemoryPoolTransactionAcceptor<'a> { } pub fn check(&self) -> Result<(), TransactionError> { - // TODO: b82 fails, when this is enabled, fix this - //try!(self.bip30.check()); + // Bip30 is not checked because we don't need to allow tx pool acceptance of an unspent duplicate. + // Tx pool validation is not strinctly a matter of consensus. try!(self.missing_inputs.check()); try!(self.maturity.check()); try!(self.overspent.check()); @@ -112,6 +110,15 @@ pub trait TransactionRule { fn check(&self) -> Result<(), TransactionError>; } +/// Bip30 validation +/// +/// A transaction hash that exists in the chain is not acceptable even if +/// the original is spent in the new block. This is not necessary nor is it +/// described by BIP30, but it is in the code referenced by BIP30. As such +/// the tx pool need only test against the chain, skipping the pool. +/// +/// source: +/// https://github.com/libbitcoin/libbitcoin/blob/61759b2fd66041bcdbc124b2f04ed5ddc20c7312/src/chain/transaction.cpp#L780-L785 pub struct TransactionBip30<'a> { transaction: CanonTransaction<'a>, store: &'a TransactionMetaProvider, @@ -134,27 +141,10 @@ impl<'a> TransactionBip30<'a> { exception: exception, } } - - fn new_for_mempool(transaction: CanonTransaction<'a>, store: &'a TransactionMetaProvider) -> Self { - TransactionBip30 { - transaction: transaction, - store: store, - exception: false, - } - } } impl<'a> TransactionRule for TransactionBip30<'a> { fn check(&self) -> Result<(), TransactionError> { - // we allow optionals here, cause previous output may be a part of current block - // yet, we do not need to check current block, cause duplicated transactions - // in the same block are also forbidden - // - // update* - // TODO: - // There is a potential consensus failure here, cause transaction before this one - // may have fully spent the output, and we, by checking only storage, have no knowladge - // of it match self.store.transaction_meta(&self.transaction.hash) { Some(ref meta) if !meta.is_fully_spent() && !self.exception => { Err(TransactionError::UnspentTransactionWithTheSameHash) From 2074076c41ff3d70beb7fdef98479de23f9daeba Mon Sep 17 00:00:00 2001 From: debris Date: Fri, 7 Apr 2017 11:46:20 +0700 Subject: [PATCH 2/3] simplify verification code --- chain/src/indexed_transaction.rs | 14 +---- chain/src/lib.rs | 2 +- db/src/block_chain_db.rs | 3 +- db/src/impls.rs | 60 +++++++------------ db/src/transaction_meta_provider.rs | 6 +- miner/src/block_assembler.rs | 12 +--- .../utils/memory_pool_transaction_provider.rs | 10 ++-- verification/src/accept_block.rs | 11 ---- verification/src/accept_chain.rs | 4 +- verification/src/accept_header.rs | 10 ---- verification/src/accept_transaction.rs | 20 +------ verification/src/duplex_store.rs | 12 ++-- verification/src/verify_block.rs | 18 ------ verification/src/verify_header.rs | 8 --- verification/src/verify_transaction.rs | 16 ----- 15 files changed, 42 insertions(+), 164 deletions(-) diff --git a/chain/src/indexed_transaction.rs b/chain/src/indexed_transaction.rs index f77d9efa..b1f1515a 100644 --- a/chain/src/indexed_transaction.rs +++ b/chain/src/indexed_transaction.rs @@ -1,4 +1,4 @@ -use std::{cmp, io, borrow, fmt}; +use std::{cmp, io, fmt}; use hash::H256; use ser::{Deserializable, Reader, Error as ReaderError}; use transaction::Transaction; @@ -56,15 +56,3 @@ impl Deserializable for IndexedTransaction { Ok(tx) } } - -pub struct IndexedTransactionsRef<'a, T> where T: 'a { - pub transactions: &'a [T], -} - -impl<'a, T> IndexedTransactionsRef<'a, T> where T: borrow::Borrow { - pub fn new(transactions: &'a [T]) -> Self { - IndexedTransactionsRef { - transactions: transactions, - } - } -} diff --git a/chain/src/lib.rs b/chain/src/lib.rs index 0c16e022..1d921b57 100644 --- a/chain/src/lib.rs +++ b/chain/src/lib.rs @@ -32,6 +32,6 @@ pub use transaction::{Transaction, TransactionInput, TransactionOutput, OutPoint pub use read_and_hash::{ReadAndHash, HashedData}; pub use indexed_block::IndexedBlock; pub use indexed_header::IndexedBlockHeader; -pub use indexed_transaction::{IndexedTransaction, IndexedTransactionsRef}; +pub use indexed_transaction::IndexedTransaction; pub type ShortTransactionID = hash::H48; diff --git a/db/src/block_chain_db.rs b/db/src/block_chain_db.rs index 8ce06536..2f338e33 100644 --- a/db/src/block_chain_db.rs +++ b/db/src/block_chain_db.rs @@ -473,9 +473,10 @@ impl PreviousTransactionOutputProvider for BlockChainDatabase where T: Key } impl TransactionOutputObserver for BlockChainDatabase where T: KeyValueDatabase { - fn is_spent(&self, prevout: &OutPoint) -> Option { + fn is_spent(&self, prevout: &OutPoint) -> bool { self.transaction_meta(&prevout.hash) .and_then(|meta| meta.is_spent(prevout.index as usize)) + .unwrap_or(false) } } diff --git a/db/src/impls.rs b/db/src/impls.rs index 6ae8bd30..a53c47cd 100644 --- a/db/src/impls.rs +++ b/db/src/impls.rs @@ -1,50 +1,36 @@ -use std::borrow::Borrow; -use chain::{OutPoint, TransactionOutput, IndexedTransactionsRef, IndexedTransaction, IndexedBlock}; +use chain::{OutPoint, TransactionOutput, IndexedBlock, IndexedTransaction}; use transaction_provider::PreviousTransactionOutputProvider; use transaction_meta_provider::TransactionOutputObserver; -impl<'a, T> PreviousTransactionOutputProvider for IndexedTransactionsRef<'a, T> - where T: Borrow + Send + Sync { - fn previous_transaction_output(&self, prevout: &OutPoint, transaction_index: usize) -> Option { - self.transactions.iter() - .take(transaction_index) - .map(Borrow::borrow) - .find(|tx| tx.hash == prevout.hash) - .and_then(|tx| tx.raw.outputs.get(prevout.index as usize)) - .cloned() - } +fn transaction_output(transactions: &[IndexedTransaction], prevout: &OutPoint) -> Option { + transactions.iter() + .find(|tx| tx.hash == prevout.hash) + .and_then(|tx| tx.raw.outputs.get(prevout.index as usize)) + .cloned() +} + +fn is_spent(transactions: &[IndexedTransaction], prevout: &OutPoint) -> bool { + // the code below is valid, but has rather poor performance + + // if previous transaction output appears more than once than we can safely + // tell that it's spent (double spent) + let spends = transactions.iter() + .flat_map(|tx| &tx.raw.inputs) + .filter(|input| &input.previous_output == prevout) + .take(2) + .count(); + + spends == 2 } impl PreviousTransactionOutputProvider for IndexedBlock { fn previous_transaction_output(&self, prevout: &OutPoint, transaction_index: usize) -> Option { - let txs = IndexedTransactionsRef::new(&self.transactions); - txs.previous_transaction_output(prevout, transaction_index) + transaction_output(&self.transactions[..transaction_index], prevout) } } impl TransactionOutputObserver for IndexedBlock { - fn is_spent(&self, prevout: &OutPoint) -> Option { - // the code below is valid, but commented out due it's poor performance - // we could optimize it by indexing all outputs once - // let tx: IndexedTransaction = { .. } - // let indexed_outputs: IndexedOutputs = tx.indexed_outputs(); - // indexed_outputs.is_spent() - //None - - // if previous transaction output appears more than once than we can safely - // tell that it's spent (double spent) - - let spends = self.transactions.iter() - .flat_map(|tx| &tx.raw.inputs) - .filter(|input| &input.previous_output == prevout) - .take(2) - .count(); - - match spends { - 0 => None, - 1 => Some(false), - 2 => Some(true), - _ => unreachable!("spends <= 2; self.take(2); qed"), - } + fn is_spent(&self, prevout: &OutPoint) -> bool { + is_spent(&self.transactions, prevout) } } diff --git a/db/src/transaction_meta_provider.rs b/db/src/transaction_meta_provider.rs index 2bf2d934..9f107ea5 100644 --- a/db/src/transaction_meta_provider.rs +++ b/db/src/transaction_meta_provider.rs @@ -4,10 +4,8 @@ use transaction_meta::TransactionMeta; /// Transaction output observers track if output has been spent pub trait TransactionOutputObserver: Send + Sync { - /// Returns None if we have no information about previous output - /// Returns Some(false) if we know that output hasn't been spent - /// Returns Some(true) if we know that output has been spent - fn is_spent(&self, prevout: &OutPoint) -> Option; + /// Returns true if we know that output has been spent + fn is_spent(&self, prevout: &OutPoint) -> bool; } /// Transaction meta provider stores transaction meta information diff --git a/miner/src/block_assembler.rs b/miner/src/block_assembler.rs index c621c8e3..3894eff5 100644 --- a/miner/src/block_assembler.rs +++ b/miner/src/block_assembler.rs @@ -277,7 +277,7 @@ impl BlockAssembler { #[cfg(test)] mod tests { - use chain::{IndexedTransaction, IndexedTransactionsRef}; + use chain::{IndexedTransaction}; use verification::constants::{MAX_BLOCK_SIZE, MAX_BLOCK_SIGOPS}; use memory_pool::Entry; use super::{SizePolicy, NextStep, FittingTransactionsIterator}; @@ -313,16 +313,6 @@ mod tests { assert_eq!(NextStep::FinishAndAppend.and(NextStep::Append), NextStep::FinishAndAppend); } - #[test] - fn test_fitting_transactions_iterator_no_transactions() { - let store: Vec = Vec::new(); - let store_ref = IndexedTransactionsRef::new(&store); - let entries: Vec = Vec::new(); - - let iter = FittingTransactionsIterator::new(&store_ref, entries.iter(), MAX_BLOCK_SIZE as u32, MAX_BLOCK_SIGOPS as u32, 0, 0); - assert!(iter.collect::>().is_empty()); - } - #[test] fn test_fitting_transactions_iterator_max_block_size_reached() { } diff --git a/sync/src/utils/memory_pool_transaction_provider.rs b/sync/src/utils/memory_pool_transaction_provider.rs index 1cc39eab..5a12575d 100644 --- a/sync/src/utils/memory_pool_transaction_provider.rs +++ b/sync/src/utils/memory_pool_transaction_provider.rs @@ -51,11 +51,11 @@ impl MemoryPoolTransactionOutputProvider { } impl TransactionOutputObserver for MemoryPoolTransactionOutputProvider { - fn is_spent(&self, prevout: &OutPoint) -> Option { + fn is_spent(&self, prevout: &OutPoint) -> bool { // check if this output is spent by some non-final mempool transaction if let Some(ref nonfinal_spends) = self.nonfinal_spends { if nonfinal_spends.double_spends.contains(&prevout.clone().into()) { - return Some(false); + return false; } } @@ -128,9 +128,9 @@ mod tests { // => // if t3 is also depending on t1[0] || t2[0], it will be rejected by verification as missing inputs let provider = MemoryPoolTransactionOutputProvider::for_transaction(storage, &memory_pool, &dchain.at(3)).unwrap(); - assert_eq!(provider.is_spent(&OutPoint { hash: dchain.at(0).hash(), index: 0, }), Some(false)); - assert_eq!(provider.is_spent(&OutPoint { hash: dchain.at(1).hash(), index: 0, }), None); - assert_eq!(provider.is_spent(&OutPoint { hash: dchain.at(2).hash(), index: 0, }), None); + assert_eq!(provider.is_spent(&OutPoint { hash: dchain.at(0).hash(), index: 0, }), false); + assert_eq!(provider.is_spent(&OutPoint { hash: dchain.at(1).hash(), index: 0, }), false); + assert_eq!(provider.is_spent(&OutPoint { hash: dchain.at(2).hash(), index: 0, }), false); assert_eq!(provider.previous_transaction_output(&OutPoint { hash: dchain.at(0).hash(), index: 0, }, 0), Some(dchain.at(0).outputs[0].clone())); assert_eq!(provider.previous_transaction_output(&OutPoint { hash: dchain.at(1).hash(), index: 0, }, 0), None); assert_eq!(provider.previous_transaction_output(&OutPoint { hash: dchain.at(2).hash(), index: 0, }, 0), None); diff --git a/verification/src/accept_block.rs b/verification/src/accept_block.rs index c8b2135e..72598e05 100644 --- a/verification/src/accept_block.rs +++ b/verification/src/accept_block.rs @@ -32,11 +32,6 @@ impl<'a> BlockAcceptor<'a> { } } -trait BlockRule { - /// If verification fails returns an error - fn check(&self) -> Result<(), Error>; -} - pub struct BlockFinality<'a> { block: CanonBlock<'a>, height: u32, @@ -49,9 +44,7 @@ impl<'a> BlockFinality<'a> { height: height, } } -} -impl<'a> BlockRule for BlockFinality<'a> { fn check(&self) -> Result<(), Error> { if self.block.is_final(self.height) { Ok(()) @@ -77,9 +70,7 @@ impl<'a> BlockSigops<'a> { max_sigops: max_sigops, } } -} -impl<'a> BlockRule for BlockSigops<'a> { fn check(&self) -> Result<(), Error> { let store = DuplexTransactionOutputProvider::new(self.store, &*self.block); let bip16_active = self.block.header.raw.time >= self.consensus_params.bip16_time; @@ -109,9 +100,7 @@ impl<'a> BlockCoinbaseClaim<'a> { height: height, } } -} -impl<'a> BlockRule for BlockCoinbaseClaim<'a> { fn check(&self) -> Result<(), Error> { let store = DuplexTransactionOutputProvider::new(self.store, &*self.block); diff --git a/verification/src/accept_chain.rs b/verification/src/accept_chain.rs index 37543135..e6f8f9af 100644 --- a/verification/src/accept_chain.rs +++ b/verification/src/accept_chain.rs @@ -25,7 +25,7 @@ impl<'a> ChainAcceptor<'a> { transactions: block.transactions() .into_iter() .enumerate() - .map(|(index, tx)| TransactionAcceptor::new( + .map(|(tx_index, tx)| TransactionAcceptor::new( store.as_transaction_meta_provider(), prevouts, spents, @@ -34,7 +34,7 @@ impl<'a> ChainAcceptor<'a> { block.hash(), height, block.header.raw.time, - index + tx_index )) .collect(), } diff --git a/verification/src/accept_header.rs b/verification/src/accept_header.rs index 426fa712..7bbbf7e5 100644 --- a/verification/src/accept_header.rs +++ b/verification/src/accept_header.rs @@ -30,10 +30,6 @@ impl<'a> HeaderAcceptor<'a> { } } -pub trait HeaderRule { - fn check(&self) -> Result<(), Error>; -} - pub struct HeaderVersion<'a> { header: CanonHeader<'a>, min_version: u32, @@ -46,9 +42,7 @@ impl<'a> HeaderVersion<'a> { min_version: min_version, } } -} -impl<'a> HeaderRule for HeaderVersion<'a> { fn check(&self) -> Result<(), Error> { if self.header.raw.version < self.min_version { Err(Error::OldVersionBlock) @@ -74,9 +68,7 @@ impl<'a> HeaderWork<'a> { network: network, } } -} -impl<'a> HeaderRule for HeaderWork<'a> { fn check(&self) -> Result<(), Error> { let previous_header_hash = self.header.raw.previous_header_hash.clone(); let time = self.header.raw.time; @@ -103,9 +95,7 @@ impl<'a> HeaderMedianTimestamp<'a> { network: network, } } -} -impl<'a> HeaderRule for HeaderMedianTimestamp<'a> { fn check(&self) -> Result<(), Error> { let median = median_timestamp(&self.header.raw, self.store, self.network); if self.header.raw.time <= median { diff --git a/verification/src/accept_transaction.rs b/verification/src/accept_transaction.rs index bdc3a0c2..edb85900 100644 --- a/verification/src/accept_transaction.rs +++ b/verification/src/accept_transaction.rs @@ -106,10 +106,6 @@ impl<'a> MemoryPoolTransactionAcceptor<'a> { } } -pub trait TransactionRule { - fn check(&self) -> Result<(), TransactionError>; -} - /// Bip30 validation /// /// A transaction hash that exists in the chain is not acceptable even if @@ -141,9 +137,7 @@ impl<'a> TransactionBip30<'a> { exception: exception, } } -} -impl<'a> TransactionRule for TransactionBip30<'a> { fn check(&self) -> Result<(), TransactionError> { match self.store.transaction_meta(&self.transaction.hash) { Some(ref meta) if !meta.is_fully_spent() && !self.exception => { @@ -168,9 +162,7 @@ impl<'a> TransactionMissingInputs<'a> { transaction_index: transaction_index, } } -} -impl<'a> TransactionRule for TransactionMissingInputs<'a> { fn check(&self) -> Result<(), TransactionError> { let missing_index = self.transaction.raw.inputs.iter() .position(|input| { @@ -200,9 +192,7 @@ impl<'a> TransactionMaturity<'a> { height: height, } } -} -impl<'a> TransactionRule for TransactionMaturity<'a> { fn check(&self) -> Result<(), TransactionError> { // TODO: this is should also fail when we are trying to spend current block coinbase let immature_spend = self.transaction.raw.inputs.iter() @@ -231,9 +221,7 @@ impl<'a> TransactionOverspent<'a> { store: store, } } -} -impl<'a> TransactionRule for TransactionOverspent<'a> { fn check(&self) -> Result<(), TransactionError> { if self.transaction.raw.is_coinbase() { return Ok(()); @@ -271,9 +259,7 @@ impl<'a> TransactionSigops<'a> { time: time, } } -} -impl<'a> TransactionRule for TransactionSigops<'a> { fn check(&self) -> Result<(), TransactionError> { let bip16_active = self.time >= self.consensus_params.bip16_time; let sigops = transaction_sigops(&self.transaction.raw, &self.store, bip16_active); @@ -310,9 +296,7 @@ impl<'a> TransactionEval<'a> { verify_clocktime: verify_clocktime, } } -} -impl<'a> TransactionRule for TransactionEval<'a> { fn check(&self) -> Result<(), TransactionError> { if self.transaction.raw.is_coinbase() { return Ok(()); @@ -359,12 +343,10 @@ impl<'a> TransactionDoubleSpend<'a> { store: store, } } -} -impl<'a> TransactionRule for TransactionDoubleSpend<'a> { fn check(&self) -> Result<(), TransactionError> { for input in &self.transaction.raw.inputs { - if self.store.is_spent(&input.previous_output).unwrap_or(false) { + if self.store.is_spent(&input.previous_output) { return Err(TransactionError::UsingSpentOutput( input.previous_output.hash.clone(), input.previous_output.index diff --git a/verification/src/duplex_store.rs b/verification/src/duplex_store.rs index 6d734fd0..31ab8614 100644 --- a/verification/src/duplex_store.rs +++ b/verification/src/duplex_store.rs @@ -42,12 +42,8 @@ impl<'a> DuplexTransactionOutputObserver<'a> { } impl<'a> TransactionOutputObserver for DuplexTransactionOutputObserver<'a> { - fn is_spent(&self, prevout: &OutPoint) -> Option { - if self.first.is_spent(prevout).unwrap_or(false) { - Some(true) - } else { - self.second.is_spent(prevout) - } + fn is_spent(&self, prevout: &OutPoint) -> bool { + self.first.is_spent(prevout) || self.second.is_spent(prevout) } } @@ -60,7 +56,7 @@ impl PreviousTransactionOutputProvider for NoopStore { } impl TransactionOutputObserver for NoopStore { - fn is_spent(&self, _prevout: &OutPoint) -> Option { - None + fn is_spent(&self, _prevout: &OutPoint) -> bool { + false } } diff --git a/verification/src/verify_block.rs b/verification/src/verify_block.rs index 9672a272..a132ca40 100644 --- a/verification/src/verify_block.rs +++ b/verification/src/verify_block.rs @@ -40,10 +40,6 @@ impl<'a> BlockVerifier<'a> { } } -trait BlockRule { - fn check(&self) -> Result<(), Error>; -} - pub struct BlockEmpty<'a> { block: &'a IndexedBlock, } @@ -54,9 +50,7 @@ impl<'a> BlockEmpty<'a> { block: block, } } -} -impl<'a> BlockRule for BlockEmpty<'a> { fn check(&self) -> Result<(), Error> { if self.block.transactions.is_empty() { Err(Error::Empty) @@ -78,9 +72,7 @@ impl<'a> BlockSerializedSize<'a> { max_size: max_size, } } -} -impl<'a> BlockRule for BlockSerializedSize<'a> { fn check(&self) -> Result<(), Error> { let size = self.block.size(); if size > self.max_size { @@ -101,9 +93,7 @@ impl<'a> BlockCoinbase<'a> { block: block, } } -} -impl<'a> BlockRule for BlockCoinbase<'a> { fn check(&self) -> Result<(), Error> { if self.block.transactions.first().map(|tx| tx.raw.is_coinbase()).unwrap_or(false) { Ok(()) @@ -123,9 +113,7 @@ impl<'a> BlockExtraCoinbases<'a> { block: block, } } -} -impl<'a> BlockRule for BlockExtraCoinbases<'a> { fn check(&self) -> Result<(), Error> { let misplaced = self.block.transactions.iter() .skip(1) @@ -148,9 +136,7 @@ impl<'a> BlockTransactionsUniqueness<'a> { block: block, } } -} -impl<'a> BlockRule for BlockTransactionsUniqueness<'a> { fn check(&self) -> Result<(), Error> { let hashes = self.block.transactions.iter().map(|tx| tx.hash.clone()).collect::>(); if hashes.len() == self.block.transactions.len() { @@ -173,9 +159,7 @@ impl<'a> BlockSigops<'a> { max_sigops: max_sigops, } } -} -impl<'a> BlockRule for BlockSigops<'a> { fn check(&self) -> Result<(), Error> { // We cannot know if bip16 is enabled at this point so we disable it. let sigops = self.block.transactions.iter() @@ -200,9 +184,7 @@ impl<'a> BlockMerkleRoot<'a> { block: block, } } -} -impl<'a> BlockRule for BlockMerkleRoot<'a> { fn check(&self) -> Result<(), Error> { if self.block.merkle_root() == self.block.header.raw.merkle_root_hash { Ok(()) diff --git a/verification/src/verify_header.rs b/verification/src/verify_header.rs index 3b8c95f2..608ff9bc 100644 --- a/verification/src/verify_header.rs +++ b/verification/src/verify_header.rs @@ -25,10 +25,6 @@ impl<'a> HeaderVerifier<'a> { } } -pub trait HeaderRule { - fn check(&self) -> Result<(), Error>; -} - pub struct HeaderProofOfWork<'a> { header: &'a IndexedBlockHeader, max_work_bits: Compact, @@ -41,9 +37,7 @@ impl<'a> HeaderProofOfWork<'a> { max_work_bits: network.max_bits(), } } -} -impl<'a> HeaderRule for HeaderProofOfWork<'a> { fn check(&self) -> Result<(), Error> { if is_valid_proof_of_work(self.max_work_bits, self.header.raw.bits, &self.header.hash) { Ok(()) @@ -67,9 +61,7 @@ impl<'a> HeaderTimestamp<'a> { max_future: max_future, } } -} -impl<'a> HeaderRule for HeaderTimestamp<'a> { fn check(&self) -> Result<(), Error> { if self.header.raw.time > self.current_time + self.max_future { Err(Error::FuturisticTimestamp) diff --git a/verification/src/verify_transaction.rs b/verification/src/verify_transaction.rs index 9e3f7a34..da677a51 100644 --- a/verification/src/verify_transaction.rs +++ b/verification/src/verify_transaction.rs @@ -60,10 +60,6 @@ impl<'a> MemoryPoolTransactionVerifier<'a> { } } -trait TransactionRule { - fn check(&self) -> Result<(), TransactionError>; -} - pub struct TransactionEmpty<'a> { transaction: &'a IndexedTransaction, } @@ -74,9 +70,7 @@ impl<'a> TransactionEmpty<'a> { transaction: transaction, } } -} -impl<'a> TransactionRule for TransactionEmpty<'a> { fn check(&self) -> Result<(), TransactionError> { if self.transaction.raw.is_empty() { Err(TransactionError::Empty) @@ -96,9 +90,7 @@ impl<'a> TransactionNullNonCoinbase<'a> { transaction: transaction, } } -} -impl<'a> TransactionRule for TransactionNullNonCoinbase<'a> { fn check(&self) -> Result<(), TransactionError> { if !self.transaction.raw.is_coinbase() && self.transaction.raw.is_null() { Err(TransactionError::NullNonCoinbase) @@ -120,9 +112,7 @@ impl<'a> TransactionOversizedCoinbase<'a> { size_range: size_range, } } -} -impl<'a> TransactionRule for TransactionOversizedCoinbase<'a> { fn check(&self) -> Result<(), TransactionError> { if self.transaction.raw.is_coinbase() { let script_len = self.transaction.raw.inputs[0].script_sig.len(); @@ -144,9 +134,7 @@ impl<'a> TransactionMemoryPoolCoinbase<'a> { transaction: transaction, } } -} -impl<'a> TransactionRule for TransactionMemoryPoolCoinbase<'a> { fn check(&self) -> Result<(), TransactionError> { if self.transaction.raw.is_coinbase() { Err(TransactionError::MemoryPoolCoinbase) @@ -168,9 +156,7 @@ impl<'a> TransactionSize<'a> { max_size: max_size, } } -} -impl<'a> TransactionRule for TransactionSize<'a> { fn check(&self) -> Result<(), TransactionError> { if self.transaction.raw.serialized_size() > self.max_size { Err(TransactionError::MaxSize) @@ -192,9 +178,7 @@ impl<'a> TransactionSigops<'a> { max_sigops: max_sigops, } } -} -impl<'a> TransactionRule for TransactionSigops<'a> { fn check(&self) -> Result<(), TransactionError> { let sigops = transaction_sigops(&self.transaction.raw, &NoopStore, false); if sigops > self.max_sigops { From 21fdfabb0ee91b71310d94f9b3e6b1e9d38490d4 Mon Sep 17 00:00:00 2001 From: debris Date: Fri, 7 Apr 2017 12:54:40 +0700 Subject: [PATCH 3/3] consolidated TransactionOutputProvider and TransactionOutputObserver --- db/src/block_chain_db.rs | 12 ++--- db/src/{impls.rs => block_impls.rs} | 19 ++++--- db/src/lib.rs | 6 +-- db/src/store.rs | 17 ++---- db/src/transaction_meta_provider.rs | 16 ------ db/src/transaction_provider.rs | 27 +++++++--- miner/src/block_assembler.rs | 18 ++++--- miner/src/memory_pool.rs | 10 ++-- .../utils/memory_pool_transaction_provider.rs | 54 +++++++++---------- verification/src/accept_block.rs | 14 ++--- verification/src/accept_chain.rs | 10 ++-- verification/src/accept_transaction.rs | 46 +++++++--------- verification/src/chain_verifier.rs | 15 ++---- verification/src/duplex_store.rs | 45 +++++----------- verification/src/sigops.rs | 6 +-- 15 files changed, 135 insertions(+), 180 deletions(-) rename db/src/{impls.rs => block_impls.rs} (53%) delete mode 100644 db/src/transaction_meta_provider.rs diff --git a/db/src/block_chain_db.rs b/db/src/block_chain_db.rs index 2f338e33..51946438 100644 --- a/db/src/block_chain_db.rs +++ b/db/src/block_chain_db.rs @@ -19,8 +19,8 @@ use kv::{ use best_block::BestBlock; use { BlockRef, Error, BlockHeaderProvider, BlockProvider, BlockOrigin, TransactionMeta, IndexedBlockProvider, - TransactionMetaProvider, TransactionProvider, PreviousTransactionOutputProvider, BlockChain, Store, - SideChainOrigin, ForkChain, TransactionOutputObserver, Forkable, CanonStore + TransactionMetaProvider, TransactionProvider, TransactionOutputProvider, BlockChain, Store, + SideChainOrigin, ForkChain, Forkable, CanonStore }; const COL_COUNT: u32 = 10; @@ -463,17 +463,15 @@ impl TransactionProvider for BlockChainDatabase where T: KeyValueDatabase } } -impl PreviousTransactionOutputProvider for BlockChainDatabase where T: KeyValueDatabase { - fn previous_transaction_output(&self, prevout: &OutPoint, _transaction_index: usize) -> Option { +impl TransactionOutputProvider for BlockChainDatabase where T: KeyValueDatabase { + fn transaction_output(&self, prevout: &OutPoint, _transaction_index: usize) -> Option { // return previous transaction outputs only for canon chain transactions self.transaction_meta(&prevout.hash) .and_then(|_| self.transaction(&prevout.hash)) .and_then(|tx| tx.outputs.into_iter().nth(prevout.index as usize)) } -} -impl TransactionOutputObserver for BlockChainDatabase where T: KeyValueDatabase { - fn is_spent(&self, prevout: &OutPoint) -> bool { + fn is_double_spent(&self, prevout: &OutPoint) -> bool { self.transaction_meta(&prevout.hash) .and_then(|meta| meta.is_spent(prevout.index as usize)) .unwrap_or(false) diff --git a/db/src/impls.rs b/db/src/block_impls.rs similarity index 53% rename from db/src/impls.rs rename to db/src/block_impls.rs index a53c47cd..279c6b1d 100644 --- a/db/src/impls.rs +++ b/db/src/block_impls.rs @@ -1,6 +1,6 @@ +use std::cmp; use chain::{OutPoint, TransactionOutput, IndexedBlock, IndexedTransaction}; -use transaction_provider::PreviousTransactionOutputProvider; -use transaction_meta_provider::TransactionOutputObserver; +use {TransactionOutputProvider}; fn transaction_output(transactions: &[IndexedTransaction], prevout: &OutPoint) -> Option { transactions.iter() @@ -9,7 +9,7 @@ fn transaction_output(transactions: &[IndexedTransaction], prevout: &OutPoint) - .cloned() } -fn is_spent(transactions: &[IndexedTransaction], prevout: &OutPoint) -> bool { +fn is_double_spent(transactions: &[IndexedTransaction], prevout: &OutPoint) -> bool { // the code below is valid, but has rather poor performance // if previous transaction output appears more than once than we can safely @@ -23,14 +23,13 @@ fn is_spent(transactions: &[IndexedTransaction], prevout: &OutPoint) -> bool { spends == 2 } -impl PreviousTransactionOutputProvider for IndexedBlock { - fn previous_transaction_output(&self, prevout: &OutPoint, transaction_index: usize) -> Option { - transaction_output(&self.transactions[..transaction_index], prevout) +impl TransactionOutputProvider for IndexedBlock { + fn transaction_output(&self, outpoint: &OutPoint, transaction_index: usize) -> Option { + let take = cmp::min(transaction_index, self.transactions.len()); + transaction_output(&self.transactions[..take], outpoint) } -} -impl TransactionOutputObserver for IndexedBlock { - fn is_spent(&self, prevout: &OutPoint) -> bool { - is_spent(&self.transactions, prevout) + fn is_double_spent(&self, outpoint: &OutPoint) -> bool { + is_double_spent(&self.transactions, outpoint) } } diff --git a/db/src/lib.rs b/db/src/lib.rs index c643b326..7b060415 100644 --- a/db/src/lib.rs +++ b/db/src/lib.rs @@ -17,10 +17,9 @@ mod block_ref; mod block_chain; mod block_chain_db; mod error; -mod impls; +mod block_impls; mod store; mod transaction_meta; -mod transaction_meta_provider; mod transaction_provider; pub use primitives::{hash, bytes}; @@ -34,6 +33,5 @@ pub use block_chain_db::{BlockChainDatabase, ForkChainDatabase}; pub use error::Error; pub use store::{AsSubstore, Store, SharedStore, CanonStore}; pub use transaction_meta::TransactionMeta; -pub use transaction_meta_provider::{TransactionMetaProvider, TransactionOutputObserver}; -pub use transaction_provider::{TransactionProvider, PreviousTransactionOutputProvider}; +pub use transaction_provider::{TransactionProvider, TransactionOutputProvider, TransactionMetaProvider}; diff --git a/db/src/store.rs b/db/src/store.rs index 61ae2738..ac410a9f 100644 --- a/db/src/store.rs +++ b/db/src/store.rs @@ -2,8 +2,7 @@ use std::sync::Arc; use chain::BlockHeader; use { BestBlock, BlockProvider, BlockHeaderProvider, TransactionProvider, TransactionMetaProvider, - PreviousTransactionOutputProvider, BlockChain, IndexedBlockProvider, TransactionOutputObserver, - Forkable + TransactionOutputProvider, BlockChain, IndexedBlockProvider, Forkable }; pub trait CanonStore: Store + Forkable { @@ -23,21 +22,19 @@ pub trait Store: AsSubstore { } /// Allows casting Arc to reference to any substore type -pub trait AsSubstore: BlockChain + IndexedBlockProvider + TransactionProvider + TransactionMetaProvider + PreviousTransactionOutputProvider + TransactionOutputObserver { +pub trait AsSubstore: BlockChain + IndexedBlockProvider + TransactionProvider + TransactionMetaProvider + TransactionOutputProvider { fn as_block_provider(&self) -> &BlockProvider; fn as_block_header_provider(&self) -> &BlockHeaderProvider; fn as_transaction_provider(&self) -> &TransactionProvider; - fn as_previous_transaction_output_provider(&self) -> &PreviousTransactionOutputProvider; + fn as_transaction_output_provider(&self) -> &TransactionOutputProvider; fn as_transaction_meta_provider(&self) -> &TransactionMetaProvider; - - fn as_transaction_output_observer(&self) -> &TransactionOutputObserver; } -impl AsSubstore for T where T: BlockChain + IndexedBlockProvider + TransactionProvider + TransactionMetaProvider + PreviousTransactionOutputProvider + TransactionOutputObserver { +impl AsSubstore for T where T: BlockChain + IndexedBlockProvider + TransactionProvider + TransactionMetaProvider + TransactionOutputProvider { fn as_block_provider(&self) -> &BlockProvider { &*self } @@ -50,17 +47,13 @@ impl AsSubstore for T where T: BlockChain + IndexedBlockProvider + Transactio &*self } - fn as_previous_transaction_output_provider(&self) -> &PreviousTransactionOutputProvider { + fn as_transaction_output_provider(&self) -> &TransactionOutputProvider { &*self } fn as_transaction_meta_provider(&self) -> &TransactionMetaProvider { &*self } - - fn as_transaction_output_observer(&self) -> &TransactionOutputObserver { - &*self - } } pub type SharedStore = Arc; diff --git a/db/src/transaction_meta_provider.rs b/db/src/transaction_meta_provider.rs deleted file mode 100644 index 9f107ea5..00000000 --- a/db/src/transaction_meta_provider.rs +++ /dev/null @@ -1,16 +0,0 @@ -use primitives::hash::H256; -use chain::OutPoint; -use transaction_meta::TransactionMeta; - -/// Transaction output observers track if output has been spent -pub trait TransactionOutputObserver: Send + Sync { - /// Returns true if we know that output has been spent - fn is_spent(&self, prevout: &OutPoint) -> bool; -} - -/// Transaction meta provider stores transaction meta information -pub trait TransactionMetaProvider: Send + Sync { - /// Returns None if transactin with given hash does not exist - /// Otherwise returns transaction meta object - fn transaction_meta(&self, hash: &H256) -> Option; -} diff --git a/db/src/transaction_provider.rs b/db/src/transaction_provider.rs index 68e4c0ab..6ad2b8a3 100644 --- a/db/src/transaction_provider.rs +++ b/db/src/transaction_provider.rs @@ -1,23 +1,34 @@ use hash::H256; use bytes::Bytes; use chain::{Transaction, OutPoint, TransactionOutput}; +use {TransactionMeta}; +/// Should be used to obtain all transactions from canon chain and forks. pub trait TransactionProvider { - /// returns true if store contains given transaction + /// Returns true if store contains given transaction. fn contains_transaction(&self, hash: &H256) -> bool { self.transaction(hash).is_some() } - /// resolves transaction body bytes by transaction hash + /// Resolves transaction body bytes by transaction hash. fn transaction_bytes(&self, hash: &H256) -> Option; - /// resolves serialized transaction info by transaction hash + /// Resolves serialized transaction info by transaction hash. fn transaction(&self, hash: &H256) -> Option; } -/// During transaction verifiction the only part of old transaction that we need is `TransactionOutput`. -/// Structures like `IndexedBlock` or `MemoryPool` already have it in memory, so it would be -/// a shame to clone the whole transaction just to get single output. -pub trait PreviousTransactionOutputProvider: Send + Sync { - fn previous_transaction_output(&self, prevout: &OutPoint, transaction_index: usize) -> Option; +/// Should be used to get canon chain transaction outputs. +pub trait TransactionOutputProvider: Send + Sync { + /// Returns transaction output. + fn transaction_output(&self, outpoint: &OutPoint, transaction_index: usize) -> Option; + + /// Returns true if we know that output is double spent. + fn is_double_spent(&self, outpoint: &OutPoint) -> bool; +} + +/// Transaction meta provider stores transaction meta information +pub trait TransactionMetaProvider: Send + Sync { + /// Returns None if transactin with given hash does not exist + /// Otherwise returns transaction meta object + fn transaction_meta(&self, hash: &H256) -> Option; } diff --git a/miner/src/block_assembler.rs b/miner/src/block_assembler.rs index 3894eff5..06afa215 100644 --- a/miner/src/block_assembler.rs +++ b/miner/src/block_assembler.rs @@ -2,7 +2,7 @@ use std::collections::HashSet; use primitives::hash::H256; use primitives::compact::Compact; use chain::{OutPoint, TransactionOutput, IndexedTransaction}; -use db::{SharedStore, PreviousTransactionOutputProvider}; +use db::{SharedStore, TransactionOutputProvider}; use network::Magic; use memory_pool::{MemoryPool, OrderingStrategy, Entry}; use verification::{work_required, block_reward_satoshi, transaction_sigops}; @@ -133,7 +133,7 @@ impl Default for BlockAssembler { /// Iterator iterating over mempool transactions and yielding only those which fit the block struct FittingTransactionsIterator<'a, T> { /// Shared store is used to query previous transaction outputs from database - store: &'a PreviousTransactionOutputProvider, + store: &'a TransactionOutputProvider, /// Memory pool transactions iterator iter: T, /// New block height @@ -153,7 +153,7 @@ struct FittingTransactionsIterator<'a, T> { } impl<'a, T> FittingTransactionsIterator<'a, T> where T: Iterator { - fn new(store: &'a PreviousTransactionOutputProvider, iter: T, max_block_size: u32, max_block_sigops: u32, block_height: u32, block_time: u32) -> Self { + fn new(store: &'a TransactionOutputProvider, iter: T, max_block_size: u32, max_block_sigops: u32, block_height: u32, block_time: u32) -> Self { FittingTransactionsIterator { store: store, iter: iter, @@ -169,9 +169,9 @@ impl<'a, T> FittingTransactionsIterator<'a, T> where T: Iterator PreviousTransactionOutputProvider for FittingTransactionsIterator<'a, T> where T: Send + Sync { - fn previous_transaction_output(&self, prevout: &OutPoint, transaction_index: usize) -> Option { - self.store.previous_transaction_output(prevout, transaction_index) +impl<'a, T> TransactionOutputProvider for FittingTransactionsIterator<'a, T> where T: Send + Sync { + fn transaction_output(&self, prevout: &OutPoint, transaction_index: usize) -> Option { + self.store.transaction_output(prevout, transaction_index) .or_else(|| { self.previous_entries.iter() .find(|e| e.hash == prevout.hash) @@ -179,6 +179,10 @@ impl<'a, T> PreviousTransactionOutputProvider for FittingTransactionsIterator<'a .cloned() }) } + + fn is_double_spent(&self, _outpoint: &OutPoint) -> bool { + unimplemented!(); + } } impl<'a, T> Iterator for FittingTransactionsIterator<'a, T> where T: Iterator + Send + Sync { @@ -252,7 +256,7 @@ impl BlockAssembler { let mut transactions = Vec::new(); let mempool_iter = mempool.iter(OrderingStrategy::ByTransactionScore); - let tx_iter = FittingTransactionsIterator::new(store.as_previous_transaction_output_provider(), mempool_iter, self.max_block_size, self.max_block_sigops, height, time); + let tx_iter = FittingTransactionsIterator::new(store.as_transaction_output_provider(), mempool_iter, self.max_block_size, self.max_block_sigops, height, time); for entry in tx_iter { // miner_fee is i64, but we can safely cast it to u64 // memory pool should restrict miner fee to be positive diff --git a/miner/src/memory_pool.rs b/miner/src/memory_pool.rs index 40166e40..b9634aae 100644 --- a/miner/src/memory_pool.rs +++ b/miner/src/memory_pool.rs @@ -5,7 +5,7 @@ //! transactions. //! It also guarantees that ancestor-descendant relation won't break during ordered removal (ancestors always removed //! before descendants). Removal using `remove_by_hash` can break this rule. -use db::{TransactionProvider, PreviousTransactionOutputProvider}; +use db::{TransactionProvider, TransactionOutputProvider}; use primitives::bytes::Bytes; use primitives::hash::H256; use chain::{IndexedTransaction, Transaction, OutPoint, TransactionOutput}; @@ -812,12 +812,16 @@ impl TransactionProvider for MemoryPool { } } -impl PreviousTransactionOutputProvider for MemoryPool { - fn previous_transaction_output(&self, prevout: &OutPoint, _transaction_index: usize) -> Option { +impl TransactionOutputProvider for MemoryPool { + fn transaction_output(&self, prevout: &OutPoint, _transaction_index: usize) -> Option { self.get(&prevout.hash) .and_then(|tx| tx.outputs.get(prevout.index as usize)) .cloned() } + + fn is_double_spent(&self, outpoint: &OutPoint) -> bool { + self.is_spent(outpoint) + } } impl HeapSizeOf for MemoryPool { diff --git a/sync/src/utils/memory_pool_transaction_provider.rs b/sync/src/utils/memory_pool_transaction_provider.rs index 5a12575d..32b03cda 100644 --- a/sync/src/utils/memory_pool_transaction_provider.rs +++ b/sync/src/utils/memory_pool_transaction_provider.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; use chain::{Transaction, TransactionOutput, OutPoint}; -use db::{PreviousTransactionOutputProvider, TransactionOutputObserver}; +use db::TransactionOutputProvider; use miner::{DoubleSpendCheckResult, HashedOutPoint, NonFinalDoubleSpendSet}; use verification::TransactionError; use super::super::types::{MemoryPoolRef, StorageRef}; @@ -32,7 +32,7 @@ impl MemoryPoolTransactionOutputProvider { mempool_inputs: transaction.inputs.iter() .map(|input| ( input.previous_output.clone().into(), - memory_pool.previous_transaction_output(&input.previous_output, usize::max_value()), + memory_pool.transaction_output(&input.previous_output, usize::max_value()), )).collect(), nonfinal_spends: None, }), @@ -42,7 +42,7 @@ impl MemoryPoolTransactionOutputProvider { mempool_inputs: transaction.inputs.iter() .map(|input| ( input.previous_output.clone().into(), - memory_pool.previous_transaction_output(&input.previous_output, usize::max_value()), + memory_pool.transaction_output(&input.previous_output, usize::max_value()), )).collect(), nonfinal_spends: Some(nonfinal_spends), }), @@ -50,23 +50,8 @@ impl MemoryPoolTransactionOutputProvider { } } -impl TransactionOutputObserver for MemoryPoolTransactionOutputProvider { - fn is_spent(&self, prevout: &OutPoint) -> bool { - // check if this output is spent by some non-final mempool transaction - if let Some(ref nonfinal_spends) = self.nonfinal_spends { - if nonfinal_spends.double_spends.contains(&prevout.clone().into()) { - return false; - } - } - - // we can omit memory_pool check here, because it has been completed in `for_transaction` method - // => just check spending in storage - self.storage_provider.is_spent(prevout) - } -} - -impl PreviousTransactionOutputProvider for MemoryPoolTransactionOutputProvider { - fn previous_transaction_output(&self, prevout: &OutPoint, transaction_index: usize) -> Option { +impl TransactionOutputProvider for MemoryPoolTransactionOutputProvider { + fn transaction_output(&self, prevout: &OutPoint, transaction_index: usize) -> Option { let hashed_prevout: HashedOutPoint = prevout.clone().into(); // check if that is output of some transaction, which is vitually removed from memory pool @@ -87,7 +72,20 @@ impl PreviousTransactionOutputProvider for MemoryPoolTransactionOutputProvider { } // now check in storage - self.storage_provider.previous_transaction_output(prevout, transaction_index) + self.storage_provider.transaction_output(prevout, transaction_index) + } + + fn is_double_spent(&self, prevout: &OutPoint) -> bool { + // check if this output is spent by some non-final mempool transaction + if let Some(ref nonfinal_spends) = self.nonfinal_spends { + if nonfinal_spends.double_spends.contains(&prevout.clone().into()) { + return false; + } + } + + // we can omit memory_pool check here, because it has been completed in `for_transaction` method + // => just check spending in storage + self.storage_provider.is_double_spent(prevout) } } @@ -96,7 +94,7 @@ mod tests { use std::sync::Arc; use parking_lot::RwLock; use chain::OutPoint; - use db::{TransactionOutputObserver, PreviousTransactionOutputProvider, BlockChainDatabase}; + use db::{TransactionOutputProvider, BlockChainDatabase}; use miner::MemoryPool; use test_data; use super::MemoryPoolTransactionOutputProvider; @@ -128,11 +126,11 @@ mod tests { // => // if t3 is also depending on t1[0] || t2[0], it will be rejected by verification as missing inputs let provider = MemoryPoolTransactionOutputProvider::for_transaction(storage, &memory_pool, &dchain.at(3)).unwrap(); - assert_eq!(provider.is_spent(&OutPoint { hash: dchain.at(0).hash(), index: 0, }), false); - assert_eq!(provider.is_spent(&OutPoint { hash: dchain.at(1).hash(), index: 0, }), false); - assert_eq!(provider.is_spent(&OutPoint { hash: dchain.at(2).hash(), index: 0, }), false); - assert_eq!(provider.previous_transaction_output(&OutPoint { hash: dchain.at(0).hash(), index: 0, }, 0), Some(dchain.at(0).outputs[0].clone())); - assert_eq!(provider.previous_transaction_output(&OutPoint { hash: dchain.at(1).hash(), index: 0, }, 0), None); - assert_eq!(provider.previous_transaction_output(&OutPoint { hash: dchain.at(2).hash(), index: 0, }, 0), None); + assert_eq!(provider.is_double_spent(&OutPoint { hash: dchain.at(0).hash(), index: 0, }), false); + assert_eq!(provider.is_double_spent(&OutPoint { hash: dchain.at(1).hash(), index: 0, }), false); + assert_eq!(provider.is_double_spent(&OutPoint { hash: dchain.at(2).hash(), index: 0, }), false); + assert_eq!(provider.transaction_output(&OutPoint { hash: dchain.at(0).hash(), index: 0, }, 0), Some(dchain.at(0).outputs[0].clone())); + assert_eq!(provider.transaction_output(&OutPoint { hash: dchain.at(1).hash(), index: 0, }, 0), None); + assert_eq!(provider.transaction_output(&OutPoint { hash: dchain.at(2).hash(), index: 0, }, 0), None); } } diff --git a/verification/src/accept_block.rs b/verification/src/accept_block.rs index 72598e05..46ba6480 100644 --- a/verification/src/accept_block.rs +++ b/verification/src/accept_block.rs @@ -1,5 +1,5 @@ use network::{Magic, ConsensusParams}; -use db::PreviousTransactionOutputProvider; +use db::TransactionOutputProvider; use sigops::transaction_sigops; use work::block_reward_satoshi; use duplex_store::DuplexTransactionOutputProvider; @@ -15,7 +15,7 @@ pub struct BlockAcceptor<'a> { } impl<'a> BlockAcceptor<'a> { - pub fn new(store: &'a PreviousTransactionOutputProvider, network: Magic, block: CanonBlock<'a>, height: u32) -> Self { + pub fn new(store: &'a TransactionOutputProvider, network: Magic, block: CanonBlock<'a>, height: u32) -> Self { let params = network.consensus_params(); BlockAcceptor { finality: BlockFinality::new(block, height), @@ -56,13 +56,13 @@ impl<'a> BlockFinality<'a> { pub struct BlockSigops<'a> { block: CanonBlock<'a>, - store: &'a PreviousTransactionOutputProvider, + store: &'a TransactionOutputProvider, consensus_params: ConsensusParams, max_sigops: usize, } impl<'a> BlockSigops<'a> { - fn new(block: CanonBlock<'a>, store: &'a PreviousTransactionOutputProvider, consensus_params: ConsensusParams, max_sigops: usize) -> Self { + fn new(block: CanonBlock<'a>, store: &'a TransactionOutputProvider, consensus_params: ConsensusParams, max_sigops: usize) -> Self { BlockSigops { block: block, store: store, @@ -88,12 +88,12 @@ impl<'a> BlockSigops<'a> { pub struct BlockCoinbaseClaim<'a> { block: CanonBlock<'a>, - store: &'a PreviousTransactionOutputProvider, + store: &'a TransactionOutputProvider, height: u32, } impl<'a> BlockCoinbaseClaim<'a> { - fn new(block: CanonBlock<'a>, store: &'a PreviousTransactionOutputProvider, height: u32) -> Self { + fn new(block: CanonBlock<'a>, store: &'a TransactionOutputProvider, height: u32) -> Self { BlockCoinbaseClaim { block: block, store: store, @@ -111,7 +111,7 @@ impl<'a> BlockCoinbaseClaim<'a> { let mut incoming: u64 = 0; for input in tx.raw.inputs.iter() { let (sum, overflow) = incoming.overflowing_add( - store.previous_transaction_output(&input.previous_output, tx_idx).map(|o| o.value).unwrap_or(0)); + store.transaction_output(&input.previous_output, tx_idx).map(|o| o.value).unwrap_or(0)); if overflow { return Err(Error::ReferencedInputsSumOverflow); } diff --git a/verification/src/accept_chain.rs b/verification/src/accept_chain.rs index e6f8f9af..28306ca4 100644 --- a/verification/src/accept_chain.rs +++ b/verification/src/accept_chain.rs @@ -6,7 +6,7 @@ use canon::CanonBlock; use accept_block::BlockAcceptor; use accept_header::HeaderAcceptor; use accept_transaction::TransactionAcceptor; -use duplex_store::{DuplexTransactionOutputProvider, DuplexTransactionOutputObserver}; +use duplex_store::DuplexTransactionOutputProvider; pub struct ChainAcceptor<'a> { pub block: BlockAcceptor<'a>, @@ -17,18 +17,16 @@ pub struct ChainAcceptor<'a> { impl<'a> ChainAcceptor<'a> { pub fn new(store: &'a Store, network: Magic, block: CanonBlock<'a>, height: u32) -> Self { trace!(target: "verification", "Block verification {}", block.hash().to_reversed_str()); - let prevouts = DuplexTransactionOutputProvider::new(store.as_previous_transaction_output_provider(), block.raw()); - let spents = DuplexTransactionOutputObserver::new(store.as_transaction_output_observer(), block.raw()); + let output_store = DuplexTransactionOutputProvider::new(store.as_transaction_output_provider(), block.raw()); ChainAcceptor { - block: BlockAcceptor::new(store.as_previous_transaction_output_provider(), network, block, height), + block: BlockAcceptor::new(store.as_transaction_output_provider(), network, block, height), header: HeaderAcceptor::new(store.as_block_header_provider(), network, block.header(), height), transactions: block.transactions() .into_iter() .enumerate() .map(|(tx_index, tx)| TransactionAcceptor::new( store.as_transaction_meta_provider(), - prevouts, - spents, + output_store, network, tx, block.hash(), diff --git a/verification/src/accept_transaction.rs b/verification/src/accept_transaction.rs index edb85900..5daf0da4 100644 --- a/verification/src/accept_transaction.rs +++ b/verification/src/accept_transaction.rs @@ -1,8 +1,8 @@ use primitives::hash::H256; -use db::{TransactionMetaProvider, PreviousTransactionOutputProvider, TransactionOutputObserver}; +use db::{TransactionMetaProvider, TransactionOutputProvider}; use network::{Magic, ConsensusParams}; use script::{Script, verify_script, VerificationFlags, TransactionSignatureChecker, TransactionInputSigner}; -use duplex_store::{DuplexTransactionOutputProvider, DuplexTransactionOutputObserver}; +use duplex_store::DuplexTransactionOutputProvider; use sigops::transaction_sigops; use canon::CanonTransaction; use constants::{COINBASE_MATURITY, MAX_BLOCK_SIGOPS}; @@ -23,10 +23,7 @@ impl<'a> TransactionAcceptor<'a> { meta_store: &'a TransactionMetaProvider, // previous transaction outputs // in case of block validation, that's database and currently processed block - prevout_store: DuplexTransactionOutputProvider<'a>, - // in case of block validation, that's database and currently processed block - //spent_store: &'a TransactionOutputObserver, - spent_store: DuplexTransactionOutputObserver<'a>, + output_store: DuplexTransactionOutputProvider<'a>, network: Magic, transaction: CanonTransaction<'a>, block_hash: &'a H256, @@ -38,11 +35,11 @@ impl<'a> TransactionAcceptor<'a> { let params = network.consensus_params(); TransactionAcceptor { bip30: TransactionBip30::new_for_sync(transaction, meta_store, params.clone(), block_hash, height), - missing_inputs: TransactionMissingInputs::new(transaction, prevout_store, transaction_index), + missing_inputs: TransactionMissingInputs::new(transaction, output_store, transaction_index), maturity: TransactionMaturity::new(transaction, meta_store, height), - overspent: TransactionOverspent::new(transaction, prevout_store), - double_spent: TransactionDoubleSpend::new(transaction, spent_store), - eval: TransactionEval::new(transaction, prevout_store, params, height, time), + overspent: TransactionOverspent::new(transaction, output_store), + double_spent: TransactionDoubleSpend::new(transaction, output_store), + eval: TransactionEval::new(transaction, output_store, params, height, time), } } @@ -71,10 +68,7 @@ impl<'a> MemoryPoolTransactionAcceptor<'a> { // TODO: in case of memory pool it should be db and memory pool meta_store: &'a TransactionMetaProvider, // in case of memory pool it should be db and memory pool - prevout_store: DuplexTransactionOutputProvider<'a>, - // in case of memory pool it should be db and memory pool - //spent_store: &'a TransactionOutputObserver, - spent_store: DuplexTransactionOutputObserver<'a>, + output_store: DuplexTransactionOutputProvider<'a>, network: Magic, transaction: CanonTransaction<'a>, height: u32, @@ -84,12 +78,12 @@ impl<'a> MemoryPoolTransactionAcceptor<'a> { let params = network.consensus_params(); let transaction_index = 0; MemoryPoolTransactionAcceptor { - missing_inputs: TransactionMissingInputs::new(transaction, prevout_store, transaction_index), + missing_inputs: TransactionMissingInputs::new(transaction, output_store, transaction_index), maturity: TransactionMaturity::new(transaction, meta_store, height), - overspent: TransactionOverspent::new(transaction, prevout_store), - sigops: TransactionSigops::new(transaction, prevout_store, params.clone(), MAX_BLOCK_SIGOPS, time), - double_spent: TransactionDoubleSpend::new(transaction, spent_store), - eval: TransactionEval::new(transaction, prevout_store, params, height, time), + overspent: TransactionOverspent::new(transaction, output_store), + sigops: TransactionSigops::new(transaction, output_store, params.clone(), MAX_BLOCK_SIGOPS, time), + double_spent: TransactionDoubleSpend::new(transaction, output_store), + eval: TransactionEval::new(transaction, output_store, params, height, time), } } @@ -167,7 +161,7 @@ impl<'a> TransactionMissingInputs<'a> { let missing_index = self.transaction.raw.inputs.iter() .position(|input| { let is_not_null = !input.previous_output.is_null(); - let is_missing = self.store.previous_transaction_output(&input.previous_output, self.transaction_index).is_none(); + let is_missing = self.store.transaction_output(&input.previous_output, self.transaction_index).is_none(); is_not_null && is_missing }); @@ -228,7 +222,7 @@ impl<'a> TransactionOverspent<'a> { } let available = self.transaction.raw.inputs.iter() - .map(|input| self.store.previous_transaction_output(&input.previous_output, usize::max_value()).map(|o| o.value).unwrap_or(0)) + .map(|input| self.store.transaction_output(&input.previous_output, usize::max_value()).map(|o| o.value).unwrap_or(0)) .sum::(); let spends = self.transaction.raw.total_spends(); @@ -310,7 +304,7 @@ impl<'a> TransactionEval<'a> { }; for (index, input) in self.transaction.raw.inputs.iter().enumerate() { - let output = self.store.previous_transaction_output(&input.previous_output, usize::max_value()) + let output = self.store.transaction_output(&input.previous_output, usize::max_value()) .ok_or_else(|| TransactionError::UnknownReference(input.previous_output.hash.clone()))?; checker.input_index = index; @@ -331,13 +325,11 @@ impl<'a> TransactionEval<'a> { pub struct TransactionDoubleSpend<'a> { transaction: CanonTransaction<'a>, - //store: &'a TransactionOutputObserver, - store: DuplexTransactionOutputObserver<'a>, + store: DuplexTransactionOutputProvider<'a>, } impl<'a> TransactionDoubleSpend<'a> { - //fn new(transaction: CanonTransaction<'a>, store: &'a TransactionOutputObserver) -> Self { - fn new(transaction: CanonTransaction<'a>, store: DuplexTransactionOutputObserver<'a>) -> Self { + fn new(transaction: CanonTransaction<'a>, store: DuplexTransactionOutputProvider<'a>) -> Self { TransactionDoubleSpend { transaction: transaction, store: store, @@ -346,7 +338,7 @@ impl<'a> TransactionDoubleSpend<'a> { fn check(&self) -> Result<(), TransactionError> { for input in &self.transaction.raw.inputs { - if self.store.is_spent(&input.previous_output) { + if self.store.is_double_spent(&input.previous_output) { return Err(TransactionError::UsingSpentOutput( input.previous_output.hash.clone(), input.previous_output.index diff --git a/verification/src/chain_verifier.rs b/verification/src/chain_verifier.rs index 685b3e50..6330b885 100644 --- a/verification/src/chain_verifier.rs +++ b/verification/src/chain_verifier.rs @@ -2,14 +2,11 @@ use hash::H256; use chain::{IndexedBlock, IndexedBlockHeader, BlockHeader, Transaction}; -use db::{ - SharedStore, PreviousTransactionOutputProvider, BlockHeaderProvider, TransactionOutputObserver, - BlockOrigin -}; +use db::{SharedStore, TransactionOutputProvider, BlockHeaderProvider, BlockOrigin}; use network::Magic; use error::{Error, TransactionError}; use canon::{CanonBlock, CanonTransaction}; -use duplex_store::{DuplexTransactionOutputObserver, DuplexTransactionOutputProvider, NoopStore}; +use duplex_store::{DuplexTransactionOutputProvider, NoopStore}; use verify_chain::ChainVerifier; use verify_header::HeaderVerifier; use verify_transaction::MemoryPoolTransactionVerifier; @@ -89,7 +86,7 @@ impl BackwardsCompatibleChainVerifier { height: u32, time: u32, transaction: &Transaction, - ) -> Result<(), TransactionError> where T: PreviousTransactionOutputProvider + TransactionOutputObserver { + ) -> Result<(), TransactionError> where T: TransactionOutputProvider { let indexed_tx = transaction.clone().into(); // let's do preverification first let tx_verifier = MemoryPoolTransactionVerifier::new(&indexed_tx); @@ -98,12 +95,10 @@ impl BackwardsCompatibleChainVerifier { let canon_tx = CanonTransaction::new(&indexed_tx); // now let's do full verification let noop = NoopStore; - let prevouts = DuplexTransactionOutputProvider::new(prevout_provider, &noop); - let spents = DuplexTransactionOutputObserver::new(prevout_provider, &noop); + let output_store = DuplexTransactionOutputProvider::new(prevout_provider, &noop); let tx_acceptor = MemoryPoolTransactionAcceptor::new( self.store.as_transaction_meta_provider(), - prevouts, - spents, + output_store, self.network, canon_tx, height, diff --git a/verification/src/duplex_store.rs b/verification/src/duplex_store.rs index 31ab8614..ecae4647 100644 --- a/verification/src/duplex_store.rs +++ b/verification/src/duplex_store.rs @@ -2,16 +2,16 @@ //! require sophisticated (in more than one source) previous transaction lookups use chain::{OutPoint, TransactionOutput}; -use db::{PreviousTransactionOutputProvider, TransactionOutputObserver}; +use db::TransactionOutputProvider; #[derive(Clone, Copy)] pub struct DuplexTransactionOutputProvider<'a> { - first: &'a PreviousTransactionOutputProvider, - second: &'a PreviousTransactionOutputProvider, + first: &'a TransactionOutputProvider, + second: &'a TransactionOutputProvider, } impl<'a> DuplexTransactionOutputProvider<'a> { - pub fn new(first: &'a PreviousTransactionOutputProvider, second: &'a PreviousTransactionOutputProvider) -> Self { + pub fn new(first: &'a TransactionOutputProvider, second: &'a TransactionOutputProvider) -> Self { DuplexTransactionOutputProvider { first: first, second: second, @@ -19,44 +19,25 @@ impl<'a> DuplexTransactionOutputProvider<'a> { } } -impl<'a> PreviousTransactionOutputProvider for DuplexTransactionOutputProvider<'a> { - fn previous_transaction_output(&self, prevout: &OutPoint, transaction_index: usize) -> Option { - self.first.previous_transaction_output(prevout, transaction_index) - .or_else(|| self.second.previous_transaction_output(prevout, transaction_index)) +impl<'a> TransactionOutputProvider for DuplexTransactionOutputProvider<'a> { + fn transaction_output(&self, prevout: &OutPoint, transaction_index: usize) -> Option { + self.first.transaction_output(prevout, transaction_index) + .or_else(|| self.second.transaction_output(prevout, transaction_index)) } -} -#[derive(Clone, Copy)] -pub struct DuplexTransactionOutputObserver<'a> { - first: &'a TransactionOutputObserver, - second: &'a TransactionOutputObserver, -} - -impl<'a> DuplexTransactionOutputObserver<'a> { - pub fn new(first: &'a TransactionOutputObserver, second: &'a TransactionOutputObserver) -> Self { - DuplexTransactionOutputObserver { - first: first, - second: second, - } - } -} - -impl<'a> TransactionOutputObserver for DuplexTransactionOutputObserver<'a> { - fn is_spent(&self, prevout: &OutPoint) -> bool { - self.first.is_spent(prevout) || self.second.is_spent(prevout) + fn is_double_spent(&self, prevout: &OutPoint) -> bool { + self.first.is_double_spent(prevout) || self.second.is_double_spent(prevout) } } pub struct NoopStore; -impl PreviousTransactionOutputProvider for NoopStore { - fn previous_transaction_output(&self, _prevout: &OutPoint, _transaction_index: usize) -> Option { +impl TransactionOutputProvider for NoopStore { + fn transaction_output(&self, _prevout: &OutPoint, _transaction_index: usize) -> Option { None } -} -impl TransactionOutputObserver for NoopStore { - fn is_spent(&self, _prevout: &OutPoint) -> bool { + fn is_double_spent(&self, _prevout: &OutPoint) -> bool { false } } diff --git a/verification/src/sigops.rs b/verification/src/sigops.rs index 7dc5d770..cf9e2e16 100644 --- a/verification/src/sigops.rs +++ b/verification/src/sigops.rs @@ -1,5 +1,5 @@ use chain::Transaction; -use db::PreviousTransactionOutputProvider; +use db::TransactionOutputProvider; use script::Script; /// Counts signature operations in given transaction @@ -8,7 +8,7 @@ use script::Script; /// missing, we simply ignore that fact and just carry on counting pub fn transaction_sigops( transaction: &Transaction, - store: &PreviousTransactionOutputProvider, + store: &TransactionOutputProvider, bip16_active: bool ) -> usize { let output_sigops: usize = transaction.outputs.iter().map(|output| { @@ -27,7 +27,7 @@ pub fn transaction_sigops( let input_script: Script = input.script_sig.clone().into(); input_sigops += input_script.sigops_count(false); if bip16_active { - let previous_output = match store.previous_transaction_output(&input.previous_output, usize::max_value()) { + let previous_output = match store.transaction_output(&input.previous_output, usize::max_value()) { Some(output) => output, None => continue, };