Merge pull request #218 from ethcore/previous_transaction_output_provider

initial work on b82, abstraction for verify_transaction, use Previous…

chain/src/block.rs

@@ -63,10 +63,6 @@ impl Block {
 		&self.block_header
 	}
 
-	pub fn drain(self) -> (BlockHeader, Vec<Transaction>) {
-		(self.block_header, self.transactions)
-	}
-
 	pub fn hash(&self) -> H256 {
 		self.block_header.hash()
 	}

db/src/indexed_block.rs

@@ -1,6 +1,7 @@
 use chain;
 use primitives::hash::H256;
 use serialization::Serializable;
+use PreviousTransactionOutputProvider;
 
 #[derive(Debug)]
 pub struct IndexedBlock {
@@ -11,19 +12,24 @@ pub struct IndexedBlock {
 	transaction_hashes: Vec<H256>,
 }
 
+impl PreviousTransactionOutputProvider for IndexedBlock {
+	fn previous_transaction_output(&self, prevout: &chain::OutPoint) -> Option<chain::TransactionOutput> {
+		self.transaction(&prevout.hash)
+			.and_then(|tx| tx.outputs.get(prevout.index as usize))
+			.cloned()
+	}
+}
+
 impl From<chain::Block> for IndexedBlock {
 	fn from(block: chain::Block) -> Self {
-		let (org_header, org_txs) = block.drain();
-		let mut hashes = Vec::with_capacity(org_txs.len());
-		for tx in org_txs.iter() {
-			hashes.push(tx.hash())
-		}
-		let header_hash = org_header.hash();
+		let chain::Block { block_header, transactions } = block;
+		let header_hash = block_header.hash();
+		let hashes = transactions.iter().map(chain::Transaction::hash).collect();
 
 		IndexedBlock {
-			header: org_header,
+			header: block_header,
 			header_hash: header_hash,
-			transactions: org_txs,
+			transactions: transactions,
 			transaction_hashes: hashes,
 		}
 	}
@@ -46,6 +52,12 @@ impl IndexedBlock {
 		block
 	}
 
+	pub fn transaction(&self, hash: &H256) -> Option<&chain::Transaction> {
+		self.transaction_hashes.iter()
+			.position(|x| x == hash)
+			.map(|position| &self.transactions[position])
+	}
+
 	pub fn transactions(&self) -> IndexedTransactions {
 		IndexedTransactions {
 			position: 0,

db/src/lib.rs

@@ -58,7 +58,7 @@ pub use best_block::BestBlock;
 pub use storage::{Storage, Store};
 pub use error::Error;
 pub use kvdb::Database;
-pub use transaction_provider::{TransactionProvider, AsTransactionProvider};
+pub use transaction_provider::{TransactionProvider, AsTransactionProvider, PreviousTransactionOutputProvider};
 pub use transaction_meta_provider::TransactionMetaProvider;
 pub use block_stapler::{BlockStapler, BlockInsertedChain};
 pub use block_provider::BlockProvider;

db/src/storage.rs

@@ -151,14 +151,8 @@ impl Storage {
 
 	fn block_transactions_by_hash(&self, h: &H256) -> Vec<chain::Transaction> {
 		self.block_transaction_hashes_by_hash(h)
-			.into_iter()
-			.filter_map(|tx_hash| {
-				self.transaction_bytes(&tx_hash)
-					.map(|tx_bytes| {
-						deserialize::<_, chain::Transaction>(tx_bytes.as_ref())
-							.expect("Error deserializing transaction, possible db corruption")
-					})
-			})
+			.iter()
+			.map(|tx_hash| self.transaction(tx_hash).expect("Missing transaction, possible db corruption"))
 			.collect()
 	}
 
@@ -169,22 +163,12 @@ impl Storage {
 	}
 
 	fn block_by_hash(&self, h: &H256) -> Option<IndexedBlock> {
-
 		self.block_header_by_hash(h).map(|header| {
-			let tx_index =
-				self.block_transaction_hashes_by_hash(h)
-					.into_iter()
-					.filter_map(|tx_hash| {
-						self.transaction_bytes(&tx_hash)
-							.map(|tx_bytes| {
-								(
-									tx_hash,
-									deserialize::<_, chain::Transaction>(tx_bytes.as_ref())
-										.expect("Error deserializing transaction, possible db corruption"),
-								)
-							})
-					})
-					.collect();
+			let tx_hashes = self.block_transaction_hashes_by_hash(h);
+			let txs = tx_hashes.iter()
+				.map(|tx_hash| self.transaction(tx_hash).expect("Missing transaction, possible db corruption"))
+				.collect::<Vec<_>>();
+			let tx_index = tx_hashes.into_iter().zip(txs.into_iter()).collect();
 			IndexedBlock::new(header, tx_index)
 		})
 	}

db/src/transaction_provider.rs

@@ -21,3 +21,10 @@ pub trait AsTransactionProvider {
 	/// returns `TransactionProvider`
 	fn as_transaction_provider(&self) -> &TransactionProvider;
 }
+
+/// During transaction 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 {
+	fn previous_transaction_output(&self, prevout: &chain::OutPoint) -> Option<chain::TransactionOutput>;
+}

miner/src/memory_pool.rs

@@ -5,10 +5,10 @@
 //! 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;
+use db::{TransactionProvider, PreviousTransactionOutputProvider};
 use primitives::bytes::Bytes;
 use primitives::hash::H256;
-use chain::Transaction;
+use chain::{Transaction, OutPoint, TransactionOutput};
 use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::collections::HashSet;
@@ -705,6 +705,14 @@ impl TransactionProvider for MemoryPool {
 	}
 }
 
+impl PreviousTransactionOutputProvider for MemoryPool {
+	fn previous_transaction_output(&self, prevout: &OutPoint) -> Option<TransactionOutput> {
+		self.get(&prevout.hash)
+			.and_then(|tx| tx.outputs.get(prevout.index as usize))
+			.cloned()
+	}
+}
+
 impl HeapSizeOf for MemoryPool {
 	fn heap_size_of_children(&self) -> usize {
 		self.storage.heap_size_of_children()

verification/src/chain_verifier.rs

@@ -1,7 +1,7 @@
 //! Bitcoin chain verifier
 
 use std::collections::BTreeSet;
-use db::{self, BlockLocation};
+use db::{self, BlockLocation, PreviousTransactionOutputProvider};
 use network::Magic;
 use script::Script;
 use super::{Verify, VerificationResult, Chain, Error, TransactionError};
@@ -63,11 +63,11 @@ impl ChainVerifier {
 
 	/// Returns previous transaction output.
 	/// NOTE: This function expects all previous blocks to be already in database.
-	fn previous_transaction_output(&self, block: &db::IndexedBlock, prevout: &chain::OutPoint) -> Option<chain::TransactionOutput> {
+	fn previous_transaction_output<T>(&self, prevout_provider: &T, prevout: &chain::OutPoint) -> Option<chain::TransactionOutput>
+		where T: PreviousTransactionOutputProvider {
 		self.store.transaction(&prevout.hash)
-			.as_ref()
-			.or_else(|| block.transactions().find(|&(hash, _)| hash == &prevout.hash).and_then(|(_, tx)| Some(tx)))
-			.and_then(|tx| tx.outputs.iter().nth(prevout.index as usize).cloned())
+			.and_then(|tx| tx.outputs.into_iter().nth(prevout.index as usize))
+			.or_else(|| prevout_provider.previous_transaction_output(prevout))
 	}
 
 	/// Returns number of transaction signature operations.
@@ -181,11 +181,14 @@ impl ChainVerifier {
 		Ok(())
 	}
 
-	pub fn verify_transaction(&self,
-		block: &db::IndexedBlock,
+		//block: &db::IndexedBlock,
+	pub fn verify_transaction<T>(
+		&self,
+		prevout_provider: &T,
 		transaction: &chain::Transaction,
-		sequence: usize,
-	) -> Result<(), TransactionError> {
+		sequence: usize
+	) -> Result<(), TransactionError> where T: PreviousTransactionOutputProvider {
+
 		use script::{
 			TransactionInputSigner,
 			TransactionSignatureChecker,
@@ -204,7 +207,7 @@ impl ChainVerifier {
 		for (input_index, input) in transaction.inputs().iter().enumerate() {
 			// signature verification
 			let signer: TransactionInputSigner = transaction.clone().into();
-			let paired_output = match self.previous_transaction_output(block, &input.previous_output) {
+			let paired_output = match self.previous_transaction_output(prevout_provider, &input.previous_output) {
 				Some(output) => output,
 				_ => return Err(TransactionError::UnknownReference(input.previous_output.hash.clone()))
 			};