diff --git a/db/src/block_provider.rs b/db/src/block_provider.rs index 73fc8b36..bd3d0140 100644 --- a/db/src/block_provider.rs +++ b/db/src/block_provider.rs @@ -33,8 +33,3 @@ pub trait BlockProvider: BlockHeaderProvider { /// returns all transactions in the block by block reference (number/hash) fn block_transactions(&self, block_ref: BlockRef) -> Vec; } - -pub trait AsBlockHeaderProvider { - /// returns `BlockHeaderProvider` - fn as_block_header_provider(&self) -> &BlockHeaderProvider; -} diff --git a/db/src/block_stapler.rs b/db/src/block_stapler.rs index 088814f8..20d2cf45 100644 --- a/db/src/block_stapler.rs +++ b/db/src/block_stapler.rs @@ -49,4 +49,8 @@ pub trait BlockStapler { /// insert pre-processed block in the storage fn insert_indexed_block(&self, block: &IndexedBlock) -> Result; + + /// flushes the underlined store (if applicable) + fn flush(&self); + } diff --git a/db/src/indexed_block.rs b/db/src/indexed_block.rs index 0f06d3bd..15cf15d7 100644 --- a/db/src/indexed_block.rs +++ b/db/src/indexed_block.rs @@ -3,7 +3,7 @@ use chain::{Block, OutPoint, TransactionOutput, merkle_root}; use serialization::{Serializable, CompactInteger}; use indexed_header::IndexedBlockHeader; use indexed_transaction::IndexedTransaction; -use PreviousTransactionOutputProvider; +use {TransactionOutputObserver, PreviousTransactionOutputProvider}; #[derive(Debug, Clone)] pub struct IndexedBlock { @@ -16,9 +16,11 @@ impl PreviousTransactionOutputProvider for IndexedBlock { let txs: &[_] = &self.transactions; txs.previous_transaction_output(prevout) } +} - fn is_spent(&self, _prevout: &OutPoint) -> bool { - false +impl TransactionOutputObserver for IndexedBlock { + fn is_spent(&self, prevout: &OutPoint) -> Option { + self.previous_transaction_output(prevout).map(|_output| false) } } diff --git a/db/src/indexed_transaction.rs b/db/src/indexed_transaction.rs index 6ac70e2c..bcfec139 100644 --- a/db/src/indexed_transaction.rs +++ b/db/src/indexed_transaction.rs @@ -40,8 +40,4 @@ impl<'a> PreviousTransactionOutputProvider for &'a [IndexedTransaction] { .and_then(|tx| tx.raw.outputs.get(prevout.index as usize)) .cloned() } - - fn is_spent(&self, _prevout: &OutPoint) -> bool { - unimplemented!(); - } } diff --git a/db/src/lib.rs b/db/src/lib.rs index 6d8ed2cb..8c08c640 100644 --- a/db/src/lib.rs +++ b/db/src/lib.rs @@ -67,13 +67,13 @@ impl BlockLocation { pub type SharedStore = std::sync::Arc; pub use best_block::BestBlock; -pub use storage::{Storage, Store}; +pub use storage::{Storage, Store, AsSubstore}; pub use error::{Error, ConsistencyError}; pub use kvdb::Database; -pub use transaction_provider::{TransactionProvider, AsTransactionProvider, PreviousTransactionOutputProvider}; -pub use transaction_meta_provider::TransactionMetaProvider; +pub use transaction_provider::{TransactionProvider, PreviousTransactionOutputProvider}; +pub use transaction_meta_provider::{TransactionMetaProvider, TransactionOutputObserver}; pub use block_stapler::{BlockStapler, BlockInsertedChain}; -pub use block_provider::{BlockProvider, BlockHeaderProvider, AsBlockHeaderProvider}; +pub use block_provider::{BlockProvider, BlockHeaderProvider}; pub use indexed_block::IndexedBlock; pub use indexed_header::IndexedBlockHeader; pub use indexed_transaction::IndexedTransaction; diff --git a/db/src/storage.rs b/db/src/storage.rs index 53954925..a7c5d6d5 100644 --- a/db/src/storage.rs +++ b/db/src/storage.rs @@ -15,8 +15,8 @@ use lru_cache::LruCache; use transaction_meta::TransactionMeta; use error::{Error, ConsistencyError, MetaError}; use update_context::UpdateContext; -use block_provider::{BlockProvider, BlockHeaderProvider, AsBlockHeaderProvider}; -use transaction_provider::TransactionProvider; +use block_provider::{BlockProvider, BlockHeaderProvider}; +use transaction_provider::{TransactionProvider, PreviousTransactionOutputProvider}; use transaction_meta_provider::TransactionMetaProvider; use block_stapler::{BlockStapler, BlockInsertedChain, Reorganization}; @@ -43,7 +43,7 @@ const MAX_FORK_ROUTE_PRESET: usize = 2048; const TRANSACTION_CACHE_SIZE: usize = 524288; /// Blockchain storage interface -pub trait Store : BlockProvider + BlockStapler + TransactionProvider + TransactionMetaProvider + AsBlockHeaderProvider { +pub trait Store: AsSubstore { /// get best block fn best_block(&self) -> Option; @@ -54,6 +54,47 @@ pub trait Store : BlockProvider + BlockStapler + TransactionProvider + Transacti fn difficulty(&self) -> f64; } +/// Allows casting Arc to reference to any substore type +pub trait AsSubstore: BlockProvider + BlockStapler + TransactionProvider + TransactionMetaProvider { + fn as_block_provider(&self) -> &BlockProvider; + + fn as_block_header_provider(&self) -> &BlockHeaderProvider; + + fn as_block_stapler(&self) -> &BlockStapler; + + fn as_transaction_provider(&self) -> &TransactionProvider; + + fn as_previous_transaction_output_provider(&self) -> &PreviousTransactionOutputProvider; + + fn as_transaction_meta_provider(&self) -> &TransactionMetaProvider; +} + +impl AsSubstore for T where T: BlockProvider + BlockStapler + TransactionProvider + TransactionMetaProvider + PreviousTransactionOutputProvider { + fn as_block_provider(&self) -> &BlockProvider { + &*self + } + + fn as_block_header_provider(&self) -> &BlockHeaderProvider { + &*self + } + + fn as_block_stapler(&self) -> &BlockStapler { + &*self + } + + fn as_transaction_provider(&self) -> &TransactionProvider { + &*self + } + + fn as_previous_transaction_output_provider(&self) -> &PreviousTransactionOutputProvider { + &*self + } + + fn as_transaction_meta_provider(&self) -> &TransactionMetaProvider { + &*self + } +} + /// Blockchain storage with rocksdb database pub struct Storage { database: Database, @@ -211,7 +252,10 @@ impl Storage { match context.meta.entry(input.previous_output.hash.clone()) { Entry::Occupied(mut entry) => { let meta = entry.get_mut(); - if meta.is_spent(input.previous_output.index as usize) { + let is_spent = meta.is_spent(input.previous_output.index as usize) + .ok_or(Error::unknown_spending(&input.previous_output.hash))?; + + if is_spent { return Err(Error::double_spend(&input.previous_output.hash)); } meta.denote_used(input.previous_output.index as usize); @@ -219,8 +263,10 @@ impl Storage { Entry::Vacant(entry) => { let mut meta = self.transaction_meta(&input.previous_output.hash) .ok_or(Error::unknown_spending(&input.previous_output.hash))?; + let is_spent = meta.is_spent(input.previous_output.index as usize) + .ok_or(Error::unknown_spending(&input.previous_output.hash))?; - if meta.is_spent(input.previous_output.index as usize) { + if is_spent { return Err(Error::double_spend(&input.previous_output.hash)); } @@ -428,12 +474,6 @@ impl BlockHeaderProvider for Storage { } } -impl AsBlockHeaderProvider for Storage { - fn as_block_header_provider(&self) -> &BlockHeaderProvider { - &*self - } -} - impl BlockProvider for Storage { fn block_number(&self, hash: &H256) -> Option { self.get(COL_BLOCK_NUMBERS, &**hash) @@ -636,10 +676,13 @@ impl BlockStapler for Storage { } } } + + fn flush(&self) { + self.database.flush().expect("Failed to flush database. Out of disk space?"); + } } impl TransactionProvider for Storage { - fn transaction_bytes(&self, hash: &H256) -> Option { self.get(COL_TRANSACTIONS, &**hash) } @@ -647,53 +690,43 @@ impl TransactionProvider for Storage { fn transaction(&self, hash: &H256) -> Option { let mut cache = self.transaction_cache.write(); - let (tx, is_cached) = { - let cached_transaction = cache.get_mut(hash); - match cached_transaction { - None => { - ( - self.transaction_bytes(hash).map(|tx_bytes| { - let tx: chain::Transaction = deserialize(tx_bytes.as_ref()) - .expect("Failed to deserialize transaction: db corrupted?"); - tx - }), - false - ) - }, - Some(tx) => (Some(tx.clone()), true) - } - }; + if let Some(cached_transaction) = cache.get_mut(hash) { + return Some(cached_transaction.clone()); + } - match tx { - Some(ref tx) => { if !is_cached { cache.insert(hash.clone(), tx.clone()); } } - None => {} - }; + let tx: Option = self.transaction_bytes(hash).map(|tx_bytes| { + deserialize(tx_bytes.as_ref()).expect("Failed to deserialize transaction: db corrupted?") + }); + + if let Some(ref tx) = tx { + cache.insert(hash.clone(), tx.clone()); + } tx } } -impl TransactionMetaProvider for Storage { +impl PreviousTransactionOutputProvider for Storage { + fn previous_transaction_output(&self, prevout: &chain::OutPoint) -> Option { + self.transaction(&prevout.hash) + .and_then(|tx| tx.outputs.into_iter().nth(prevout.index as usize)) + } +} +impl TransactionMetaProvider for Storage { fn transaction_meta(&self, hash: &H256) -> Option { let mut cache = self.meta_cache.write(); - let (meta, is_cached) = { - let cached_meta = cache.get_mut(hash); - match cached_meta { - None => { - (self.get(COL_TRANSACTIONS_META, &**hash).map(|val| - TransactionMeta::from_bytes(&val).expect("Invalid transaction metadata: db corrupted?") - ), false) - }, - Some(meta) => (Some(meta.clone()), true) - } - }; + if let Some(cached_meta) = cache.get_mut(hash) { + return Some(cached_meta.clone()); + } - match meta { - Some(ref meta) => { if !is_cached { cache.insert(hash.clone(), meta.clone()); } } - None => {} - }; + let meta = self.get(COL_TRANSACTIONS_META, &**hash) + .map(|val| TransactionMeta::from_bytes(&val).expect("Invalid transaction metadata: db corrupted?")); + + if let Some(ref meta) = meta { + cache.insert(hash.clone(), meta.clone()); + } meta } @@ -836,7 +869,7 @@ mod tests { let genesis_coinbase = genesis.transactions()[0].hash(); let genesis_meta = store.transaction_meta(&genesis_coinbase).unwrap(); - assert!(!genesis_meta.is_spent(0)); + assert!(!genesis_meta.is_spent(0).unwrap()); let forged_block = test_data::block_builder() .header().parent(genesis.hash()).build() @@ -850,7 +883,7 @@ mod tests { store.insert_block(&forged_block).unwrap(); let genesis_meta = store.transaction_meta(&genesis_coinbase).unwrap(); - assert!(genesis_meta.is_spent(0)); + assert!(genesis_meta.is_spent(0).unwrap()); assert_eq!(store.best_block().expect("genesis block inserted").number, 1); } @@ -880,8 +913,8 @@ mod tests { store.insert_block(&block).unwrap(); let meta = store.transaction_meta(&block.transactions()[1].hash()).unwrap(); - assert!(meta.is_spent(0), "Transaction #1 first output in the new block should be recorded as spent"); - assert!(!meta.is_spent(1), "Transaction #1 second output in the new block should be recorded as unspent"); + assert!(meta.is_spent(0).unwrap(), "Transaction #1 first output in the new block should be recorded as spent"); + assert!(!meta.is_spent(1).unwrap(), "Transaction #1 second output in the new block should be recorded as unspent"); } #[test] @@ -925,12 +958,12 @@ mod tests { store.insert_block(&block2).unwrap(); let meta = store.transaction_meta(&tx_big).unwrap(); - assert!(meta.is_spent(0), "Transaction #1 output #0 in the new block should be recorded as spent"); - assert!(meta.is_spent(2), "Transaction #1 output #2 in the new block should be recorded as spent"); - assert!(meta.is_spent(5), "Transaction #1 output #5 in the new block should be recorded as spent"); + assert!(meta.is_spent(0).unwrap(), "Transaction #1 output #0 in the new block should be recorded as spent"); + assert!(meta.is_spent(2).unwrap(), "Transaction #1 output #2 in the new block should be recorded as spent"); + assert!(meta.is_spent(5).unwrap(), "Transaction #1 output #5 in the new block should be recorded as spent"); - assert!(!meta.is_spent(1), "Transaction #1 output #1 in the new block should be recorded as unspent"); - assert!(!meta.is_spent(3), "Transaction #1 second #3 in the new block should be recorded as unspent"); + assert!(!meta.is_spent(1).unwrap(), "Transaction #1 output #1 in the new block should be recorded as unspent"); + assert!(!meta.is_spent(3).unwrap(), "Transaction #1 second #3 in the new block should be recorded as unspent"); } #[test] @@ -1133,15 +1166,15 @@ mod tests { // outputs 0, 1, 2 should be spent in the side chain branch // we reorganized to the side chain branch // so, outputs 0, 1, 2 should be spent - assert!(meta.is_spent(0)); - assert!(meta.is_spent(1)); - assert!(meta.is_spent(2)); + assert!(meta.is_spent(0).unwrap()); + assert!(meta.is_spent(1).unwrap()); + assert!(meta.is_spent(2).unwrap()); // outputs 3, 4 should not be spent in the side chain branch // we reorganized to the side chain branch // so, outputs 3, 4 should not be spent - assert!(!meta.is_spent(3)); - assert!(!meta.is_spent(4)); + assert!(!meta.is_spent(3).unwrap()); + assert!(!meta.is_spent(4).unwrap()); // Reorganizing back to main chain with 2 blocks in a row @@ -1169,15 +1202,15 @@ mod tests { // outputs 1, 3, 4 should be spent in the main branch // we reorganized to the main branch again after reorganized to side branch // so, outputs 1, 3, 4 should be spent - assert!(meta.is_spent(1)); - assert!(meta.is_spent(3)); - assert!(meta.is_spent(4)); + assert!(meta.is_spent(1).unwrap()); + assert!(meta.is_spent(3).unwrap()); + assert!(meta.is_spent(4).unwrap()); // outputs 0, 2 should not be spent in the main branch // we reorganized to the main branch again after reorganized to side branch // so, outputs 0, 2 should not be spent - assert!(!meta.is_spent(0)); - assert!(!meta.is_spent(2)); + assert!(!meta.is_spent(0).unwrap()); + assert!(!meta.is_spent(2).unwrap()); } // test simulates when main chain and side chain are competing all along, each adding @@ -1256,7 +1289,7 @@ mod tests { let genesis_meta = store.transaction_meta(&genesis_coinbase) .expect("Transaction meta for the genesis coinbase transaction should exist"); - assert!(genesis_meta.is_spent(0), "Genesis coinbase should be recorded as spent because block#1 transaction spends it"); + assert!(genesis_meta.is_spent(0).unwrap(), "Genesis coinbase should be recorded as spent because block#1 transaction spends it"); let mut update_context = UpdateContext::new(&store.database, &block_hash); store.decanonize_block(&mut update_context, &block_hash) @@ -1266,7 +1299,7 @@ mod tests { let genesis_meta = store.transaction_meta(&genesis_coinbase) .expect("Transaction meta for the genesis coinbase transaction should exist"); - assert!(!genesis_meta.is_spent(0), "Genesis coinbase should be recorded as unspent because we retracted block #1"); + assert!(!genesis_meta.is_spent(0).unwrap(), "Genesis coinbase should be recorded as unspent because we retracted block #1"); assert_eq!(store.block_number(&block_hash), None); } diff --git a/db/src/test_storage.rs b/db/src/test_storage.rs index 1c0739ce..c5b3928f 100644 --- a/db/src/test_storage.rs +++ b/db/src/test_storage.rs @@ -2,8 +2,8 @@ use super::{ BlockRef, Store, Error, BestBlock, BlockLocation, BlockInsertedChain, BlockProvider, - BlockStapler, TransactionMetaProvider, TransactionProvider, AsTransactionProvider, - IndexedBlock, BlockHeaderProvider, AsBlockHeaderProvider, + BlockStapler, TransactionMetaProvider, TransactionProvider, PreviousTransactionOutputProvider, + IndexedBlock, BlockHeaderProvider, }; use chain::{self, Block}; use primitives::hash::H256; @@ -84,12 +84,6 @@ impl BlockHeaderProvider for TestStorage { } } -impl AsBlockHeaderProvider for TestStorage { - fn as_block_header_provider(&self) -> &BlockHeaderProvider { - &*self - } -} - impl BlockProvider for TestStorage { fn block_number(&self, hash: &H256) -> Option { let data = self.data.read(); @@ -176,10 +170,12 @@ impl BlockStapler for TestStorage { _ => None } } + + fn flush(&self) { + } } impl TransactionProvider for TestStorage { - fn transaction_bytes(&self, hash: &H256) -> Option { self.transaction(hash).map(|tx| serialization::serialize(&tx)) } @@ -192,9 +188,10 @@ impl TransactionProvider for TestStorage { } } -impl AsTransactionProvider for TestStorage { - fn as_transaction_provider(&self) -> &TransactionProvider { - &*self +impl PreviousTransactionOutputProvider for TestStorage { + fn previous_transaction_output(&self, prevout: &chain::OutPoint) -> Option { + self.transaction(&prevout.hash) + .and_then(|tx| tx.outputs.into_iter().nth(prevout.index as usize)) } } diff --git a/db/src/transaction_meta.rs b/db/src/transaction_meta.rs index 8e9cfa15..ddbd9d67 100644 --- a/db/src/transaction_meta.rs +++ b/db/src/transaction_meta.rs @@ -69,8 +69,8 @@ impl TransactionMeta { self.block_height } - pub fn is_spent(&self, idx: usize) -> bool { - self.bits.get(idx + 1).expect("Index should be verified by the caller") + pub fn is_spent(&self, idx: usize) -> Option { + self.bits.get(idx + 1) //.expect("Index should be verified by the caller") } pub fn is_fully_spent(&self) -> bool { diff --git a/db/src/transaction_meta_provider.rs b/db/src/transaction_meta_provider.rs index a85c92da..a352b143 100644 --- a/db/src/transaction_meta_provider.rs +++ b/db/src/transaction_meta_provider.rs @@ -1,5 +1,10 @@ -use transaction_meta::TransactionMeta; use primitives::hash::H256; +use chain::OutPoint; +use transaction_meta::TransactionMeta; + +pub trait TransactionOutputObserver { + fn is_spent(&self, prevout: &OutPoint) -> Option; +} pub trait TransactionMetaProvider { /// get transaction metadata diff --git a/db/src/transaction_provider.rs b/db/src/transaction_provider.rs index 2d51269e..e1cc817f 100644 --- a/db/src/transaction_provider.rs +++ b/db/src/transaction_provider.rs @@ -3,7 +3,6 @@ use primitives::bytes::Bytes; use chain; pub trait TransactionProvider { - /// returns true if store contains given transaction fn contains_transaction(&self, hash: &H256) -> bool { self.transaction(hash).is_some() @@ -14,12 +13,6 @@ pub trait TransactionProvider { /// resolves serialized transaction info by transaction hash fn transaction(&self, hash: &H256) -> Option; - -} - -pub trait AsTransactionProvider { - /// returns `TransactionProvider` - fn as_transaction_provider(&self) -> &TransactionProvider; } /// During transaction the only part of old transaction that we need is `TransactionOutput`. @@ -27,7 +20,4 @@ pub trait AsTransactionProvider { /// a shame to clone the whole transaction just to get single output. pub trait PreviousTransactionOutputProvider { fn previous_transaction_output(&self, prevout: &chain::OutPoint) -> Option; - - // TODO: this should not be here, cause it requires meta data - fn is_spent(&self, prevout: &chain::OutPoint) -> bool; } diff --git a/db/src/update_context.rs b/db/src/update_context.rs index eb93e17f..be9aaaa5 100644 --- a/db/src/update_context.rs +++ b/db/src/update_context.rs @@ -28,7 +28,7 @@ impl UpdateContext { self.db_transaction.put(Some(COL_TRANSACTIONS_META), &*hash, &meta.into_bytes()); } - try!(db.write(self.db_transaction)); + db.write_buffered(self.db_transaction); trace!("Applied transaction for block {:?}", &self.target.to_reversed_str()); Ok(()) diff --git a/miner/src/fee.rs b/miner/src/fee.rs index c2045096..5a5c7e81 100644 --- a/miner/src/fee.rs +++ b/miner/src/fee.rs @@ -19,7 +19,7 @@ pub fn transaction_fee_rate(store: &TransactionProvider, transaction: &Transacti #[cfg(test)] mod tests { use std::sync::Arc; - use db::{TestStorage, AsTransactionProvider}; + use db::{TestStorage, AsSubstore}; use test_data; use super::*; diff --git a/miner/src/memory_pool.rs b/miner/src/memory_pool.rs index 0c113de4..72a20d0f 100644 --- a/miner/src/memory_pool.rs +++ b/miner/src/memory_pool.rs @@ -669,6 +669,11 @@ impl MemoryPool { self.storage.get_transactions_ids() } + /// Returns true if output was spent + pub fn is_spent(&self, prevout: &OutPoint) -> bool { + self.storage.is_output_spent(prevout) + } + fn make_entry(&mut self, t: Transaction) -> Entry { let hash = t.hash(); let ancestors = self.get_ancestors(&t); @@ -741,10 +746,6 @@ impl PreviousTransactionOutputProvider for MemoryPool { .and_then(|tx| tx.outputs.get(prevout.index as usize)) .cloned() } - - fn is_spent(&self, prevout: &OutPoint) -> bool { - self.storage.is_output_spent(prevout) - } } impl HeapSizeOf for MemoryPool { @@ -792,7 +793,6 @@ impl<'a> Iterator for MemoryPoolIterator<'a> { #[cfg(test)] mod tests { - use db::PreviousTransactionOutputProvider; use chain::{Transaction, OutPoint}; use heapsize::HeapSizeOf; use super::{MemoryPool, OrderingStrategy}; diff --git a/sync/src/orphan_transactions_pool.rs b/sync/src/orphan_transactions_pool.rs index 162830ec..fac813b3 100644 --- a/sync/src/orphan_transactions_pool.rs +++ b/sync/src/orphan_transactions_pool.rs @@ -46,6 +46,11 @@ impl OrphanTransactionsPool { &self.by_hash } + /// Check if pool contains this transaction + pub fn contains(&mut self, hash: &H256) -> bool { + self.by_hash.contains_key(hash) + } + /// Insert orphan transaction pub fn insert(&mut self, hash: H256, transaction: Transaction, unknown_parents: HashSet) { assert!(!self.by_hash.contains_key(&hash)); diff --git a/sync/src/synchronization_chain.rs b/sync/src/synchronization_chain.rs index b40e51ae..1a1299f6 100644 --- a/sync/src/synchronization_chain.rs +++ b/sync/src/synchronization_chain.rs @@ -195,6 +195,12 @@ impl Chain { &self.memory_pool } + /// Get mutable memory pool + #[cfg(test)] + pub fn memory_pool_mut(&mut self) -> &mut MemoryPool { + &mut self.memory_pool + } + /// Get number of blocks in given state pub fn length_of_blocks_state(&self, state: BlockState) -> u32 { match state { diff --git a/sync/src/synchronization_client.rs b/sync/src/synchronization_client.rs index 0392c02d..018fe265 100644 --- a/sync/src/synchronization_client.rs +++ b/sync/src/synchronization_client.rs @@ -766,6 +766,14 @@ impl ClientCore for SynchronizationClientCore where T: TaskExecutor { fn on_peer_transaction(&mut self, peer_index: usize, transaction: Transaction) -> Option> { let transaction_hash = transaction.hash(); + // check if this transaction is already known + if self.orphaned_transactions_pool.contains(&transaction_hash) || { + let chain = self.chain.read(); + chain.transaction_state(&transaction_hash) != TransactionState::Unknown + } { + return None; + } + // remember that peer has this transaction self.peers.on_transaction_received(peer_index, &transaction_hash); @@ -2914,4 +2922,17 @@ pub mod tests { // check that only one block (b2) is requested assert_eq!(sync.information().chain.requested, 1); } + + #[test] + fn when_got_same_orphan_transaction_twice() { + let (_, _, _, _, _, sync) = create_sync(None, None); + let mut sync = sync.lock(); + + sync.on_peer_transaction(1, test_data::TransactionBuilder::with_default_input(0).into()); + assert_eq!(sync.information().chain.transactions.transactions_count, 0); + assert_eq!(sync.information().orphaned_transactions, 1); + + // should not panic + sync.on_peer_transaction(1, test_data::TransactionBuilder::with_default_input(0).into()); + } } diff --git a/sync/src/synchronization_verifier.rs b/sync/src/synchronization_verifier.rs index 102bd85d..dd4e8ecb 100644 --- a/sync/src/synchronization_verifier.rs +++ b/sync/src/synchronization_verifier.rs @@ -7,7 +7,7 @@ use network::Magic; use primitives::hash::H256; use synchronization_chain::ChainRef; use verification::{ChainVerifier, Verify as VerificationVerify, Chain}; -use db::{SharedStore, IndexedBlock, PreviousTransactionOutputProvider}; +use db::{SharedStore, IndexedBlock, PreviousTransactionOutputProvider, TransactionOutputObserver}; use time::get_time; /// Block verification events sink @@ -153,7 +153,7 @@ impl Verifier for SyncVerifier where T: VerificationSink { } /// Execute single verification task -fn execute_verification_task(sink: &Arc, tx_output_provider: &U, verifier: &ChainVerifier, task: VerificationTask) { +fn execute_verification_task(sink: &Arc, tx_output_provider: &U, verifier: &ChainVerifier, task: VerificationTask) { let mut tasks_queue: VecDeque = VecDeque::new(); tasks_queue.push_back(task); @@ -198,11 +198,19 @@ impl ChainMemoryPoolTransactionOutputProvider { impl PreviousTransactionOutputProvider for ChainMemoryPoolTransactionOutputProvider { fn previous_transaction_output(&self, prevout: &OutPoint) -> Option { - self.chain.read().memory_pool().previous_transaction_output(prevout) + let chain = self.chain.read(); + chain.memory_pool().previous_transaction_output(prevout) + .or_else(|| chain.storage().as_previous_transaction_output_provider().previous_transaction_output(prevout)) } +} - fn is_spent(&self, prevout: &OutPoint) -> bool { - self.chain.read().storage().transaction_meta(&prevout.hash).and_then(|tm| Some(tm.is_spent(prevout.index as usize))).unwrap_or(false) +impl TransactionOutputObserver for ChainMemoryPoolTransactionOutputProvider { + fn is_spent(&self, prevout: &OutPoint) -> Option { + let chain = self.chain.read(); + if chain.memory_pool().is_spent(prevout) { + return Some(true); + } + chain.storage().transaction_meta(&prevout.hash).and_then(|tm| tm.is_spent(prevout.index as usize)) } } @@ -210,9 +218,11 @@ impl PreviousTransactionOutputProvider for EmptyTransactionOutputProvider { fn previous_transaction_output(&self, _prevout: &OutPoint) -> Option { None } +} - fn is_spent(&self, _prevout: &OutPoint) -> bool { - false +impl TransactionOutputObserver for EmptyTransactionOutputProvider { + fn is_spent(&self, _prevout: &OutPoint) -> Option { + None } } @@ -220,12 +230,15 @@ impl PreviousTransactionOutputProvider for EmptyTransactionOutputProvider { pub mod tests { use std::sync::Arc; use std::collections::HashMap; + use parking_lot::RwLock; use chain::Transaction; + use synchronization_chain::{Chain, ChainRef}; use synchronization_client::CoreVerificationSink; use synchronization_executor::tests::DummyTaskExecutor; use primitives::hash::H256; - use super::{Verifier, BlockVerificationSink, TransactionVerificationSink}; - use db::IndexedBlock; + use super::{Verifier, BlockVerificationSink, TransactionVerificationSink, ChainMemoryPoolTransactionOutputProvider}; + use db::{self, IndexedBlock}; + use test_data; #[derive(Default)] pub struct DummyVerifier { @@ -267,4 +280,19 @@ pub mod tests { } } } + + #[test] + fn when_transaction_spends_output_twice() { + use db::TransactionOutputObserver; + let tx1: Transaction = test_data::TransactionBuilder::with_default_input(0).into(); + let tx2: Transaction = test_data::TransactionBuilder::with_default_input(1).into(); + let out1 = tx1.inputs[0].previous_output.clone(); + let out2 = tx2.inputs[0].previous_output.clone(); + let mut chain = Chain::new(Arc::new(db::TestStorage::with_genesis_block())); + chain.memory_pool_mut().insert_verified(tx1); + let chain = ChainRef::new(RwLock::new(chain)); + let provider = ChainMemoryPoolTransactionOutputProvider::with_chain(chain); + assert!(provider.is_spent(&out1).unwrap_or_default()); + assert!(!provider.is_spent(&out2).unwrap_or_default()); + } } diff --git a/verification/src/chain_verifier.rs b/verification/src/chain_verifier.rs index 90477a87..fd29ee0d 100644 --- a/verification/src/chain_verifier.rs +++ b/verification/src/chain_verifier.rs @@ -3,7 +3,7 @@ use std::collections::BTreeSet; use scoped_pool::Pool; use hash::H256; -use db::{self, BlockLocation, PreviousTransactionOutputProvider, BlockHeaderProvider}; +use db::{self, BlockLocation, PreviousTransactionOutputProvider, BlockHeaderProvider, TransactionOutputObserver}; use network::{Magic, ConsensusParams}; use error::{Error, TransactionError}; use sigops::{StoreWithUnretainedOutputs, transaction_sigops}; @@ -168,7 +168,7 @@ impl ChainVerifier { time: u32, transaction: &chain::Transaction, sequence: usize - ) -> Result<(), TransactionError> where T: PreviousTransactionOutputProvider { + ) -> Result<(), TransactionError> where T: PreviousTransactionOutputProvider + TransactionOutputObserver { use script::{ TransactionInputSigner, @@ -194,7 +194,14 @@ impl ChainVerifier { _ => return Err(TransactionError::UnknownReference(input.previous_output.hash.clone())) }; - if prevout_provider.is_spent(&input.previous_output) { + // unwrap_or(false) is actually not right! + // but can be here because of two reasons + // - this function is not responsible for checking if previous transactions + // in currently processed block / mempool already spent this output + // - if we process transactions from mempool we shouldn't care if transactions before it + // spent this output, cause they may not make their way into the block due to their size + // or sigops limit + if prevout_provider.is_spent(&input.previous_output).unwrap_or(false) { return Err(TransactionError::UsingSpentOutput(input.previous_output.hash.clone(), input.previous_output.index)) } @@ -311,6 +318,7 @@ impl ChainVerifier { for task in transaction_tasks.iter_mut() { scope.execute(move || task.progress(self)) } + self.store.flush(); }); @@ -501,12 +509,12 @@ mod tests { .build(); storage.insert_block(&genesis).expect("Genesis should be inserted with no errors"); - let genesis_coinbase = genesis.transactions()[1].hash(); + let first_tx_hash = genesis.transactions()[1].hash(); let block = test_data::block_builder() .transaction().coinbase().build() .transaction() - .input().hash(genesis_coinbase).build() + .input().hash(first_tx_hash).build() .output().value(30).build() .output().value(20).build() .build() diff --git a/verification/src/sigops.rs b/verification/src/sigops.rs index e9aa6594..a47f7aef 100644 --- a/verification/src/sigops.rs +++ b/verification/src/sigops.rs @@ -22,10 +22,6 @@ impl<'a, T> PreviousTransactionOutputProvider for StoreWithUnretainedOutputs<'a, .and_then(|tx| tx.outputs.into_iter().nth(prevout.index as usize)) .or_else(|| self.outputs.previous_transaction_output(prevout)) } - - fn is_spent(&self, _prevout: &OutPoint) -> bool { - unimplemented!(); - } } pub fn transaction_sigops(