simplify verification code

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<IndexedTransaction> {
-	pub fn new(transactions: &'a [T]) -> Self {
-		IndexedTransactionsRef {
-			transactions: transactions,
-		}
-	}
-}

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;

db/src/block_chain_db.rs

@@ -473,9 +473,10 @@ impl<T> PreviousTransactionOutputProvider for BlockChainDatabase<T> where T: Key
 }
 
 impl<T> TransactionOutputObserver for BlockChainDatabase<T> where T: KeyValueDatabase {
-	fn is_spent(&self, prevout: &OutPoint) -> Option<bool> {
+	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)
 	}
 }
 

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<IndexedTransaction> + Send + Sync {
-	fn previous_transaction_output(&self, prevout: &OutPoint, transaction_index: usize) -> Option<TransactionOutput> {
-		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<TransactionOutput> {
+	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<TransactionOutput> {
-		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<bool> {
-		// 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)
 	}
 }

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<bool>;
+	/// Returns true if we know that output has been spent
+	fn is_spent(&self, prevout: &OutPoint) -> bool;
 }
 
 /// Transaction meta provider stores transaction meta information

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<IndexedTransaction> = Vec::new();
-		let store_ref = IndexedTransactionsRef::new(&store);
-		let entries: Vec<Entry> = 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::<Vec<_>>().is_empty());
-	}
-
 	#[test]
 	fn test_fitting_transactions_iterator_max_block_size_reached() {
 	}

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<bool> {
+	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);

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);
 

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(),
 		}

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 {

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

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<bool> {
-		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<bool> {
-		None
+	fn is_spent(&self, _prevout: &OutPoint) -> bool {
+		false
 	}
 }

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::<HashSet<_>>();
 		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(())

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)

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 {