crc: code cleanup in signer_state

Author: hstove

stacks-signer/src/v0/signer_state.rs

@@ -350,7 +350,7 @@ pub enum LocalStateMachine {
 /// A pending update for a signer state machine
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub enum StateMachineUpdate {
-    /// A new burn block at height u64 is expected
+    /// A new burn block is expected
     BurnBlock(NewBurnBlock),
 }
 
@@ -419,7 +419,7 @@ impl LocalStateMachine {
             },
             other => {
                 return Err(CodecError::DeserializeError(format!(
-                    "Active signer ptocol version is unknown: {other}"
+                    "Active signer protocol version is unknown: {other}"
                 )))
             }
         };
@@ -780,13 +780,16 @@ impl LocalStateMachine {
         if let Some(expected_burn_block) = expected_burn_block {
             // If the next height is less than the expected height, we need to wait.
             // OR if the next height is the same, but with a different hash, we need to wait.
-            if next_burn_block_height < expected_burn_block.burn_block_height || {
-                next_burn_block_height == expected_burn_block.burn_block_height
-                    && next_burn_block_hash != expected_burn_block.consensus_hash
-            } {
+            let node_behind_expected =
+                next_burn_block_height < expected_burn_block.burn_block_height;
+            let node_on_equal_fork = next_burn_block_height
+                == expected_burn_block.burn_block_height
+                && next_burn_block_hash != expected_burn_block.consensus_hash;
+            if node_behind_expected || node_on_equal_fork {
                 let err_msg = format!(
-                    "Node has not processed the next burn block ({}) yet",
-                    expected_burn_block.burn_block_height
+                    "Node has not processed the next burn block yet. Expected height = {}, Expected consensus hash = {}",
+                    expected_burn_block.burn_block_height,
+                    expected_burn_block.consensus_hash,
                 );
                 *self = Self::Pending {
                     update: StateMachineUpdate::BurnBlock(expected_burn_block),
@@ -1028,62 +1031,63 @@ impl LocalStateMachine {
         prior_state_machine: &SignerStateMachine,
         is_in_tx_replay_mode: bool,
     ) -> Result<Option<Vec<StacksTransaction>>, SignerChainstateError> {
-        if expected_burn_block.burn_block_height <= prior_state_machine.burn_block_height
-                && expected_burn_block.consensus_hash != prior_state_machine.burn_block
-                // TODO: handle fork while still in replay
-                && !is_in_tx_replay_mode
-        {
-            info!("Signer State: fork detected";
-                "expected_burn_block.height" => expected_burn_block.burn_block_height,
-                "expected_burn_block.hash" => %expected_burn_block.consensus_hash,
-                "prior_state_machine.burn_block_height" => prior_state_machine.burn_block_height,
-                "prior_state_machine.burn_block" => %prior_state_machine.burn_block,
-            );
-            // Determine the tenures that were forked
-            let mut parent_burn_block_info =
-                db.get_burn_block_by_ch(&prior_state_machine.burn_block)?;
-            let last_forked_tenure = prior_state_machine.burn_block;
-            let mut first_forked_tenure = prior_state_machine.burn_block;
-            let mut forked_tenures = vec![(
-                prior_state_machine.burn_block,
-                prior_state_machine.burn_block_height,
-            )];
-            while parent_burn_block_info.block_height > expected_burn_block.burn_block_height {
-                parent_burn_block_info =
-                    db.get_burn_block_by_hash(&parent_burn_block_info.parent_burn_block_hash)?;
-                first_forked_tenure = parent_burn_block_info.consensus_hash;
-                forked_tenures.push((
-                    parent_burn_block_info.consensus_hash,
-                    parent_burn_block_info.block_height,
-                ));
-            }
-            let fork_info =
-                client.get_tenure_forking_info(&first_forked_tenure, &last_forked_tenure)?;
-            let mut forked_blocks = fork_info
-                .iter()
-                .flat_map(|fork_info| {
-                    fork_info
-                        .nakamoto_blocks
-                        .iter()
-                        .flat_map(|blocks| blocks.iter())
-                })
-                .collect::<Vec<_>>();
-            forked_blocks.sort_by_key(|block| block.header.chain_length);
-            let forked_txs = forked_blocks
-                .iter()
-                .flat_map(|block| block.txs.iter())
-                .filter(|tx| match tx.payload {
-                    // Don't include Coinbase, TenureChange, or PoisonMicroblock transactions
-                    TransactionPayload::TenureChange(..)
-                    | TransactionPayload::Coinbase(..)
-                    | TransactionPayload::PoisonMicroblock(..) => false,
-                    _ => true,
-                })
-                .cloned()
-                .collect::<Vec<_>>();
-            Ok(Some(forked_txs))
-        } else {
-            Ok(None)
+        if expected_burn_block.burn_block_height > prior_state_machine.burn_block_height {
+            // no bitcoin fork, because we're advancing the burn block height
+            return Ok(None);
+        }
+        if expected_burn_block.consensus_hash == prior_state_machine.burn_block {
+            // no bitcoin fork, because we're at the same burn block hash as before
+            return Ok(None);
         }
+        if is_in_tx_replay_mode {
+            // TODO: handle fork while still in replay
+            info!("Detected bitcoin fork while in replay mode, will not try to handle the fork");
+            return Ok(None);
+        }
+        info!("Signer State: fork detected";
+            "expected_burn_block.height" => expected_burn_block.burn_block_height,
+            "expected_burn_block.hash" => %expected_burn_block.consensus_hash,
+            "prior_state_machine.burn_block_height" => prior_state_machine.burn_block_height,
+            "prior_state_machine.burn_block" => %prior_state_machine.burn_block,
+        );
+        // Determine the tenures that were forked
+        let mut parent_burn_block_info =
+            db.get_burn_block_by_ch(&prior_state_machine.burn_block)?;
+        let last_forked_tenure = prior_state_machine.burn_block;
+        let mut first_forked_tenure = prior_state_machine.burn_block;
+        let mut forked_tenures = vec![(
+            prior_state_machine.burn_block,
+            prior_state_machine.burn_block_height,
+        )];
+        while parent_burn_block_info.block_height > expected_burn_block.burn_block_height {
+            parent_burn_block_info =
+                db.get_burn_block_by_hash(&parent_burn_block_info.parent_burn_block_hash)?;
+            first_forked_tenure = parent_burn_block_info.consensus_hash;
+            forked_tenures.push((
+                parent_burn_block_info.consensus_hash,
+                parent_burn_block_info.block_height,
+            ));
+        }
+        let fork_info =
+            client.get_tenure_forking_info(&first_forked_tenure, &last_forked_tenure)?;
+        let mut forked_blocks = fork_info
+            .iter()
+            .flat_map(|fork_info| fork_info.nakamoto_blocks.iter().flatten())
+            .collect::<Vec<_>>();
+        forked_blocks.sort_by_key(|block| block.header.chain_length);
+        let forked_txs = forked_blocks
+            .iter()
+            .flat_map(|block| block.txs.iter())
+            .filter(|tx|
+                // Don't include Coinbase, TenureChange, or PoisonMicroblock transactions
+                !matches!(
+                    tx.payload,
+                    TransactionPayload::TenureChange(..)
+                        | TransactionPayload::Coinbase(..)
+                        | TransactionPayload::PoisonMicroblock(..)
+                ))
+            .cloned()
+            .collect::<Vec<_>>();
+        Ok(Some(forked_txs))
     }
 }