Fix bug on block sync (#1271) (#1281)

Do not add orphans to the "already seen" cache.

chain/src/chain.rs

@@ -197,7 +197,7 @@ impl Chain {
 		b: Block,
 		opts: Options,
 	) -> Result<(Option<Tip>, Option<Block>), 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<Tip>, Option<Block>), 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

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")]

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<Option<Tip>, Error> {
+pub fn process_block(b: &Block, ctx: &mut BlockContext, from_cache: bool,) -> Result<Option<Tip>, 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<Option<Tip>, E
 		b.outputs.len(),
 		b.kernels.len(),
 	);
+	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(())
 }