From 75b72e48ff7117664d54866d6a596add14f36af7 Mon Sep 17 00:00:00 2001 From: Gary Yu Date: Mon, 23 Jul 2018 03:25:56 +0800 Subject: [PATCH] Fix bug on block sync (#1271) (#1281) Do not add orphans to the "already seen" cache. --- chain/src/chain.rs | 26 ++++++++++++++++++++------ chain/src/error.rs | 2 +- chain/src/pipe.rs | 14 ++++++++------ 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/chain/src/chain.rs b/chain/src/chain.rs index 0fbd0dfdd..05396cb8c 100644 --- a/chain/src/chain.rs +++ b/chain/src/chain.rs @@ -197,7 +197,7 @@ impl Chain { b: Block, opts: Options, ) -> Result<(Option, Option), Error> { - let res = self.process_block_no_orphans(b, opts); + let res = self.process_block_no_orphans(b, opts, false); match res { Ok((t, b)) => { // We accepted a block, so see if we can accept any orphans @@ -217,13 +217,14 @@ impl Chain { &self, b: Block, opts: Options, + from_cache: bool, ) -> Result<(Option, Option), Error> { let head = self.store.head()?; let mut ctx = self.ctx_from_head(head, opts)?; - let res = pipe::process_block(&b, &mut ctx); - - { + let res = pipe::process_block(&b, &mut ctx, from_cache); + + if !from_cache{ let mut cache = self.block_hashes_cache.write().unwrap(); cache.push_front(b.hash()); cache.truncate(HASHES_CACHE_SIZE); @@ -350,7 +351,7 @@ impl Chain { pub fn check_orphans(&self, mut height: u64) { trace!( LOGGER, - "chain: check_orphans at {}, # orphans {}", + "chain: doing check_orphans at {}, # orphans {}", height, self.orphans.len(), ); @@ -358,7 +359,14 @@ impl Chain { loop { if let Some(orphans) = self.orphans.remove_by_height(&height) { for orphan in orphans { - let res = self.process_block_no_orphans(orphan.block, orphan.opts); + trace!( + LOGGER, + "chain: got block {} at {} from orphans. # orphans remaining {}", + orphan.block.hash(), + height, + self.orphans.len(), + ); + let res = self.process_block_no_orphans(orphan.block, orphan.opts, true); if let Ok((_, Some(b))) = res { // We accepted a block, so see if we can accept any orphans height = b.header.height + 1; @@ -370,6 +378,12 @@ impl Chain { break; } } + trace!( + LOGGER, + "chain: done check_orphans at {}. # remaining orphans {}", + height, + self.orphans.len(), + ); } /// For the given commitment find the unspent output and return the diff --git a/chain/src/error.rs b/chain/src/error.rs index d9c774d1f..8d2c676cc 100644 --- a/chain/src/error.rs +++ b/chain/src/error.rs @@ -52,7 +52,7 @@ pub enum ErrorKind { #[fail(display = "Invalid PoW")] InvalidPow, /// Peer abusively sending us an old block we already have - #[fail(display = "Invalid PoW")] + #[fail(display = "Old Block")] OldBlock, /// The block doesn't sum correctly or a tx signature is invalid #[fail(display = "Invalid Block Proof")] diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index 9c9ae577a..77e5e9a6d 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -55,7 +55,7 @@ pub struct BlockContext { /// Runs the block processing pipeline, including validation and finding a /// place for the new block in the chain. Returns the new chain head if /// updated. -pub fn process_block(b: &Block, ctx: &mut BlockContext) -> Result, Error> { +pub fn process_block(b: &Block, ctx: &mut BlockContext, from_cache: bool,) -> Result, Error> { // TODO should just take a promise for a block with a full header so we don't // spend resources reading the full block when its header is invalid @@ -68,7 +68,9 @@ pub fn process_block(b: &Block, ctx: &mut BlockContext) -> Result, E b.outputs.len(), b.kernels.len(), ); - check_known(b.hash(), ctx)?; + if !from_cache { + check_known(b.hash(), ctx)?; + } validate_header(&b.header, ctx)?; @@ -183,9 +185,9 @@ fn check_header_known(bh: Hash, ctx: &mut BlockContext) -> Result<(), Error> { if bh == ctx.head.last_block_h || bh == ctx.head.prev_block_h { return Err(ErrorKind::Unfit("already known".to_string()).into()); } - let cache = ctx.block_hashes_cache.read().unwrap(); + let cache = ctx.header_hashes_cache.read().unwrap(); if cache.contains(&bh) { - return Err(ErrorKind::Unfit("already known".to_string()).into()); + return Err(ErrorKind::Unfit("already known in cache".to_string()).into()); } Ok(()) } @@ -196,9 +198,9 @@ fn check_known(bh: Hash, ctx: &mut BlockContext) -> Result<(), Error> { if bh == ctx.head.last_block_h || bh == ctx.head.prev_block_h { return Err(ErrorKind::Unfit("already known".to_string()).into()); } - let cache = ctx.header_hashes_cache.read().unwrap(); + let cache = ctx.block_hashes_cache.read().unwrap(); if cache.contains(&bh) { - return Err(ErrorKind::Unfit("already known".to_string()).into()); + return Err(ErrorKind::Unfit("already known in cache".to_string()).into()); } Ok(()) }