Dry up how we check if a header is on the current chain (#431)

Uses the header_by_height index

chain/src/chain.rs

@@ -25,7 +25,7 @@ use core::core::pmmr::{HashSum, NoSum};
 
 use core::core::{Block, BlockHeader, Output, TxKernel};
 use core::core::target::Difficulty;
-use core::core::hash::{Hash, Hashed};
+use core::core::hash::Hash;
 use grin_store::Error::NotFoundErr;
 use pipe;
 use store;
@@ -103,8 +103,7 @@ impl Chain {
 		let _ = match chain_store.get_sync_head() {
 			Ok(tip) => tip,
 			Err(NotFoundErr) => {
-				let gen = chain_store.get_header_by_height(0).unwrap();
+				let tip = chain_store.head().unwrap();
-				let tip = Tip::new(gen.hash());
 				chain_store.save_sync_head(&tip)?;
 				tip
 			},
@@ -362,6 +361,14 @@ impl Chain {
 		})
 	}
 
+	/// Verifies the given block header is actually on the current chain.
+	/// Checks the header_by_height index to verify the header is where we say it is
+	pub fn is_on_current_chain(&self, header: &BlockHeader) -> Result<(), Error> {
+		self.store.is_on_current_chain(header).map_err(|e| {
+			Error::StoreErr(e, "chain is_on_current_chain".to_owned())
+		})
+	}
+
 	/// Gets the block header by the provided output commitment
 	pub fn get_block_header_by_output_commit(
 		&self,

chain/src/pipe.rs

@@ -245,22 +245,12 @@ fn validate_block(
 		let mut hashes = vec![];
 		loop {
 			let curr_header = ctx.store.get_block_header(&current)?;
-			match ctx.store.get_header_by_height(curr_header.height) {
+
-				Ok(height_header) => {
+			if let Ok(_) = ctx.store.is_on_current_chain(&curr_header) {
-					if curr_header.hash() != height_header.hash() {
+				break;
-						hashes.insert(0, curr_header.hash());
+			} else {
-						current = curr_header.previous;
+				hashes.insert(0, curr_header.hash());
-					} else {
+				current = curr_header.previous;
-						break;
-					}
-				},
-				Err(grin_store::Error::NotFoundErr) => {
-					hashes.insert(0, curr_header.hash());
-					current = curr_header.previous;
-				},
-				Err(e) => {
-					return Err(Error::StoreErr(e, format!("header by height lookup failed")));
-				}
 			}
 		}
 

chain/src/store.rs

@@ -156,17 +156,22 @@ impl ChainStore for ChainKVStore {
 		match block_hash {
 			Some(hash) => {
 				let block_header = self.get_block_header(&hash)?;
-				let header_at_height = self.get_header_by_height(block_header.height)?;
+				self.is_on_current_chain(&block_header)?;
-				if block_header.hash() == header_at_height.hash() {
+				Ok(block_header)
-					Ok(block_header)
-				} else {
-					Err(Error::NotFoundErr)
-				}
 			}
 			None => Err(Error::NotFoundErr),
 		}
 	}
 
+	fn is_on_current_chain(&self, header: &BlockHeader) -> Result<(), Error> {
+		let header_at_height = self.get_header_by_height(header.height)?;
+		if header.hash() == header_at_height.hash() {
+			Ok(())
+		} else {
+			Err(Error::NotFoundErr)
+		}
+	}
+
 	fn save_block_header(&self, bh: &BlockHeader) -> Result<(), Error> {
 		self.db.put_ser(
 			&to_key(BLOCK_HEADER_PREFIX, &mut bh.hash().to_vec())[..],
@@ -222,29 +227,19 @@ impl ChainStore for ChainKVStore {
 	/// We need to handle the case where we have no index entry for a given height to
 	/// account for the case where we just switched to a new fork and the height jumped
 	/// beyond current chain height.
-	fn setup_height(&self, bh: &BlockHeader) -> Result<(), Error> {
+	fn setup_height(&self, header: &BlockHeader) -> Result<(), Error> {
 		self.db
-			.put_ser(&u64_to_key(HEADER_HEIGHT_PREFIX, bh.height), bh)?;
+			.put_ser(&u64_to_key(HEADER_HEIGHT_PREFIX, header.height), header)?;
-		if bh.height == 0 {
+
+		if header.height == 0 {
 			return Ok(());
 		}
 
-		let prev_h = bh.previous;
+		let prev_header = self.get_block_header(&header.previous)?;
-		let prev_height = bh.height - 1;
+		if let Ok(_) = self.is_on_current_chain(&prev_header) {
-		match self.get_header_by_height(prev_height) {
+			Ok(())
-			Ok(prev) => {
+		} else {
-				if prev.hash() != prev_h {
+			self.setup_height(&prev_header)
-					let real_prev = self.get_block_header(&prev_h)?;
-					self.setup_height(&real_prev)
-				} else {
-					Ok(())
-				}
-			},
-			Err(Error::NotFoundErr) => {
-				let real_prev = self.get_block_header(&prev_h)?;
-				self.setup_height(&real_prev)
-			},
-			Err(e) => Err(e)
 		}
 	}
 }

chain/src/types.rs

@@ -226,6 +226,10 @@ pub trait ChainStore: Send + Sync {
 	/// Gets the block header at the provided height
 	fn get_header_by_height(&self, height: u64) -> Result<BlockHeader, store::Error>;
 
+	/// Is the block header on the current chain?
+	/// Use the header_by_height index to verify the block header is where we think it is.
+	fn is_on_current_chain(&self, header: &BlockHeader) -> Result<(), store::Error>;
+
 	/// Gets an output by its commitment
 	fn get_output_by_commit(&self, commit: &Commitment) -> Result<Output, store::Error>;
 

grin/src/sync.rs

@@ -94,15 +94,10 @@ fn body_sync(
 		let mut current = chain.get_block_header(&header_head.last_block_h);
 		while let Ok(header) = current {
 
-			// look back through the sync chain until we find a header
+			// break out of the while loop when we find a header common
-			// that is consistent with the height index (we know this is in the real chain)
+			// between the this chain and the current chain
-			match chain.get_header_by_height(header.height) {
+			if let Ok(_) = chain.is_on_current_chain(&header) {
-				Ok(height_header) => {
+				break;
-					if header.hash() == height_header.hash() {
-						break;
-					}
-				},
-				Err(_) => {},
 			}
 
 			hashes.push(header.hash());