From 399a313653b6a79061ceb32dc552a829402eeb7d Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Fri, 17 Apr 2020 10:35:42 +0100 Subject: [PATCH 1/2] check_known now takes total_difficulty into consideration --- chain/src/pipe.rs | 49 ++++++++++++-------- chain/tests/test_block_known.rs | 81 +++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 18 deletions(-) create mode 100644 chain/tests/test_block_known.rs diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index 66651f51c3..226365bb2e 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -45,12 +45,17 @@ pub struct BlockContext<'a> { pub verifier_cache: Arc>, } -// Check if we already know about this block for various reasons -// from cheapest to most expensive (delay hitting the db until last). -fn check_known(header: &BlockHeader, ctx: &mut BlockContext<'_>) -> Result<(), Error> { - check_known_head(header, ctx)?; - check_known_store(header, ctx)?; - Ok(()) +// If this block has greater total difficulty than treat as unknown in current context. +// If it matches current chain head (latest or previous hash) then we know about it. +// If it exists in the local db then we know about it. +fn check_known(header: &BlockHeader, head: &Tip, ctx: &BlockContext<'_>) -> Result<(), Error> { + if header.total_difficulty() > head.total_difficulty { + Ok(()) + } else { + check_known_head(header, head)?; + check_known_store(header, head, ctx)?; + Ok(()) + } } // Validate only the proof of work in a block header. @@ -86,15 +91,18 @@ pub fn process_block(b: &Block, ctx: &mut BlockContext<'_>) -> Result) -> // Check this header is not an orphan, we must know about the previous header to continue. let prev_header = ctx.batch.get_previous_header(&header)?; - // Check if we know about the full block for this header. - if check_known(header, ctx).is_err() { - return Ok(()); + // If we have already processed the full block for this header then done. + // Note: "already known" in this context is success so subsequent processing can continue. + { + let head = ctx.batch.head()?; + if check_known(header, &head, ctx).is_err() { + return Ok(()); + } } // If we have not yet seen the full block then check if we have seen this header. @@ -267,11 +279,9 @@ pub fn process_block_header(header: &BlockHeader, ctx: &mut BlockContext<'_>) -> Ok(()) } -/// Quick in-memory check to fast-reject any block handled recently. -/// Keeps duplicates from the network in check. -/// Checks against the last_block_h and prev_block_h of the chain head. -fn check_known_head(header: &BlockHeader, ctx: &mut BlockContext<'_>) -> Result<(), Error> { - let head = ctx.batch.head()?; +/// Quick check to reject recently handled blocks. +/// Checks against last_block_h and prev_block_h of the chain head. +fn check_known_head(header: &BlockHeader, head: &Tip) -> Result<(), Error> { let bh = header.hash(); if bh == head.last_block_h || bh == head.prev_block_h { return Err(ErrorKind::Unfit("already known in head".to_string()).into()); @@ -280,10 +290,13 @@ fn check_known_head(header: &BlockHeader, ctx: &mut BlockContext<'_>) -> Result< } // Check if this block is in the store already. -fn check_known_store(header: &BlockHeader, ctx: &mut BlockContext<'_>) -> Result<(), Error> { +fn check_known_store( + header: &BlockHeader, + head: &Tip, + ctx: &BlockContext<'_>, +) -> Result<(), Error> { match ctx.batch.block_exists(&header.hash()) { Ok(true) => { - let head = ctx.batch.head()?; if header.height < head.height.saturating_sub(50) { // TODO - we flag this as an "abusive peer" but only in the case // where we have the full block in our store. diff --git a/chain/tests/test_block_known.rs b/chain/tests/test_block_known.rs new file mode 100644 index 0000000000..2ee0a852d6 --- /dev/null +++ b/chain/tests/test_block_known.rs @@ -0,0 +1,81 @@ +// Copyright 2020 The Grin Developers +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +mod chain_test_helper; +use self::chain_test_helper::{clean_output_dir, init_chain, mine_chain}; +use chain::ErrorKind; +use chain::Tip; +use grin_chain as chain; +use grin_core::core::hash::Hashed; +use grin_util as util; + +#[test] +fn check_known() { + let chain_dir = ".grin.check_known"; + util::init_test_logger(); + clean_output_dir(chain_dir); + + // mine some blocks + let (latest, genesis) = { + let chain = mine_chain(chain_dir, 3); + let genesis = chain + .get_block(&chain.get_header_by_height(0).unwrap().hash()) + .unwrap(); + let head = chain.head().unwrap(); + let latest = chain.get_block(&head.last_block_h).unwrap(); + (latest, genesis) + }; + + // attempt to reprocess latest block + { + let chain = init_chain(chain_dir, genesis.clone()); + let res = chain.process_block(latest.clone(), chain::Options::NONE); + assert_eq!( + res.unwrap_err().kind(), + ErrorKind::Unfit("already known in head".to_string()).into() + ); + } + + // attempt to reprocess genesis block + { + let chain = init_chain(chain_dir, genesis.clone()); + let res = chain.process_block(genesis.clone(), chain::Options::NONE); + assert_eq!( + res.unwrap_err().kind(), + ErrorKind::Unfit("already known in store".to_string()).into() + ); + } + + // reset chain head to earlier state + { + let chain = init_chain(chain_dir, genesis.clone()); + let store = chain.store(); + let batch = store.batch().unwrap(); + let head_header = chain.head_header().unwrap(); + let prev = batch.get_previous_header(&head_header).unwrap(); + batch.save_body_head(&Tip::from_header(&prev)).unwrap(); + batch.commit().unwrap(); + } + + // reprocess latest block and check the updated head + { + let chain = init_chain(chain_dir, genesis.clone()); + let head = chain + .process_block(latest.clone(), chain::Options::NONE) + .unwrap(); + assert_eq!(head, Some(Tip::from_header(&latest.header))); + } + + clean_output_dir(chain_dir); +} From bec9c400d5519da63e958c2f70deb6f55a74747d Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 4 May 2020 16:05:30 +0100 Subject: [PATCH 2/2] update based on feedback --- chain/src/pipe.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index 226365bb2e..8f6bc34337 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -49,13 +49,11 @@ pub struct BlockContext<'a> { // If it matches current chain head (latest or previous hash) then we know about it. // If it exists in the local db then we know about it. fn check_known(header: &BlockHeader, head: &Tip, ctx: &BlockContext<'_>) -> Result<(), Error> { - if header.total_difficulty() > head.total_difficulty { - Ok(()) - } else { + if header.total_difficulty() <= head.total_difficulty { check_known_head(header, head)?; check_known_store(header, head, ctx)?; - Ok(()) } + Ok(()) } // Validate only the proof of work in a block header.