txhashset extension now implements Committed (#1049)

* txhashset now implements committed for consistency

* rustfmt and cleanup

chain/src/txhashset.rs

@@ -26,7 +26,8 @@ use util::static_secp_instance;
 use util::secp::pedersen::{Commitment, RangeProof};
 
 use core::consensus::REWARD;
-use core::core::{Block, BlockHeader, Input, Output, OutputFeatures, OutputIdentifier, TxKernel};
+use core::core::{Block, BlockHeader, Committed, Input, Output, OutputFeatures, OutputIdentifier,
+                 TxKernel};
 use core::core::pmmr::{self, MerkleProof, PMMR};
 use core::global;
 use core::core::hash::{Hash, Hashed};
@@ -348,6 +349,36 @@ pub struct Extension<'a> {
 	rollback: bool,
 }
 
+impl<'a> Committed for Extension<'a> {
+	fn inputs_committed(&self) -> Vec<Commitment> {
+		vec![]
+	}
+
+	fn outputs_committed(&self) -> Vec<Commitment> {
+		let mut commitments = vec![];
+		for n in 1..self.output_pmmr.unpruned_size() + 1 {
+			if pmmr::is_leaf(n) {
+				if let Some(out) = self.output_pmmr.get_data(n) {
+					commitments.push(out.commit);
+				}
+			}
+		}
+		commitments
+	}
+
+	fn kernels_committed(&self) -> Vec<Commitment> {
+		let mut commitments = vec![];
+		for n in 1..self.kernel_pmmr.unpruned_size() + 1 {
+			if pmmr::is_leaf(n) {
+				if let Some(kernel) = self.kernel_pmmr.get_data(n) {
+					commitments.push(kernel.excess);
+				}
+			}
+		}
+		commitments
+	}
+}
+
 impl<'a> Extension<'a> {
 	// constructor
 	fn new(trees: &'a mut TxHashSet, commit_index: Arc<ChainStore>) -> Extension<'a> {
@@ -634,18 +665,6 @@ impl<'a> Extension<'a> {
 	pub fn validate_sums(&self, header: &BlockHeader) -> Result<((Commitment, Commitment)), Error> {
 		let now = Instant::now();
 
-		// supply is the sum of the coinbase outputs from each block header
-		let supply_commit = {
-			let secp = static_secp_instance();
-			let secp = secp.lock().unwrap();
-			let supply = header.height * REWARD;
-			secp.commit_value(supply)?
-		};
-		let output_sum = self.sum_outputs(supply_commit)?;
-		let kernel_sum = self.sum_kernels()?;
-
-		let zero_commit = secp_static::commit_to_zero_value();
-
 		let offset = {
 			let secp = static_secp_instance();
 			let secp = secp.lock().unwrap();
@@ -653,14 +672,13 @@ impl<'a> Extension<'a> {
 			secp.commit(0, key)?
 		};
 
-		let mut excesses = vec![kernel_sum, offset];
-		excesses.retain(|x| *x != zero_commit);
+		// Treat the total "supply" as one huge overage that needs to be accounted for.
+		// If we have a supply of 6,000 grin then we should
+		// have a corresponding 6,000 grin in unspent outputs.
+		let supply = ((header.height * REWARD) as i64).checked_neg().unwrap_or(0);
+		let output_sum = self.sum_commitments(supply, None)?;
 
-		let kernel_sum_plus_offset = {
-			let secp = static_secp_instance();
-			let secp = secp.lock().unwrap();
-			secp.commit_sum(excesses, vec![])?
-		};
+		let (kernel_sum, kernel_sum_plus_offset) = self.sum_kernel_excesses(&offset, None)?;
 
 		if output_sum != kernel_sum_plus_offset {
 			return Err(Error::InvalidTxHashSet(
@@ -775,37 +793,6 @@ impl<'a> Extension<'a> {
 		)
 	}
 
-	/// Sums the excess of all our kernels.
-	fn sum_kernels(&self) -> Result<Commitment, Error> {
-		let now = Instant::now();
-
-		let mut commitments = vec![];
-		for n in 1..self.kernel_pmmr.unpruned_size() + 1 {
-			if pmmr::is_leaf(n) {
-				if let Some(kernel) = self.kernel_pmmr.get_data(n) {
-					commitments.push(kernel.excess);
-				}
-			}
-		}
-
-		let kern_count = commitments.len();
-
-		let kernel_sum = {
-			let secp = static_secp_instance();
-			let secp = secp.lock().unwrap();
-			secp.commit_sum(commitments, vec![])?
-		};
-
-		debug!(
-			LOGGER,
-			"txhashset: summed {} kernels, pmmr size {}, took {}s",
-			kern_count,
-			self.kernel_pmmr.unpruned_size(),
-			now.elapsed().as_secs(),
-		);
-		Ok(kernel_sum)
-	}
-
 	fn verify_kernel_signatures(&self) -> Result<(), Error> {
 		let now = Instant::now();
 
@@ -863,38 +850,6 @@ impl<'a> Extension<'a> {
 		);
 		Ok(())
 	}
-
-	/// Sums all our unspent output commitments.
-	fn sum_outputs(&self, supply_commit: Commitment) -> Result<Commitment, Error> {
-		let now = Instant::now();
-
-		let mut commitments = vec![];
-		for n in 1..self.output_pmmr.unpruned_size() + 1 {
-			if pmmr::is_leaf(n) {
-				if let Some(out) = self.output_pmmr.get_data(n) {
-					commitments.push(out.commit);
-				}
-			}
-		}
-
-		let commit_count = commitments.len();
-
-		let output_sum = {
-			let secp = static_secp_instance();
-			let secp = secp.lock().unwrap();
-			secp.commit_sum(commitments, vec![supply_commit])?
-		};
-
-		debug!(
-			LOGGER,
-			"txhashset: summed {} outputs, pmmr size {}, took {}s",
-			commit_count,
-			self.output_pmmr.unpruned_size(),
-			now.elapsed().as_secs(),
-		);
-
-		Ok(output_sum)
-	}
 }
 
 /// Packages the txhashset data files into a zip and returns a Read to the

core/src/core/block.rs

@@ -32,7 +32,7 @@ use keychain;
 use keychain::BlindingFactor;
 use util::kernel_sig_msg;
 use util::LOGGER;
-use util::{secp, secp_static, static_secp_instance};
+use util::{secp, static_secp_instance};
 
 /// Errors thrown by Block validation
 #[derive(Debug, Clone, PartialEq)]
@@ -367,14 +367,16 @@ impl Readable for Block {
 /// Provides all information from a block that allows the calculation of total
 /// Pedersen commitment.
 impl Committed for Block {
-	fn inputs_committed(&self) -> &Vec<Input> {
-		&self.inputs
+	fn inputs_committed(&self) -> Vec<Commitment> {
+		self.inputs.iter().map(|x| x.commitment()).collect()
 	}
-	fn outputs_committed(&self) -> &Vec<Output> {
-		&self.outputs
+
+	fn outputs_committed(&self) -> Vec<Commitment> {
+		self.outputs.iter().map(|x| x.commitment()).collect()
 	}
-	fn overage(&self) -> i64 {
-		-(REWARD as i64)
+
+	fn kernels_committed(&self) -> Vec<Commitment> {
+		self.kernels.iter().map(|x| x.excess()).collect()
 	}
 }
 
@@ -708,23 +710,21 @@ impl Block {
 		prev_output_sum: &Commitment,
 		prev_kernel_sum: &Commitment,
 	) -> Result<((Commitment, Commitment)), Error> {
-		// Note: we check the rangeproofs in here (expensive)
-		let io_sum = self.sum_commitments()?;
+		// Verify the output rangeproofs.
+		// Note: this is expensive.
+		for x in &self.outputs {
+			x.verify_proof()?;
+		}
 
-		// Note: we check the kernel signatures in here (expensive)
-		let kernel_sum = self.sum_kernel_excesses(prev_kernel_sum)?;
+		// Verify the kernel signatures.
+		// Note: this is expensive.
+		for x in &self.kernels {
+			x.verify()?;
+		}
 
-		let mut output_commits = vec![io_sum, prev_output_sum.clone()];
-
-		// We do not (yet) know how to sum a "zero commit" so remove them
-		let zero_commit = secp_static::commit_to_zero_value();
-		output_commits.retain(|x| *x != zero_commit);
-
-		let output_sum = {
-			let secp = static_secp_instance();
-			let secp = secp.lock().unwrap();
-			secp.commit_sum(output_commits, vec![])?
-		};
+		// Sum all input|output|overage commitments.
+		let overage = (REWARD as i64).checked_neg().unwrap_or(0);
+		let io_sum = self.sum_commitments(overage, Some(prev_output_sum))?;
 
 		let offset = {
 			let secp = static_secp_instance();
@@ -733,41 +733,15 @@ impl Block {
 			secp.commit(0, key)?
 		};
 
-		let mut excesses = vec![kernel_sum, offset];
-		excesses.retain(|x| *x != zero_commit);
+		// Sum the kernel excesses accounting for the kernel offset.
+		let (kernel_sum, kernel_sum_plus_offset) =
+			self.sum_kernel_excesses(&offset, Some(prev_kernel_sum))?;
 
-		let kernel_sum_plus_offset = {
-			let secp = static_secp_instance();
-			let secp = secp.lock().unwrap();
-			secp.commit_sum(excesses, vec![])?
-		};
-
-		if output_sum != kernel_sum_plus_offset {
+		if io_sum != kernel_sum_plus_offset {
 			return Err(Error::KernelSumMismatch);
 		}
 
-		Ok((output_sum, kernel_sum))
-	}
-
-	fn sum_kernel_excesses(&self, prev_excess: &Commitment) -> Result<Commitment, Error> {
-		for kernel in &self.kernels {
-			kernel.verify()?;
-		}
-
-		let mut excesses = self.kernels.iter().map(|x| x.excess).collect::<Vec<_>>();
-		excesses.push(prev_excess.clone());
-
-		// we do not (yet) know how to sum a "zero commit" so remove them
-		let zero_commit = secp_static::commit_to_zero_value();
-		excesses.retain(|x| *x != zero_commit);
-
-		let sum = {
-			let secp = static_secp_instance();
-			let secp = secp.lock().unwrap();
-			secp.commit_sum(excesses, vec![])?
-		};
-
-		Ok(sum)
+		Ok((io_sum, kernel_sum))
 	}
 
 	// Validate the coinbase outputs generated by miners. Entails 2 main checks:
@@ -860,15 +834,15 @@ impl Block {
 
 #[cfg(test)]
 mod test {
+	use std::time::Instant;
+
 	use super::*;
 	use core::Transaction;
 	use core::build::{self, input, output, with_fee};
 	use core::test::{tx1i2o, tx2i1o};
 	use keychain::{Identifier, Keychain};
 	use consensus::{BLOCK_OUTPUT_WEIGHT, MAX_BLOCK_WEIGHT};
-	use std::time::Instant;
-
-	use util::secp;
+	use util::{secp, secp_static};
 
 	// utility to create a block without worrying about the key or previous
 	// header

core/src/core/mod.rs

@@ -30,7 +30,7 @@ use std::cmp::Ordering;
 use std::num::ParseFloatError;
 use consensus::GRIN_BASE;
 
-use util::{secp, static_secp_instance};
+use util::{secp, secp_static, static_secp_instance};
 use util::secp::pedersen::*;
 
 pub use self::block::*;
@@ -40,25 +40,64 @@ use core::hash::Hashed;
 use ser::{Error, Readable, Reader, Writeable, Writer};
 use global;
 
-/// Implemented by types that hold inputs and outputs including Pedersen
-/// commitments. Handles the collection of the commitments as well as their
+/// Implemented by types that hold inputs and outputs (and kernels)
+/// containing Pedersen commitments.
+/// Handles the collection of the commitments as well as their
 /// summing, taking potential explicit overages of fees into account.
 pub trait Committed {
-	/// Gathers commitments and sum them.
-	fn sum_commitments(&self) -> Result<Commitment, secp::Error> {
-		// first, verify each range proof
-		let ref outputs = self.outputs_committed();
-		for output in *outputs {
-			try!(output.verify_proof())
+	/// Gather the kernel excesses and sum them.
+	fn sum_kernel_excesses(
+		&self,
+		offset: &Commitment,
+		extra_excess: Option<&Commitment>,
+	) -> Result<(Commitment, Commitment), secp::Error> {
+		let zero_commit = secp_static::commit_to_zero_value();
+
+		// then gather the kernel excess commitments
+		let mut kernel_commits = self.kernels_committed();
+
+		if let Some(extra) = extra_excess {
+			kernel_commits.push(*extra);
 		}
 
-		// then gather the commitments
-		let mut input_commits = map_vec!(self.inputs_committed(), |inp| inp.commitment());
-		let mut output_commits = map_vec!(self.outputs_committed(), |out| out.commitment());
+		// handle "zero commit" values by filtering them out here
+		kernel_commits.retain(|x| *x != zero_commit);
 
-		// add the overage as output commitment if positive, as an input commitment if
-		// negative
-		let overage = self.overage();
+		// sum the commitments
+		let kernel_sum = {
+			let secp = static_secp_instance();
+			let secp = secp.lock().unwrap();
+			secp.commit_sum(kernel_commits, vec![])?
+		};
+
+		// sum the commitments along with the specified offset
+		let kernel_sum_plus_offset = {
+			let secp = static_secp_instance();
+			let secp = secp.lock().unwrap();
+			let mut commits = vec![kernel_sum];
+			if *offset != zero_commit {
+				commits.push(*offset);
+			}
+			secp.commit_sum(commits, vec![])?
+		};
+
+		Ok((kernel_sum, kernel_sum_plus_offset))
+	}
+
+	/// Gathers commitments and sum them.
+	fn sum_commitments(
+		&self,
+		overage: i64,
+		extra_commit: Option<&Commitment>,
+	) -> Result<Commitment, secp::Error> {
+		let zero_commit = secp_static::commit_to_zero_value();
+
+		// then gather the commitments
+		let mut input_commits = self.inputs_committed();
+		let mut output_commits = self.outputs_committed();
+
+		// add the overage as output commitment if positive,
+		// or as an input commitment if negative
 		if overage != 0 {
 			let over_commit = {
 				let secp = static_secp_instance();
@@ -72,6 +111,14 @@ pub trait Committed {
 			}
 		}
 
+		if let Some(extra) = extra_commit {
+			output_commits.push(*extra);
+		}
+
+		// handle "zero commit" values by filtering them out here
+		output_commits.retain(|x| *x != zero_commit);
+		input_commits.retain(|x| *x != zero_commit);
+
 		// sum all that stuff
 		{
 			let secp = static_secp_instance();
@@ -80,15 +127,14 @@ pub trait Committed {
 		}
 	}
 
-	/// Vector of committed inputs to verify
-	fn inputs_committed(&self) -> &Vec<Input>;
+	/// Vector of input commitments to verify.
+	fn inputs_committed(&self) -> Vec<Commitment>;
 
-	/// Vector of committed inputs to verify
-	fn outputs_committed(&self) -> &Vec<Output>;
+	/// Vector of output commitments to verify.
+	fn outputs_committed(&self) -> Vec<Commitment>;
 
-	/// The overage amount expected over the commitments. Can be negative (a
-	/// fee) or positive (a reward).
-	fn overage(&self) -> i64;
+	/// Vector of kernel excesses to verify.
+	fn kernels_committed(&self) -> Vec<Commitment>;
 }
 
 /// Proof of work

core/src/core/transaction.rs

@@ -35,7 +35,6 @@ use ser::{self, read_and_verify_sorted, ser_vec, PMMRable, Readable, Reader, Wri
           WriteableSorted, Writer};
 use util;
 use util::LOGGER;
-use util::secp_static;
 
 bitflags! {
 	/// Options for a kernel's structure or use
@@ -174,6 +173,11 @@ impl Readable for TxKernel {
 }
 
 impl TxKernel {
+	/// Return the excess commitment for this tx_kernel.
+	pub fn excess(&self) -> Commitment {
+		self.excess
+	}
+
 	/// Verify the transaction proof validity. Entails handling the commitment
 	/// as a public key and checking the signature verifies with the fee as
 	/// message.
@@ -299,14 +303,16 @@ impl Readable for Transaction {
 }
 
 impl Committed for Transaction {
-	fn inputs_committed(&self) -> &Vec<Input> {
-		&self.inputs
+	fn inputs_committed(&self) -> Vec<Commitment> {
+		self.inputs.iter().map(|x| x.commitment()).collect()
 	}
-	fn outputs_committed(&self) -> &Vec<Output> {
-		&self.outputs
+
+	fn outputs_committed(&self) -> Vec<Commitment> {
+		self.outputs.iter().map(|x| x.commitment()).collect()
 	}
-	fn overage(&self) -> i64 {
-		self.fee() as i64
+
+	fn kernels_committed(&self) -> Vec<Commitment> {
+		self.kernels.iter().map(|x| x.excess()).collect()
 	}
 }
 
@@ -388,41 +394,38 @@ impl Transaction {
 	///  * 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> {
-		// sum all input and output commitments
-		let io_sum = self.sum_commitments()?;
+		// Verify all the output rangeproofs.
+		// Note: this is expensive.
+		for x in &self.outputs {
+			x.verify_proof()?;
+		}
 
-		// sum all kernels commitments
-		let kernel_sum = {
-			let mut kernel_commits = self.kernels.iter().map(|x| x.excess).collect::<Vec<_>>();
+		// Verify the kernel signatures.
+		// Note: this is expensive.
+		for x in &self.kernels {
+			x.verify()?;
+		}
 
-			let offset = {
-				let secp = static_secp_instance();
-				let secp = secp.lock().unwrap();
-				let key = self.offset.secret_key(&secp)?;
-				secp.commit(0, key)?
-			};
-
-			kernel_commits.push(offset);
-
-			// We cannot sum zero commits so remove them here
-			let zero_commit = secp_static::commit_to_zero_value();
-			kernel_commits.retain(|x| *x != zero_commit);
+		// Sum all input|output|overage commitments.
+		let overage = self.fee() as i64;
+		let io_sum = self.sum_commitments(overage, None)?;
 
+		let offset = {
 			let secp = static_secp_instance();
 			let secp = secp.lock().unwrap();
-			secp.commit_sum(kernel_commits, vec![])?
+			let key = self.offset.secret_key(&secp)?;
+			secp.commit(0, key)?
 		};
 
+		// Sum the kernel excesses accounting for the kernel offset.
+		let (_, kernel_sum) = self.sum_kernel_excesses(&offset, None)?;
+
 		// sum of kernel commitments (including the offset) must match
 		// the sum of input/output commitments (minus fee)
-		if kernel_sum != io_sum {
+		if io_sum != kernel_sum {
 			return Err(Error::KernelSumMismatch);
 		}
 
-		for kernel in &self.kernels {
-			kernel.verify()?;
-		}
-
 		Ok(())
 	}
 

wallet/src/receiver.rs

@@ -478,8 +478,16 @@ fn build_final_transaction(
 
 	// build the final excess based on final tx and offset
 	let final_excess = {
+		// TODO - do we need to verify rangeproofs here?
+		for x in &final_tx.outputs {
+			x.verify_proof().context(ErrorKind::Transaction)?;
+		}
+
 		// sum the input/output commitments on the final tx
-		let tx_excess = final_tx.sum_commitments().context(ErrorKind::Transaction)?;
+		let overage = final_tx.fee() as i64;
+		let tx_excess = final_tx
+			.sum_commitments(overage, None)
+			.context(ErrorKind::Transaction)?;
 
 		// subtract the kernel_excess (built from kernel_offset)
 		let offset_excess = keychain