From caa6b8c747788e32953950a2dfd0a7c1e5d29cd2 Mon Sep 17 00:00:00 2001
From: Antioch Peverell <apeverell@protonmail.com>
Date: Thu, 20 Aug 2020 17:28:35 +0100
Subject: [PATCH] further tweaks to block_accepted logs for clarity (#3379)

* further tweaks to block_accepted logs for clarity

* fix tests

* depth based off prev_head
---
 chain/src/chain.rs               | 22 +++++++++++++++----
 chain/src/types.rs               | 12 +++++++----
 chain/tests/mine_simple_chain.rs |  1 +
 servers/src/common/hooks.rs      | 37 +++++++++++++++++++++-----------
 4 files changed, 51 insertions(+), 21 deletions(-)

diff --git a/chain/src/chain.rs b/chain/src/chain.rs
index 87d4709c8..a71abb320 100644
--- a/chain/src/chain.rs
+++ b/chain/src/chain.rs
@@ -306,21 +306,29 @@ impl Chain {
 		})
 	}
 
-	fn determine_status(&self, head: Option<Tip>, prev_head: Tip, fork_point: Tip) -> BlockStatus {
+	fn determine_status(
+		&self,
+		head: Option<Tip>,
+		prev: Tip,
+		prev_head: Tip,
+		fork_point: Tip,
+	) -> BlockStatus {
 		// If head is updated then we are either "next" block or we just experienced a "reorg" to new head.
 		// Otherwise this is a "fork" off the main chain.
 		if let Some(head) = head {
 			if head.prev_block_h == prev_head.last_block_h {
-				BlockStatus::Next { prev_head }
+				BlockStatus::Next { prev }
 			} else {
 				BlockStatus::Reorg {
+					prev,
 					prev_head,
 					fork_point,
 				}
 			}
 		} else {
 			BlockStatus::Fork {
-				prev_head,
+				prev,
+				head: prev_head,
 				fork_point,
 			}
 		}
@@ -416,7 +424,13 @@ impl Chain {
 
 		match maybe_new_head {
 			Ok((head, fork_point)) => {
-				let status = self.determine_status(head, prev_head, Tip::from_header(&fork_point));
+				let prev = self.get_previous_header(&b.header)?;
+				let status = self.determine_status(
+					head,
+					Tip::from_header(&prev),
+					prev_head,
+					Tip::from_header(&fork_point),
+				);
 
 				// notifying other parts of the system of the update
 				self.adapter.block_accepted(&b, status, opts);
diff --git a/chain/src/types.rs b/chain/src/types.rs
index 65f68fc1b..b11c8b588 100644
--- a/chain/src/types.rs
+++ b/chain/src/types.rs
@@ -448,19 +448,23 @@ impl ChainAdapter for NoopAdapter {
 pub enum BlockStatus {
 	/// Block is the "next" block, updating the chain head.
 	Next {
-		/// Previous chain head.
-		prev_head: Tip,
+		/// Previous block (previous chain head).
+		prev: Tip,
 	},
 	/// Block does not update the chain head and is a fork.
 	Fork {
-		/// Previous chain head.
-		prev_head: Tip,
+		/// Previous block on this fork.
+		prev: Tip,
+		/// Current chain head.
+		head: Tip,
 		/// Fork point for rewind.
 		fork_point: Tip,
 	},
 	/// Block updates the chain head via a (potentially disruptive) "reorg".
 	/// Previous block was not our previous chain head.
 	Reorg {
+		/// Previous block on this fork.
+		prev: Tip,
 		/// Previous chain head.
 		prev_head: Tip,
 		/// Fork point for rewind.
diff --git a/chain/tests/mine_simple_chain.rs b/chain/tests/mine_simple_chain.rs
index a55e99707..37dadf36d 100644
--- a/chain/tests/mine_simple_chain.rs
+++ b/chain/tests/mine_simple_chain.rs
@@ -385,6 +385,7 @@ fn mine_reorg() {
 		assert_eq!(
 			*adapter.last_status.read(),
 			Some(BlockStatus::Reorg {
+				prev: Tip::from_header(&fork_head),
 				prev_head: head,
 				fork_point: Tip::from_header(&fork_point)
 			})
diff --git a/servers/src/common/hooks.rs b/servers/src/common/hooks.rs
index d43805e7e..7fc2ceba7 100644
--- a/servers/src/common/hooks.rs
+++ b/servers/src/common/hooks.rs
@@ -119,42 +119,48 @@ impl ChainEvents for EventLogger {
 	fn on_block_accepted(&self, block: &core::Block, status: BlockStatus) {
 		match status {
 			BlockStatus::Reorg {
+				prev,
 				prev_head,
 				fork_point,
 			} => {
 				warn!(
-					"block_accepted (REORG!): {} at {}, (prev_head: {} at {}, fork_point: {} at {}, depth: {})",
+					"block_accepted (REORG!): {} at {}, (prev: {} at {}, prev_head: {} at {}, fork_point: {} at {}, depth: {})",
 					block.hash(),
 					block.header.height,
+					prev.hash(),
+					prev.height,
 					prev_head.hash(),
 					prev_head.height,
 					fork_point.hash(),
 					fork_point.height,
-					block.header.height.saturating_sub(fork_point.height + 1),
+					prev_head.height.saturating_sub(fork_point.height),
 				);
 			}
 			BlockStatus::Fork {
-				prev_head,
+				prev,
+				head,
 				fork_point,
 			} => {
 				debug!(
-					"block_accepted (fork?): {} at {}, (prev_head: {} at {}, fork_point: {} at {}, depth: {})",
+					"block_accepted (fork?): {} at {}, (prev: {} at {}, head: {} at {}, fork_point: {} at {}, depth: {})",
 					block.hash(),
 					block.header.height,
-					prev_head.hash(),
-					prev_head.height,
+					prev.hash(),
+					prev.height,
+					head.hash(),
+					head.height,
 					fork_point.hash(),
 					fork_point.height,
-					block.header.height.saturating_sub(fork_point.height + 1),
+					head.height.saturating_sub(fork_point.height),
 				);
 			}
-			BlockStatus::Next { prev_head } => {
+			BlockStatus::Next { prev } => {
 				debug!(
-					"block_accepted (head+): {} at {} (prev_head: {} at {})",
+					"block_accepted (head+): {} at {} (prev: {} at {})",
 					block.hash(),
 					block.header.height,
-					prev_head.hash(),
-					prev_head.height,
+					prev.hash(),
+					prev.height,
 				);
 			}
 		}
@@ -284,8 +290,13 @@ impl ChainEvents for WebHook {
 		};
 
 		// Add additional `depth` field to the JSON in case of reorg
-		let payload = if let BlockStatus::Reorg { fork_point, .. } = status {
-			let depth = block.header.height.saturating_sub(fork_point.height + 1);
+		let payload = if let BlockStatus::Reorg {
+			fork_point,
+			prev_head,
+			..
+		} = status
+		{
+			let depth = prev_head.height.saturating_sub(fork_point.height);
 			json!({
 				"hash": block.header.hash().to_hex(),
 				"status": status_str,