From 207ded6889404c08322878d9375e4edaa5a34fdd Mon Sep 17 00:00:00 2001
From: teor <teor@riseup.net>
Date: Thu, 3 Dec 2020 14:48:34 +1000
Subject: [PATCH] Add error context for contextual validation

---
 zebra-state/src/error.rs         | 40 +++++++++++++++++++++--------
 zebra-state/src/service/check.rs | 43 +++++++++++++++++++++-----------
 2 files changed, 59 insertions(+), 24 deletions(-)

diff --git a/zebra-state/src/error.rs b/zebra-state/src/error.rs
index e7425bf9..7a8ff794 100644
--- a/zebra-state/src/error.rs
+++ b/zebra-state/src/error.rs
@@ -1,6 +1,10 @@
 use std::sync::Arc;
+
+use chrono::{DateTime, Utc};
 use thiserror::Error;
 
+use zebra_chain::{block, work::difficulty::CompactDifficulty};
+
 /// A wrapper for type erased errors that is itself clonable and implements the
 /// Error trait
 #[derive(Debug, Error, Clone)]
@@ -34,24 +38,40 @@ pub struct CommitBlockError(#[from] ValidateContextError);
 /// An error describing why a block failed contextual validation.
 #[derive(displaydoc::Display, Debug, Error)]
 #[non_exhaustive]
+#[allow(missing_docs)]
 pub enum ValidateContextError {
-    /// block.height is lower than the current finalized height
+    /// block height {candidate_height:?} is lower than the current finalized height {finalized_tip_height:?}
     #[non_exhaustive]
-    OrphanedBlock,
+    OrphanedBlock {
+        candidate_height: block::Height,
+        finalized_tip_height: block::Height,
+    },
 
-    /// block.height is not one greater than its parent block's height
+    /// block height {candidate_height:?} is not one greater than its parent block's height {parent_height:?}
     #[non_exhaustive]
-    NonSequentialBlock,
+    NonSequentialBlock {
+        candidate_height: block::Height,
+        parent_height: block::Height,
+    },
 
-    /// block.header.time is less than or equal to the median-time-past for the block
+    /// block time {candidate_time:?} is less than or equal to the median-time-past for the block {median_time_past:?}
     #[non_exhaustive]
-    TimeTooEarly,
+    TimeTooEarly {
+        candidate_time: DateTime<Utc>,
+        median_time_past: DateTime<Utc>,
+    },
 
-    /// block.header.time is greater than the median-time-past for the block plus 90 minutes
+    /// block time {candidate_time:?} is greater than the median-time-past for the block plus 90 minutes {block_time_max:?}
     #[non_exhaustive]
-    TimeTooLate,
+    TimeTooLate {
+        candidate_time: DateTime<Utc>,
+        block_time_max: DateTime<Utc>,
+    },
 
-    /// block.header.difficulty_threshold is not equal to the adjusted difficulty for the block
+    /// block difficulty threshold {difficulty_threshold:?} is not equal to the expected difficulty for the block {expected_difficulty:?}
     #[non_exhaustive]
-    InvalidDifficultyThreshold,
+    InvalidDifficultyThreshold {
+        difficulty_threshold: CompactDifficulty,
+        expected_difficulty: CompactDifficulty,
+    },
 }
diff --git a/zebra-state/src/service/check.rs b/zebra-state/src/service/check.rs
index 1e6b2225..d36f1587 100644
--- a/zebra-state/src/service/check.rs
+++ b/zebra-state/src/service/check.rs
@@ -90,10 +90,13 @@ where
 /// block is less than or equal to the finalized tip height.
 fn block_is_not_orphaned(
     finalized_tip_height: block::Height,
-    height: block::Height,
+    candidate_height: block::Height,
 ) -> Result<(), ValidateContextError> {
-    if height <= finalized_tip_height {
-        Err(ValidateContextError::OrphanedBlock)
+    if candidate_height <= finalized_tip_height {
+        Err(ValidateContextError::OrphanedBlock {
+            candidate_height,
+            finalized_tip_height,
+        })
     } else {
         Ok(())
     }
@@ -103,10 +106,13 @@ fn block_is_not_orphaned(
 /// equal to the parent_height+1.
 fn height_one_more_than_parent_height(
     parent_height: block::Height,
-    height: block::Height,
+    candidate_height: block::Height,
 ) -> Result<(), ValidateContextError> {
-    if parent_height + 1 != Some(height) {
-        Err(ValidateContextError::NonSequentialBlock)
+    if parent_height + 1 != Some(candidate_height) {
+        Err(ValidateContextError::NonSequentialBlock {
+            candidate_height,
+            parent_height,
+        })
     } else {
         Ok(())
     }
@@ -127,19 +133,28 @@ fn difficulty_threshold_is_valid(
     difficulty_adjustment: AdjustedDifficulty,
 ) -> Result<(), ValidateContextError> {
     // Check the block header time consensus rules from the Zcash specification
+    let candidate_time = difficulty_adjustment.candidate_time();
     let median_time_past = difficulty_adjustment.median_time_past();
-    let block_max_time_since_median = Duration::seconds(difficulty::BLOCK_MAX_TIME_SINCE_MEDIAN);
-    if difficulty_adjustment.candidate_time() <= median_time_past {
-        Err(ValidateContextError::TimeTooEarly)?
-    } else if difficulty_adjustment.candidate_time()
-        > median_time_past + block_max_time_since_median
-    {
-        Err(ValidateContextError::TimeTooLate)?
+    let block_time_max =
+        median_time_past + Duration::seconds(difficulty::BLOCK_MAX_TIME_SINCE_MEDIAN);
+    if candidate_time <= median_time_past {
+        Err(ValidateContextError::TimeTooEarly {
+            candidate_time,
+            median_time_past,
+        })?
+    } else if candidate_time > block_time_max {
+        Err(ValidateContextError::TimeTooLate {
+            candidate_time,
+            block_time_max,
+        })?
     }
 
     let expected_difficulty = difficulty_adjustment.expected_difficulty_threshold();
     if difficulty_threshold != expected_difficulty {
-        Err(ValidateContextError::InvalidDifficultyThreshold)?
+        Err(ValidateContextError::InvalidDifficultyThreshold {
+            difficulty_threshold,
+            expected_difficulty,
+        })?
     }
 
     Ok(())