From f5d24c5a9c2c1b44d4adedecfcf283f46c9e2716 Mon Sep 17 00:00:00 2001 From: AntiochP <30642645+antiochp@users.noreply.github.com> Date: Tue, 5 Dec 2017 13:32:57 -0500 Subject: [PATCH] Dry up how we check if a header is on the current chain (#431) Uses the header_by_height index --- chain/src/chain.rs | 13 ++++++++++--- chain/src/pipe.rs | 22 ++++++---------------- chain/src/store.rs | 45 ++++++++++++++++++++------------------------- chain/src/types.rs | 4 ++++ grin/src/sync.rs | 13 ++++--------- 5 files changed, 44 insertions(+), 53 deletions(-) diff --git a/chain/src/chain.rs b/chain/src/chain.rs index d25138826..d76a93aac 100644 --- a/chain/src/chain.rs +++ b/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 = Tip::new(gen.hash()); + let tip = chain_store.head().unwrap(); 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, diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index 30a2dce95..056a144c6 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -245,22 +245,12 @@ fn validate_block( let mut hashes = vec![]; loop { let curr_header = ctx.store.get_block_header(¤t)?; - match ctx.store.get_header_by_height(curr_header.height) { - Ok(height_header) => { - if curr_header.hash() != height_header.hash() { - hashes.insert(0, curr_header.hash()); - current = curr_header.previous; - } else { - 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"))); - } + + if let Ok(_) = ctx.store.is_on_current_chain(&curr_header) { + break; + } else { + hashes.insert(0, curr_header.hash()); + current = curr_header.previous; } } diff --git a/chain/src/store.rs b/chain/src/store.rs index c6b403d73..6164b432b 100644 --- a/chain/src/store.rs +++ b/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)?; - if block_header.hash() == header_at_height.hash() { - Ok(block_header) - } else { - Err(Error::NotFoundErr) - } + self.is_on_current_chain(&block_header)?; + Ok(block_header) } 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)?; - if bh.height == 0 { + .put_ser(&u64_to_key(HEADER_HEIGHT_PREFIX, header.height), header)?; + + if header.height == 0 { return Ok(()); } - let prev_h = bh.previous; - let prev_height = bh.height - 1; - match self.get_header_by_height(prev_height) { - Ok(prev) => { - if prev.hash() != prev_h { - 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) + let prev_header = self.get_block_header(&header.previous)?; + if let Ok(_) = self.is_on_current_chain(&prev_header) { + Ok(()) + } else { + self.setup_height(&prev_header) } } } diff --git a/chain/src/types.rs b/chain/src/types.rs index 9521c633e..bcd7ef56b 100644 --- a/chain/src/types.rs +++ b/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; + /// 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; diff --git a/grin/src/sync.rs b/grin/src/sync.rs index 000450072..d285e5ddd 100644 --- a/grin/src/sync.rs +++ b/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 - // that is consistent with the height index (we know this is in the real chain) - match chain.get_header_by_height(header.height) { - Ok(height_header) => { - if header.hash() == height_header.hash() { - break; - } - }, - Err(_) => {}, + // break out of the while loop when we find a header common + // between the this chain and the current chain + if let Ok(_) = chain.is_on_current_chain(&header) { + break; } hashes.push(header.hash());