From 2f53ff44f7cac079b45bdfd758efae20d7a2fbe7 Mon Sep 17 00:00:00 2001
From: teor <teor@riseup.net>
Date: Tue, 17 Nov 2020 12:25:35 +1000
Subject: [PATCH] Move chain order assertions to commit_finalized_direct

And remove a duplicate assert in the contextual verification function.
---
 zebra-state/src/service/check.rs |  3 --
 zebra-state/src/sled_state.rs    | 70 ++++++++++++++------------------
 2 files changed, 30 insertions(+), 43 deletions(-)

diff --git a/zebra-state/src/service/check.rs b/zebra-state/src/service/check.rs
index f5a72700..b7a6a18c 100644
--- a/zebra-state/src/service/check.rs
+++ b/zebra-state/src/service/check.rs
@@ -49,10 +49,7 @@ where
     let parent_height = parent_block
         .coinbase_height()
         .expect("valid blocks have a coinbase height");
-    let parent_hash = parent_block.hash();
     check::height_one_more_than_parent_height(parent_height, block)?;
-    // should be impossible by design, so no handleable error is thrown
-    assert_eq!(parent_hash, block.header.previous_block_hash);
 
     // TODO: validate difficulty adjustment
     // TODO: other contextual validation design and implelentation
diff --git a/zebra-state/src/sled_state.rs b/zebra-state/src/sled_state.rs
index 62a3a2b0..31f99da8 100644
--- a/zebra-state/src/sled_state.rs
+++ b/zebra-state/src/sled_state.rs
@@ -148,45 +148,7 @@ impl FinalizedState {
         let height = queued_block.block.coinbase_height().unwrap();
         self.queued_by_prev_hash.insert(prev_hash, queued_block);
 
-        loop {
-            let finalized_tip_hash = self.finalized_tip_hash();
-            let queued_block =
-                if let Some(queued_block) = self.queued_by_prev_hash.remove(&finalized_tip_hash) {
-                    queued_block
-                } else {
-                    break;
-                };
-
-            let height = queued_block
-                .block
-                .coinbase_height()
-                .expect("valid blocks must have a height");
-
-            if self.block_by_height.is_empty() {
-                assert_eq!(
-                    block::Hash([0; 32]),
-                    prev_hash,
-                    "the first block added to an empty state must be a genesis block"
-                );
-                assert_eq!(
-                    block::Height(0),
-                    height,
-                    "cannot commit genesis: invalid height"
-                );
-            } else {
-                assert_eq!(
-                    self.finalized_tip_height()
-                        .expect("state must have a genesis block committed")
-                        + 1,
-                    Some(height)
-                );
-
-                assert_eq!(
-                    finalized_tip_hash,
-                    queued_block.block.header.previous_block_hash
-                );
-            }
-
+        while let Some(queued_block) = self.queued_by_prev_hash.remove(&self.finalized_tip_hash()) {
             self.commit_finalized(queued_block);
             metrics::counter!("state.finalized.committed.block.count", 1);
             metrics::gauge!("state.finalized.committed.block.height", height.0 as _);
@@ -229,7 +191,35 @@ impl FinalizedState {
             .expect("finalized blocks are valid and have a coinbase height");
         let hash = block.hash();
 
-        trace!(?height, "Finalized block");
+        trace!(?height, ?hash, "Finalized block");
+
+        // Assert that callers (including unit tests) get the chain order correct
+        if self.block_by_height.is_empty() {
+            assert_eq!(
+                block::Hash([0; 32]),
+                block.header.previous_block_hash,
+                "the first block added to an empty state must be a genesis block"
+            );
+            assert_eq!(
+                block::Height(0),
+                height,
+                "cannot commit genesis: invalid height"
+            );
+        } else {
+            assert_eq!(
+                self.finalized_tip_height()
+                    .expect("state must have a genesis block committed")
+                    + 1,
+                Some(height),
+                "committed block height must be 1 more than the finalized tip height"
+            );
+
+            assert_eq!(
+                self.finalized_tip_hash(),
+                block.header.previous_block_hash,
+                "committed block must be a child of the finalized tip"
+            );
+        }
 
         let result = (
             &self.hash_by_height,