Merge #508

508: NDRS-571: LinearChainSync: Verify parent hashes. r=afck a=afck

https://casperlabs.atlassian.net/browse/NDRS-571

Co-authored-by: Andreas Fackler <andreas@casperlabs.io>

node/src/components/linear_chain_sync.rs

@@ -75,18 +75,16 @@ enum State {
         /// Chain of downloaded blocks from the linear chain.
         /// We will `pop()` when executing blocks.
         linear_chain: Vec<BlockHeader>,
-        /// Block being downloaded.
-        /// Block we received from a node and are currently executing.
-        /// Will be used to verify whether results we got from the execution are the same.
-        current_block: Box<Option<BlockHeader>>,
+        /// The most recent block we started to execute. This is updated whenever we start
+        /// downloading deploys for the next block to be executed.
+        latest_block: Box<Option<BlockHeader>>,
     },
     /// Synchronizing the descendants of the trusted hash.
     SyncingDescendants {
         trusted_hash: BlockHash,
-        /// Linear chain block being downloaded.
-        linear_chain_block: Box<Option<BlockHeader>>,
-        /// Block we received from a node and are currently executing.
-        current_block: Box<Option<BlockHeader>>,
+        /// The most recent block we started to execute. This is updated whenever we start
+        /// downloading deploys for the next block to be executed.
+        latest_block: Box<BlockHeader>,
         /// During synchronization we might see new eras being created.
         /// Track the highest height and wait until it's handled by consensus.
         highest_block_seen: u64,
@@ -120,15 +118,14 @@ impl State {
             trusted_hash,
             highest_block_seen: 0,
             linear_chain: Vec::new(),
-            current_block: Box::new(None),
+            latest_block: Box::new(None),
         }
     }
 
-    fn sync_descendants(trusted_hash: BlockHash) -> Self {
+    fn sync_descendants(trusted_hash: BlockHash, latest_block: BlockHeader) -> Self {
         State::SyncingDescendants {
             trusted_hash,
-            linear_chain_block: Box::new(None),
-            current_block: Box::new(None),
+            latest_block: Box::new(latest_block),
             highest_block_seen: 0,
         }
     }
@@ -200,9 +197,7 @@ impl<I: Clone + PartialEq + 'static> LinearChainSync<I> {
         match &mut self.state {
             State::None | State::Done => {}
             State::SyncingTrustedHash { linear_chain, .. } => linear_chain.push(block_header),
-            State::SyncingDescendants {
-                linear_chain_block, ..
-            } => *linear_chain_block = Box::new(Some(block_header)),
+            State::SyncingDescendants { latest_block, .. } => **latest_block = block_header,
         };
     }
 
@@ -277,10 +272,10 @@ impl<I: Clone + PartialEq + 'static> LinearChainSync<I> {
             State::SyncingTrustedHash {
                 highest_block_seen,
                 trusted_hash,
-                ref current_block,
+                ref latest_block,
                 ..
             } => {
-                match current_block.as_ref() {
+                match latest_block.as_ref() {
                     Some(expected) => assert_eq!(
                         expected, &block_header,
                         "Block execution result doesn't match received block."
@@ -291,23 +286,20 @@ impl<I: Clone + PartialEq + 'static> LinearChainSync<I> {
                     info!(%block_height, "Finished synchronizing linear chain up until trusted hash.");
                     let peer = self.random_peer_unsafe();
                     // Kick off syncing trusted hash descendants.
-                    self.state = State::sync_descendants(trusted_hash);
+                    self.state = State::sync_descendants(trusted_hash, block_header);
                     fetch_block_at_height(effect_builder, peer, block_height + 1)
                 } else {
                     self.state = curr_state;
                     self.fetch_next_block_deploys(effect_builder)
                 }
             }
             State::SyncingDescendants {
-                ref current_block, ..
+                ref latest_block, ..
             } => {
-                match current_block.as_ref() {
-                    Some(expected) => assert_eq!(
-                        expected, &block_header,
-                        "Block execution result doesn't match received block."
-                    ),
-                    None => panic!("Unexpected block execution results."),
-                }
+                assert_eq!(
+                    **latest_block, block_header,
+                    "Block execution result doesn't match received block."
+                );
                 self.state = curr_state;
                 self.fetch_next_block(effect_builder, rng, &block_header)
             }
@@ -325,36 +317,24 @@ impl<I: Clone + PartialEq + 'static> LinearChainSync<I> {
     {
         let peer = self.random_peer_unsafe();
 
-        let next_block = match self.state {
+        let next_block = match &mut self.state {
             State::None | State::Done => {
                 panic!("Tried fetching next block when in {:?} state.", self.state)
             }
             State::SyncingTrustedHash {
-                ref mut linear_chain,
-                ref mut current_block,
+                linear_chain,
+                latest_block,
                 ..
             } => match linear_chain.pop() {
                 None => None,
                 Some(block) => {
-                    // Update `current_block` so that we can verify whether result of execution
-                    // matches the expected value.
-                    current_block.replace(block.clone());
-                    Some(block)
-                }
-            },
-            State::SyncingDescendants {
-                ref mut linear_chain_block,
-                ref mut current_block,
-                ..
-            } => match linear_chain_block.take() {
-                None => None,
-                Some(block) => {
-                    // Update `current_block` so that we can verify whether result of execution
+                    // Update `latest_block` so that we can verify whether result of execution
                     // matches the expected value.
-                    current_block.replace(block.clone());
+                    latest_block.replace(block.clone());
                     Some(block)
                 }
             },
+            State::SyncingDescendants { latest_block, .. } => Some((**latest_block).clone()),
         };
 
         next_block.map_or_else(
@@ -392,6 +372,14 @@ impl<I: Clone + PartialEq + 'static> LinearChainSync<I> {
             }
         }
     }
+
+    fn latest_block(&self) -> Option<&BlockHeader> {
+        match &self.state {
+            State::SyncingTrustedHash { latest_block, .. } => Option::as_ref(&*latest_block),
+            State::SyncingDescendants { latest_block, .. } => Some(&*latest_block),
+            State::Done | State::None => None,
+        }
+    }
 }
 
 impl<I, REv> Component<REv> for LinearChainSync<I>
@@ -445,12 +433,16 @@ where
                     self.block_downloaded(rng, effect_builder, block.header())
                 }
                 BlockByHeightResult::FromPeer(block, peer) => {
-                    if block.height() != block_height {
+                    if block.height() != block_height
+                        || *block.header().parent_hash() != self.latest_block().unwrap().hash()
+                    {
                         warn!(
-                            "Block height mismatch. Expected {} got {} from {}.",
-                            block_height,
-                            block.height(),
-                            peer
+                            %peer,
+                            got_height = block.height(),
+                            expected_height = block_height,
+                            got_parent = %block.header().parent_hash(),
+                            expected_parent = %self.latest_block().unwrap().hash(),
+                            "block mismatch",
                         );
                         // NOTE: Signal misbehaving validator to networking layer.
                         self.ban_peer(peer);