[testnet2] Store output sum (#1043)

* block sums and reworked block validation
read and write block_sums
refactor validate on both block and txhashset
write block_sum on fast sync
we store the kernel_sum (need to account for the offset)

* block_sums

* rustfmt

* cleanup

chain/src/chain.rs

@@ -187,9 +187,32 @@ impl Chain {
 						);
 
 						extension.rewind(&header)?;
+
 						extension.validate_roots(&header)?;
+
+						// now check we have the "block sums" for the block in question
+						// if we have no sums (migrating an existing node) we need to go
+						// back to the txhashset and sum the outputs and kernels
+						if header.height > 0 && store.get_block_sums(&header.hash()).is_err() {
+							debug!(
+								LOGGER,
+								"chain: init: building (missing) block sums for {} @ {}",
+								header.height,
+								header.hash()
+							);
+							let (output_sum, kernel_sum) = extension.validate_sums(&header)?;
+							store.save_block_sums(
+								&header.hash(),
+								&BlockSums {
+									output_sum,
+									kernel_sum,
+								},
+							)?;
+						}
+
 						Ok(())
 					});
+
 					if res.is_ok() {
 						break;
 					} else {
@@ -453,7 +476,8 @@ impl Chain {
 		// consistent.
 		txhashset::extending_readonly(&mut txhashset, |extension| {
 			extension.rewind(&header)?;
-			extension.validate(&header, skip_rproofs)
+			extension.validate(&header, skip_rproofs)?;
+			Ok(())
 		})
 	}
 
@@ -574,7 +598,8 @@ impl Chain {
 		// Note: we are validating against a writeable extension.
 		txhashset::extending(&mut txhashset, |extension| {
 			extension.rewind(&header)?;
-			extension.validate(&header, false)?;
+			let (output_sum, kernel_sum) = extension.validate(&header, false)?;
+			extension.save_latest_block_sums(&header, output_sum, kernel_sum)?;
 			extension.rebuild_index()?;
 			Ok(())
 		})?;
@@ -647,6 +672,7 @@ impl Chain {
 				Ok(b) => {
 					self.store.delete_block(&b.hash())?;
 					self.store.delete_block_marker(&b.hash())?;
+					self.store.delete_block_sums(&b.hash())?;
 				}
 				Err(NotFoundErr) => {
 					break;
@@ -764,6 +790,13 @@ impl Chain {
 			.map_err(|e| Error::StoreErr(e, "chain get block marker".to_owned()))
 	}
 
+	/// Get the blocks sums for the specified block hash.
+	pub fn get_block_sums(&self, bh: &Hash) -> Result<BlockSums, Error> {
+		self.store
+			.get_block_sums(bh)
+			.map_err(|e| Error::StoreErr(e, "chain get block sums".to_owned()))
+	}
+
 	/// Gets the block header at the provided height
 	pub fn get_header_by_height(&self, height: u64) -> Result<BlockHeader, Error> {
 		self.store

chain/src/pipe.rs

@@ -64,7 +64,7 @@ pub fn process_block(b: &Block, mut ctx: BlockContext) -> Result<Option<Tip>, Er
 
 	validate_header(&b.header, &mut ctx)?;
 
-	// valid header, now check we actually have the previous block in the store
+	// now check we actually have the previous block in the store
 	// not just the header but the block itself
 	// short circuit the test first both for performance (in-mem vs db access)
 	// but also for the specific case of the first fast sync full block
@@ -83,8 +83,12 @@ pub fn process_block(b: &Block, mut ctx: BlockContext) -> Result<Option<Tip>, Er
 		}
 	}
 
-	// valid header and we have a previous block, time to take the lock on the sum
-	// trees
+	// validate the block itself
+	// we can do this now before interact with the txhashset
+	validate_block(b, &mut ctx)?;
+
+	// header and block both valid, and we have a previous block
+	// so take the lock on the txhashset
 	let local_txhashset = ctx.txhashset.clone();
 	let mut txhashset = local_txhashset.write().unwrap();
 
@@ -96,7 +100,7 @@ pub fn process_block(b: &Block, mut ctx: BlockContext) -> Result<Option<Tip>, Er
 	// start a chain extension unit of work dependent on the success of the
 	// internal validation and saving operations
 	let result = txhashset::extending(&mut txhashset, |mut extension| {
-		validate_block(b, &mut ctx, &mut extension)?;
+		validate_block_via_txhashset(b, &mut ctx, &mut extension)?;
 		trace!(
 			LOGGER,
 			"pipe: process_block: {} at {} is valid, save and append.",
@@ -295,18 +299,38 @@ fn validate_header(header: &BlockHeader, ctx: &mut BlockContext) -> Result<(), E
 	Ok(())
 }
 
-/// Fully validate the block content.
-fn validate_block(
+fn validate_block(b: &Block, ctx: &mut BlockContext) -> Result<(), Error> {
+	// If this is the first block then we have no previous block sums stored.
+	let block_sums = if b.header.height == 1 {
+		BlockSums::default()
+	} else {
+		ctx.store.get_block_sums(&b.header.previous)?
+	};
+
+	let (new_output_sum, new_kernel_sum) =
+		b.validate(&block_sums.output_sum, &block_sums.kernel_sum)
+			.map_err(&Error::InvalidBlockProof)?;
+
+	ctx.store.save_block_sums(
+		&b.hash(),
+		&BlockSums {
+			output_sum: new_output_sum,
+			kernel_sum: new_kernel_sum,
+		},
+	)?;
+
+	Ok(())
+}
+
+/// Fully validate the block by applying it to the txhashset extension
+/// and checking the roots.
+/// Rewind and reapply forked blocks if necessary to put the txhashset extension
+/// in the correct state to accept the block.
+fn validate_block_via_txhashset(
 	b: &Block,
 	ctx: &mut BlockContext,
 	ext: &mut txhashset::Extension,
 ) -> Result<(), Error> {
-	let prev_header = ctx.store.get_block_header(&b.header.previous)?;
-
-	// main isolated block validation
-	// checks all commitment sums and sigs
-	b.validate(&prev_header).map_err(&Error::InvalidBlockProof)?;
-
 	if b.header.previous != ctx.head.last_block_h {
 		rewind_and_apply_fork(b, ctx.store.clone(), ext)?;
 	}
@@ -322,17 +346,21 @@ fn validate_block(
 
 		debug!(
 			LOGGER,
-			"validate_block: output roots - {:?}, {:?}", roots.output_root, b.header.output_root,
+			"validate_block_via_txhashset: output roots - {:?}, {:?}",
+			roots.output_root,
+			b.header.output_root,
 		);
 		debug!(
 			LOGGER,
-			"validate_block: rproof roots - {:?}, {:?}",
+			"validate_block_via_txhashset: rproof roots - {:?}, {:?}",
 			roots.rproof_root,
 			b.header.range_proof_root,
 		);
 		debug!(
 			LOGGER,
-			"validate_block: kernel roots - {:?}, {:?}", roots.kernel_root, b.header.kernel_root,
+			"validate_block_via_txhashset: kernel roots - {:?}, {:?}",
+			roots.kernel_root,
+			b.header.kernel_root,
 		);
 
 		return Err(Error::InvalidRoot);

chain/src/store.rs

@@ -37,6 +37,7 @@ const SYNC_HEAD_PREFIX: u8 = 's' as u8;
 const HEADER_HEIGHT_PREFIX: u8 = '8' as u8;
 const COMMIT_POS_PREFIX: u8 = 'c' as u8;
 const BLOCK_MARKER_PREFIX: u8 = 'm' as u8;
+const BLOCK_SUMS_PREFIX: u8 = 'M' as u8;
 
 /// An implementation of the ChainStore trait backed by a simple key-value
 /// store.
@@ -238,6 +239,22 @@ impl ChainStore for ChainKVStore {
 			.delete(&to_key(BLOCK_MARKER_PREFIX, &mut bh.to_vec()))
 	}
 
+	fn save_block_sums(&self, bh: &Hash, marker: &BlockSums) -> Result<(), Error> {
+		self.db
+			.put_ser(&to_key(BLOCK_SUMS_PREFIX, &mut bh.to_vec())[..], &marker)
+	}
+
+	fn get_block_sums(&self, bh: &Hash) -> Result<BlockSums, Error> {
+		option_to_not_found(
+			self.db
+				.get_ser(&to_key(BLOCK_SUMS_PREFIX, &mut bh.to_vec())),
+		)
+	}
+
+	fn delete_block_sums(&self, bh: &Hash) -> Result<(), Error> {
+		self.db.delete(&to_key(BLOCK_SUMS_PREFIX, &mut bh.to_vec()))
+	}
+
 	/// Maintain consistency of the "header_by_height" index by traversing back
 	/// through the current chain and updating "header_by_height" until we reach
 	/// a block_header that is consistent with its height (everything prior to

chain/src/txhashset.rs

@@ -35,9 +35,8 @@ use core::ser::{PMMRIndexHashable, PMMRable};
 use grin_store;
 use grin_store::pmmr::PMMRBackend;
 use grin_store::types::prune_noop;
-use keychain::BlindingFactor;
-use types::{BlockMarker, ChainStore, Error, TxHashSetRoots};
-use util::{zip, LOGGER};
+use types::{BlockMarker, BlockSums, ChainStore, Error, TxHashSetRoots};
+use util::{secp_static, zip, LOGGER};
 
 const TXHASHSET_SUBDIR: &'static str = "txhashset";
 const OUTPUT_SUBDIR: &'static str = "output";
@@ -607,8 +606,9 @@ impl<'a> Extension<'a> {
 		Ok(())
 	}
 
-	/// Validate the txhashset state against the provided block header.
-	pub fn validate(&mut self, header: &BlockHeader, skip_rproofs: bool) -> Result<(), Error> {
+	fn validate_mmrs(&self) -> Result<(), Error> {
+		let now = Instant::now();
+
 		// validate all hashes and sums within the trees
 		if let Err(e) = self.output_pmmr.validate() {
 			return Err(Error::InvalidTxHashSet(e));
@@ -620,40 +620,106 @@ impl<'a> Extension<'a> {
 			return Err(Error::InvalidTxHashSet(e));
 		}
 
+		debug!(
+			LOGGER,
+			"txhashset: validated the output|rproof|kernel mmrs, took {}s",
+			now.elapsed().as_secs(),
+		);
+
+		Ok(())
+	}
+
+	/// The real magicking: the sum of all kernel excess should equal the sum
+	/// of all output commitments, minus the total supply.
+	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();
+			let key = header.total_kernel_offset.secret_key(&secp)?;
+			secp.commit(0, key)?
+		};
+
+		let mut excesses = vec![kernel_sum, offset];
+		excesses.retain(|x| *x != zero_commit);
+
+		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 {
+			return Err(Error::InvalidTxHashSet(
+				"Differing Output commitment and kernel excess sums.".to_owned(),
+			));
+		}
+
+		debug!(
+			LOGGER,
+			"txhashset: validated sums, took (total) {}s",
+			now.elapsed().as_secs(),
+		);
+
+		Ok((output_sum, kernel_sum))
+	}
+
+	/// Validate the txhashset state against the provided block header.
+	pub fn validate(
+		&mut self,
+		header: &BlockHeader,
+		skip_rproofs: bool,
+	) -> Result<((Commitment, Commitment)), Error> {
+		self.validate_mmrs()?;
 		self.validate_roots(header)?;
 
 		if header.height == 0 {
-			return Ok(());
+			let zero_commit = secp_static::commit_to_zero_value();
+			return Ok((zero_commit.clone(), zero_commit.clone()));
 		}
 
-		// the real magicking: the sum of all kernel excess should equal the sum
-		// of all Output commitments, minus the total supply
-		let kernel_offset = self.sum_kernel_offsets(&header)?;
-		let kernel_sum = self.sum_kernels(kernel_offset)?;
-		let output_sum = self.sum_outputs()?;
+		let (output_sum, kernel_sum) = self.validate_sums(header)?;
 
-		// supply is the sum of the coinbase outputs from all the block headers
-		let supply = header.height * REWARD;
+		// this is a relatively expensive verification step
+		self.verify_kernel_signatures()?;
 
-		{
-			let secp = static_secp_instance();
-			let secp = secp.lock().unwrap();
-
-			let over_commit = secp.commit_value(supply)?;
-			let adjusted_sum_output = secp.commit_sum(vec![output_sum], vec![over_commit])?;
-			if adjusted_sum_output != kernel_sum {
-				return Err(Error::InvalidTxHashSet(
-					"Differing Output commitment and kernel excess sums.".to_owned(),
-				));
-			}
-		}
-
-		// now verify the rangeproof for each output in the sum above
+		// verify the rangeproof for each output in the sum above
 		// this is an expensive operation (only verified if requested)
 		if !skip_rproofs {
 			self.verify_rangeproofs()?;
 		}
 
+		Ok((output_sum, kernel_sum))
+	}
+
+	/// Save blocks sums (the output_sum and kernel_sum) for the given block
+	/// header.
+	pub fn save_latest_block_sums(
+		&self,
+		header: &BlockHeader,
+		output_sum: Commitment,
+		kernel_sum: Commitment,
+	) -> Result<(), Error> {
+		self.commit_index.save_block_sums(
+			&header.hash(),
+			&BlockSums {
+				output_sum,
+				kernel_sum,
+			},
+		)?;
 		Ok(())
 	}
 
@@ -709,53 +775,59 @@ impl<'a> Extension<'a> {
 		)
 	}
 
-	// We maintain the total accumulated kernel offset in each block header.
-	// So "summing" is just a case of taking the total kernel offset
-	// directly from the current block header.
-	fn sum_kernel_offsets(&self, header: &BlockHeader) -> Result<Option<Commitment>, Error> {
-		let offset = if header.total_kernel_offset == BlindingFactor::zero() {
-			None
-		} else {
-			let secp = static_secp_instance();
-			let secp = secp.lock().unwrap();
-			let skey = header.total_kernel_offset.secret_key(&secp)?;
-			Some(secp.commit(0, skey)?)
-		};
-		Ok(offset)
-	}
-
-	/// Sums the excess of all our kernels, validating their signatures on the
-	/// way
-	fn sum_kernels(&self, kernel_offset: Option<Commitment>) -> Result<Commitment, Error> {
+	/// Sums the excess of all our kernels.
+	fn sum_kernels(&self) -> Result<Commitment, Error> {
 		let now = Instant::now();
 
 		let mut commitments = vec![];
-		if let Some(offset) = kernel_offset {
-			commitments.push(offset);
-		}
-
 		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) {
-					kernel.verify()?;
 					commitments.push(kernel.excess);
 				}
 			}
 		}
 
-		let secp = static_secp_instance();
-		let secp = secp.lock().unwrap();
 		let kern_count = commitments.len();
-		let sum_kernel = secp.commit_sum(commitments, vec![])?;
+
+		let kernel_sum = {
+			let secp = static_secp_instance();
+			let secp = secp.lock().unwrap();
+			secp.commit_sum(commitments, vec![])?
+		};
 
 		debug!(
 			LOGGER,
-			"Validated, summed (and offset) {} kernels, pmmr size {}, took {}s",
+			"txhashset: summed {} kernels, pmmr size {}, took {}s",
 			kern_count,
 			self.kernel_pmmr.unpruned_size(),
 			now.elapsed().as_secs(),
 		);
-		Ok(sum_kernel)
+		Ok(kernel_sum)
+	}
+
+	fn verify_kernel_signatures(&self) -> Result<(), Error> {
+		let now = Instant::now();
+
+		let mut kern_count = 0;
+		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) {
+					kernel.verify()?;
+					kern_count += 1;
+				}
+			}
+		}
+
+		debug!(
+			LOGGER,
+			"txhashset: verified {} kernel signatures, pmmr size {}, took {}s",
+			kern_count,
+			self.kernel_pmmr.unpruned_size(),
+			now.elapsed().as_secs(),
+		);
+
+		Ok(())
 	}
 
 	fn verify_rangeproofs(&self) -> Result<(), Error> {
@@ -784,7 +856,7 @@ impl<'a> Extension<'a> {
 		}
 		debug!(
 			LOGGER,
-			"Verified {} Rangeproofs, pmmr size {}, took {}s",
+			"txhashset: verified {} rangeproofs, pmmr size {}, took {}s",
 			proof_count,
 			self.rproof_pmmr.unpruned_size(),
 			now.elapsed().as_secs(),
@@ -792,8 +864,8 @@ impl<'a> Extension<'a> {
 		Ok(())
 	}
 
-	/// Sums all our Output commitments, checking range proofs at the same time
-	fn sum_outputs(&self) -> Result<Commitment, Error> {
+	/// Sums all our unspent output commitments.
+	fn sum_outputs(&self, supply_commit: Commitment) -> Result<Commitment, Error> {
 		let now = Instant::now();
 
 		let mut commitments = vec![];
@@ -805,20 +877,23 @@ impl<'a> Extension<'a> {
 			}
 		}
 
-		let secp = static_secp_instance();
-		let secp = secp.lock().unwrap();
 		let commit_count = commitments.len();
-		let sum_output = secp.commit_sum(commitments, vec![])?;
+
+		let output_sum = {
+			let secp = static_secp_instance();
+			let secp = secp.lock().unwrap();
+			secp.commit_sum(commitments, vec![supply_commit])?
+		};
 
 		debug!(
 			LOGGER,
-			"Summed {} Outputs, pmmr size {}, took {}s",
+			"txhashset: summed {} outputs, pmmr size {}, took {}s",
 			commit_count,
 			self.output_pmmr.unpruned_size(),
 			now.elapsed().as_secs(),
 		);
 
-		Ok(sum_output)
+		Ok(output_sum)
 	}
 }
 

chain/src/types.rs

@@ -17,6 +17,7 @@
 use std::{error, fmt, io};
 
 use util::secp;
+use util::secp_static;
 use util::secp::pedersen::Commitment;
 
 use core::core::hash::{Hash, Hashed};
@@ -328,6 +329,15 @@ pub trait ChainStore: Send + Sync {
 	/// Deletes a block marker associated with the provided hash
 	fn delete_block_marker(&self, bh: &Hash) -> Result<(), store::Error>;
 
+	/// Save block sums for the given block hash.
+	fn save_block_sums(&self, bh: &Hash, marker: &BlockSums) -> Result<(), store::Error>;
+
+	/// Get block sums for the given block hash.
+	fn get_block_sums(&self, bh: &Hash) -> Result<BlockSums, store::Error>;
+
+	/// Delete block sums for the given block hash.
+	fn delete_block_sums(&self, bh: &Hash) -> Result<(), store::Error>;
+
 	/// Saves the provided block header at the corresponding height. Also check
 	/// the consistency of the height chain in store by assuring previous
 	/// headers are also at their respective heights.
@@ -389,3 +399,42 @@ impl Default for BlockMarker {
 		}
 	}
 }
+
+/// The output_sum and kernel_sum for a given block.
+/// This is used to validate the next block being processed by applying
+/// the inputs, outputs, kernels and kernel_offset from the new block
+/// and checking everything sums correctly.
+#[derive(Debug, Clone)]
+pub struct BlockSums {
+	/// The total output sum so far.
+	pub output_sum: Commitment,
+	/// The total kernel sum so far.
+	pub kernel_sum: Commitment,
+}
+
+impl Writeable for BlockSums {
+	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
+		writer.write_fixed_bytes(&self.output_sum)?;
+		writer.write_fixed_bytes(&self.kernel_sum)?;
+		Ok(())
+	}
+}
+
+impl Readable for BlockSums {
+	fn read(reader: &mut Reader) -> Result<BlockSums, ser::Error> {
+		Ok(BlockSums {
+			output_sum: Commitment::read(reader)?,
+			kernel_sum: Commitment::read(reader)?,
+		})
+	}
+}
+
+impl Default for BlockSums {
+	fn default() -> BlockSums {
+		let zero_commit = secp_static::commit_to_zero_value();
+		BlockSums {
+			output_sum: zero_commit.clone(),
+			kernel_sum: zero_commit.clone(),
+		}
+	}
+}