Minimal Transaction Pool (#1067)

* verify a tx like we verify a block (experimental)

* first minimal_pool test up and running but not testing what we need to

* rework tx_pool validation to use txhashset extension

* minimal tx pool wired up but rough

* works locally (rough statew though)
delete "legacy" pool and graph code

* rework the new pool into TransactionPool and Pool impls

* rework pool to store pool entries
with associated timer and source etc.

* all_transactions

* extra_txs so we can validate stempool against existing txpool

* rework reconcile_block

* txhashset apply_raw_tx can now rewind to a checkpoint (prev raw tx)

* wip - txhashset tx tests

* more flexible rewind on MMRs

* add tests to cover apply_raw_txs on txhashset extension

* add_to_stempool and add_to_txpool

* deaggregate multi kernel tx when adding to txpoool

* handle freshness in stempool
handle propagation of stempool txs via dandelion monitor

* patience timer and fluff if we cannot propagate
to next relay

* aggregate and fluff stempool is we have no relay

* refactor coinbase maturity

* rewrote basic tx pool tests to use a real txhashset via chain adapter

* rework dandelion monitor to reflect recent discussion
works locally but needs a cleanup

* refactor dandelion_monitor - split out phases

* more pool test coverage

* remove old test code from pool (still wip)

* block_building and block_reconciliation tests

* tracked down chain test failure...

* fix test_coinbase_maturity

* dandelion_monitor now runs...

* refactor dandelion config, shared across p2p and pool components

* fix pool tests with new config

* fix p2p tests

* rework tx pool to deal with duplicate commitments (testnet2 limitation)

* cleanup and address some PR feedback

* add big comment about pre_tx...

core/src/core/block.rs

@@ -14,22 +14,22 @@
 
 //! Blocks and blockheaders
 
-use time;
 use rand::{thread_rng, Rng};
 use std::collections::HashSet;
+use time;
 
-use core::{Commitment, Committed, Input, KernelFeatures, Output, OutputFeatures, Proof, ShortId,
-           Transaction, TxKernel};
 use consensus;
 use consensus::{exceeds_weight, reward, VerifySortOrder, REWARD};
 use core::hash::{Hash, HashWriter, Hashed, ZERO_HASH};
 use core::id::ShortIdentifiable;
 use core::target::Difficulty;
 use core::transaction;
-use ser::{self, read_and_verify_sorted, Readable, Reader, Writeable, WriteableSorted, Writer};
+use core::{Commitment, Committed, Input, KernelFeatures, Output, OutputFeatures, Proof, ShortId,
+           Transaction, TxKernel};
 use global;
 use keychain;
 use keychain::BlindingFactor;
+use ser::{self, read_and_verify_sorted, Readable, Reader, Writeable, WriteableSorted, Writer};
 use util::LOGGER;
 use util::{secp, static_secp_instance};
 
@@ -222,9 +222,9 @@ impl BlockHeader {
 
 /// Compact representation of a full block.
 /// Each input/output/kernel is represented as a short_id.
-/// A node is reasonably likely to have already seen all tx data (tx broadcast before block)
-/// and can go request missing tx data from peers if necessary to hydrate a compact block
-/// into a full block.
+/// A node is reasonably likely to have already seen all tx data (tx broadcast
+/// before block) and can go request missing tx data from peers if necessary to
+/// hydrate a compact block into a full block.
 #[derive(Debug, Clone)]
 pub struct CompactBlock {
 	/// The header with metadata and commitments to the rest of the data
@@ -240,9 +240,9 @@ pub struct CompactBlock {
 	pub kern_ids: Vec<ShortId>,
 }
 
-/// Implementation of Writeable for a compact block, defines how to write the block to a
-/// binary writer. Differentiates between writing the block for the purpose of
-/// full serialization and the one of just extracting a hash.
+/// Implementation of Writeable for a compact block, defines how to write the
+/// block to a binary writer. Differentiates between writing the block for the
+/// purpose of full serialization and the one of just extracting a hash.
 impl Writeable for CompactBlock {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
 		try!(self.header.write(writer));
@@ -271,8 +271,8 @@ impl Writeable for CompactBlock {
 	}
 }
 
-/// Implementation of Readable for a compact block, defines how to read a compact block
-/// from a binary stream.
+/// Implementation of Readable for a compact block, defines how to read a
+/// compact block from a binary stream.
 impl Readable for CompactBlock {
 	fn read(reader: &mut Reader) -> Result<CompactBlock, ser::Error> {
 		let header = try!(BlockHeader::read(reader));
@@ -400,7 +400,7 @@ impl Block {
 	///
 	pub fn new(
 		prev: &BlockHeader,
-		txs: Vec<&Transaction>,
+		txs: Vec<Transaction>,
 		difficulty: Difficulty,
 		reward_output: (Output, TxKernel),
 	) -> Result<Block, Error> {
@@ -409,7 +409,8 @@ impl Block {
 	}
 
 	/// Hydrate a block from a compact block.
-	/// Note: caller must validate the block themselves, we do not validate it here.
+	/// Note: caller must validate the block themselves, we do not validate it
+	/// here.
 	pub fn hydrate_from(cb: CompactBlock, txs: Vec<Transaction>) -> Block {
 		trace!(
 			LOGGER,
@@ -495,7 +496,7 @@ impl Block {
 	/// that all transactions are valid and calculates the Merkle tree.
 	pub fn with_reward(
 		prev: &BlockHeader,
-		txs: Vec<&Transaction>,
+		txs: Vec<Transaction>,
 		reward_out: Output,
 		reward_kern: TxKernel,
 		difficulty: Difficulty,
@@ -670,10 +671,11 @@ impl Block {
 	}
 
 	/// We can verify the Merkle proof (for coinbase inputs) here in isolation.
-	/// But we cannot check the following as we need data from the index and the PMMR.
-	/// So we must be sure to check these at the appropriate point during block validation.
-	///   * node is in the correct pos in the PMMR
-	///   * block is the correct one (based on output_root from block_header via the index)
+	/// But we cannot check the following as we need data from the index and
+	/// the PMMR. So we must be sure to check these at the appropriate point
+	/// during block validation.   * node is in the correct pos in the PMMR
+	/// * block is the correct one (based on output_root from block_header
+	/// via the index)
 	fn verify_inputs(&self) -> Result<(), Error> {
 		let coinbase_inputs = self.inputs
 			.iter()
@@ -733,7 +735,8 @@ impl Block {
 		Ok((io_sum, kernel_sum))
 	}
 
-	/// Validate the coinbase outputs generated by miners. Entails 2 main checks:
+	/// Validate the coinbase outputs generated by miners. Entails 2 main
+	/// checks:
 	///
 	/// * That the sum of all coinbase-marked outputs equal the supply.
 	/// * That the sum of blinding factors for all coinbase-marked outputs match

core/src/core/pmmr.rs

@@ -35,12 +35,12 @@
 //! The underlying Hashes are stored in a Backend implementation that can
 //! either be a simple Vec or a database.
 
-use std::clone::Clone;
-use std::marker;
 use core::hash::Hash;
 use ser;
-use ser::{Readable, Reader, Writeable, Writer};
 use ser::{PMMRIndexHashable, PMMRable};
+use ser::{Readable, Reader, Writeable, Writer};
+use std::clone::Clone;
+use std::marker;
 use util;
 use util::LOGGER;
 
@@ -52,9 +52,10 @@ pub trait Backend<T>
 where
 	T: PMMRable,
 {
-	/// Append the provided Hashes to the backend storage, and optionally an associated
-	/// data element to flatfile storage (for leaf nodes only). The position of the
-	/// first element of the Vec in the MMR is provided to help the implementation.
+	/// Append the provided Hashes to the backend storage, and optionally an
+	/// associated data element to flatfile storage (for leaf nodes only). The
+	/// position of the first element of the Vec in the MMR is provided to
+	/// help the implementation.
 	fn append(&mut self, position: u64, data: Vec<(Hash, Option<T>)>) -> Result<(), String>;
 
 	/// Rewind the backend state to a previous position, as if all append
@@ -102,8 +103,8 @@ pub const MAX_PATH: u64 = 100;
 /// Proves inclusion of an output (node) in the output MMR.
 /// We can use this to prove an output was unspent at the time of a given block
 /// as the root will match the output_root of the block header.
-/// The path and left_right can be used to reconstruct the peak hash for a given tree
-/// in the MMR.
+/// The path and left_right can be used to reconstruct the peak hash for a
+/// given tree in the MMR.
 /// The root is the result of hashing all the peaks together.
 #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Serialize, Deserialize)]
 pub struct MerkleProof {
@@ -207,8 +208,9 @@ impl MerkleProof {
 
 	/// Verify the Merkle proof.
 	/// We do this by verifying the folloiwing -
-	///  * inclusion of the node beneath a peak (via the Merkle path/branch of siblings)
-	///  * inclusion of the peak in the "bag of peaks" beneath the root
+	/// * inclusion of the node beneath a peak (via the Merkle path/branch of
+	/// siblings) * inclusion of the peak in the "bag of peaks" beneath the
+	/// root
 	pub fn verify(&self) -> bool {
 		// if we have no further elements in the path
 		// then this proof verifies successfully if our node is
@@ -455,7 +457,6 @@ where
 				break;
 			}
 		}
-
 		self.backend.remove(to_prune, index)?;
 		Ok(true)
 	}
@@ -937,8 +938,9 @@ pub fn bintree_postorder_height(num: u64) -> u64 {
 }
 
 /// Is this position a leaf in the MMR?
-/// We know the positions of all leaves based on the postorder height of an MMR of any size
-/// (somewhat unintuitively but this is how the PMMR is "append only").
+/// We know the positions of all leaves based on the postorder height of an MMR
+/// of any size (somewhat unintuitively but this is how the PMMR is "append
+/// only").
 pub fn is_leaf(pos: u64) -> bool {
 	bintree_postorder_height(pos) == 0
 }
@@ -1037,9 +1039,9 @@ fn most_significant_pos(num: u64) -> u64 {
 #[cfg(test)]
 mod test {
 	use super::*;
-	use ser::{Error, Readable, Writeable};
-	use core::{Reader, Writer};
 	use core::hash::Hash;
+	use core::{Reader, Writer};
+	use ser::{Error, Readable, Writeable};
 	use ser::{PMMRIndexHashable, PMMRable};
 
 	/// Simple MMR backend implementation based on a Vector. Pruning does not

core/src/core/transaction.rs

@@ -13,11 +13,13 @@
 // limitations under the License.
 
 //! Transactions
+
 use std::cmp::Ordering;
 use std::cmp::max;
 use std::collections::HashSet;
 use std::io::Cursor;
 use std::{error, fmt};
+
 use util::secp::pedersen::{Commitment, ProofMessage, RangeProof};
 use util::secp::{self, Message, Signature};
 use util::{kernel_sig_msg, static_secp_instance};
@@ -34,7 +36,6 @@ use keychain::BlindingFactor;
 use ser::{self, read_and_verify_sorted, ser_vec, PMMRable, Readable, Reader, Writeable,
           WriteableSorted, Writer};
 use util;
-use util::LOGGER;
 
 bitflags! {
 	/// Options for a kernel's structure or use
@@ -74,6 +75,9 @@ pub enum Error {
 	/// Returns if the value hidden within the a RangeProof message isn't
 	/// repeated 3 times, indicating it's incorrect
 	InvalidProofMessage,
+	/// Error when sums do not verify correctly during tx aggregation.
+	/// Likely a "double spend" across two unconfirmed txs.
+	AggregationError,
 }
 
 impl error::Error for Error {
@@ -391,24 +395,30 @@ impl Transaction {
 			.fold(0, |acc, ref x| max(acc, x.lock_height))
 	}
 
-	/// To verify transaction kernels we check that -
-	///  * all kernels have an even fee
-	/// * sum of input/output commitments matches sum of kernel commitments
-	/// after applying offset * each kernel sig is valid (i.e. tx commitments
-	/// sum to zero, given above is true)
-	fn verify_kernels(&self) -> Result<(), Error> {
-		// Verify all the output rangeproofs.
-		// Note: this is expensive.
-		for x in &self.outputs {
-			x.verify_proof()?;
-		}
-
+	fn verify_kernel_signatures(&self) -> Result<(), Error> {
 		// Verify the kernel signatures.
 		// Note: this is expensive.
 		for x in &self.kernels {
 			x.verify()?;
 		}
+		Ok(())
+	}
 
+	fn verify_rangeproofs(&self) -> Result<(), Error> {
+		// Verify all the output rangeproofs.
+		// Note: this is expensive.
+		for x in &self.outputs {
+			x.verify_proof()?;
+		}
+		Ok(())
+	}
+
+	/// To verify transaction kernels we check that -
+	///  * all kernels have an even fee
+	/// * sum of input/output commitments matches sum of kernel commitments
+	/// after applying offset * each kernel sig is valid (i.e. tx commitments
+	/// sum to zero, given above is true)
+	fn verify_kernel_sums(&self) -> Result<(), Error> {
 		// Sum all input|output|overage commitments.
 		let overage = self.fee() as i64;
 		let io_sum = self.sum_commitments(overage, None)?;
@@ -433,8 +443,9 @@ impl Transaction {
 			return Err(Error::TooManyInputs);
 		}
 		self.verify_sorted()?;
-		self.verify_inputs()?;
-		self.verify_kernels()?;
+		self.verify_kernel_sums()?;
+		self.verify_rangeproofs()?;
+		self.verify_kernel_signatures()?;
 
 		Ok(())
 	}
@@ -472,32 +483,11 @@ impl Transaction {
 		self.kernels.verify_sort_order()?;
 		Ok(())
 	}
-
-	/// We can verify the Merkle proof (for coinbase inputs) here in isolation.
-	/// But we cannot check the following as we need data from the index and
-	/// the PMMR. So we must be sure to check these at the appropriate point
-	/// during block validation.   * node is in the correct pos in the PMMR
-	/// * block is the correct one (based on output_root from block_header
-	/// via the index)
-	fn verify_inputs(&self) -> Result<(), Error> {
-		let coinbase_inputs = self.inputs
-			.iter()
-			.filter(|x| x.features.contains(OutputFeatures::COINBASE_OUTPUT));
-
-		for input in coinbase_inputs {
-			let merkle_proof = input.merkle_proof();
-			if !merkle_proof.verify() {
-				return Err(Error::MerkleProof);
-			}
-		}
-
-		Ok(())
-	}
 }
 
 /// Aggregate a vec of transactions into a multi-kernel transaction with
 /// cut_through
-pub fn aggregate_with_cut_through(transactions: Vec<Transaction>) -> Result<Transaction, Error> {
+pub fn aggregate(transactions: Vec<Transaction>) -> Result<Transaction, Error> {
 	let mut inputs: Vec<Input> = vec![];
 	let mut outputs: Vec<Output> = vec![];
 	let mut kernels: Vec<TxKernel> = vec![];
@@ -565,58 +555,14 @@ pub fn aggregate_with_cut_through(transactions: Vec<Transaction>) -> Result<Tran
 	new_outputs.sort();
 	kernels.sort();
 
-	let tx = Transaction::new(new_inputs, new_outputs, kernels);
+	let tx = Transaction::new(new_inputs, new_outputs, kernels).with_offset(total_kernel_offset);
 
-	Ok(tx.with_offset(total_kernel_offset))
-}
+	// We need to check sums here as aggregation/cut-through may have created an
+	// invalid tx.
+	tx.verify_kernel_sums()
+		.map_err(|_| Error::AggregationError)?;
 
-/// Aggregate a vec of transactions into a multi-kernel transaction
-pub fn aggregate(transactions: Vec<Transaction>) -> Result<Transaction, Error> {
-	let mut inputs: Vec<Input> = vec![];
-	let mut outputs: Vec<Output> = vec![];
-	let mut kernels: Vec<TxKernel> = vec![];
-
-	// we will sum these together at the end to give us the overall offset for the
-	// transaction
-	let mut kernel_offsets = vec![];
-
-	for mut transaction in transactions {
-		// we will summ these later to give a single aggregate offset
-		kernel_offsets.push(transaction.offset);
-
-		inputs.append(&mut transaction.inputs);
-		outputs.append(&mut transaction.outputs);
-		kernels.append(&mut transaction.kernels);
-	}
-
-	// now sum the kernel_offsets up to give us an aggregate offset for the
-	// transaction
-	let total_kernel_offset = {
-		let secp = static_secp_instance();
-		let secp = secp.lock().unwrap();
-		let mut keys = kernel_offsets
-			.iter()
-			.cloned()
-			.filter(|x| *x != BlindingFactor::zero())
-			.filter_map(|x| x.secret_key(&secp).ok())
-			.collect::<Vec<_>>();
-
-		if keys.is_empty() {
-			BlindingFactor::zero()
-		} else {
-			let sum = secp.blind_sum(keys, vec![])?;
-			BlindingFactor::from_secret_key(sum)
-		}
-	};
-
-	// sort them lexicographically
-	inputs.sort();
-	outputs.sort();
-	kernels.sort();
-
-	let tx = Transaction::new(inputs, outputs, kernels);
-
-	Ok(tx.with_offset(total_kernel_offset))
+	Ok(tx)
 }
 
 /// Attempt to deaggregate a multi-kernel transaction based on multiple
@@ -855,11 +801,6 @@ impl Input {
 			if lock_height > height {
 				return Err(Error::ImmatureCoinbase);
 			}
-
-			debug!(
-				LOGGER,
-				"input: verify_maturity: success via Merkle proof: {} vs {}", lock_height, height,
-			);
 		}
 		Ok(())
 	}
@@ -1206,11 +1147,9 @@ mod test {
 		let key_id = keychain.derive_key_id(1).unwrap();
 
 		let commit = keychain.commit(1003, &key_id).unwrap();
-		println!("commit: {:?}", commit);
 		let key_id = keychain.derive_key_id(1).unwrap();
 
 		let commit_2 = keychain.commit(1003, &key_id).unwrap();
-		println!("commit2 : {:?}", commit_2);
 
 		assert!(commit == commit_2);
 	}