Merge pull request #6319 from jferrant/feat/signer-two-phase-commit-impl

Feat/signer two phase commit impl

Author: jferrant

clarity/src/vm/tests/representations.rs

@@ -74,7 +74,7 @@ fn prop_clarity_name_valid_patterns() {
         prop_assume!(name.len() <= MAX_STRING_LEN as usize);
 
         let clarity_name = ClarityName::try_from(name.clone())
-            .unwrap_or_else(|_| panic!("Should parse valid clarity name: {}", name));
+            .unwrap_or_else(|_| panic!("Should parse valid clarity name: {name}"));
         prop_assert_eq!(clarity_name.as_str(), name);
     });
 }
@@ -224,7 +224,7 @@ fn prop_contract_name_valid_patterns() {
         prop_assume!(name.len() <= MAX_STRING_LEN as usize);
 
         let contract_name = ContractName::try_from(name.clone())
-            .unwrap_or_else(|_| panic!("Should parse valid contract name: {}", name));
+            .unwrap_or_else(|_| panic!("Should parse valid contract name: {name}"));
         prop_assert_eq!(contract_name.as_str(), name);
     });
 }

libsigner/src/v0/messages.rs

@@ -71,7 +71,9 @@ MessageSlotID {
     /// Block Response message from signers
     BlockResponse = 1,
     /// Signer State Machine Update
-    StateMachineUpdate = 2
+    StateMachineUpdate = 2,
+    /// Block Pre-commit message from signers before they commit to a block response
+    BlockPreCommit = 3
 });
 
 define_u8_enum!(
@@ -114,7 +116,9 @@ SignerMessageTypePrefix {
     /// Mock block message from Epoch 2.5 miners
     MockBlock = 5,
     /// State machine update
-    StateMachineUpdate = 6
+    StateMachineUpdate = 6,
+    /// Block Pre-commit message
+    BlockPreCommit = 7
 });
 
 #[cfg_attr(test, mutants::skip)]
@@ -137,7 +141,7 @@ impl MessageSlotID {
 #[cfg_attr(test, mutants::skip)]
 impl Display for MessageSlotID {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{:?}({})", self, self.to_u8())
+        write!(f, "{self:?}({})", self.to_u8())
     }
 }
 
@@ -161,6 +165,7 @@ impl From<&SignerMessage> for SignerMessageTypePrefix {
             SignerMessage::MockSignature(_) => SignerMessageTypePrefix::MockSignature,
             SignerMessage::MockBlock(_) => SignerMessageTypePrefix::MockBlock,
             SignerMessage::StateMachineUpdate(_) => SignerMessageTypePrefix::StateMachineUpdate,
+            SignerMessage::BlockPreCommit(_) => SignerMessageTypePrefix::BlockPreCommit,
         }
     }
 }
@@ -182,6 +187,8 @@ pub enum SignerMessage {
     MockBlock(MockBlock),
     /// A state machine update
     StateMachineUpdate(StateMachineUpdate),
+    /// The pre-commit message from signers for other signers to observe
+    BlockPreCommit(Sha512Trunc256Sum),
 }
 
 impl SignerMessage {
@@ -197,6 +204,7 @@ impl SignerMessage {
             | Self::MockBlock(_) => None,
             Self::BlockResponse(_) | Self::MockSignature(_) => Some(MessageSlotID::BlockResponse), // Mock signature uses the same slot as block response since its exclusively for epoch 2.5 testing
             Self::StateMachineUpdate(_) => Some(MessageSlotID::StateMachineUpdate),
+            Self::BlockPreCommit(_) => Some(MessageSlotID::BlockPreCommit),
         }
     }
 }
@@ -216,6 +224,9 @@ impl StacksMessageCodec for SignerMessage {
             SignerMessage::StateMachineUpdate(state_machine_update) => {
                 state_machine_update.consensus_serialize(fd)
             }
+            SignerMessage::BlockPreCommit(block_pre_commit) => {
+                block_pre_commit.consensus_serialize(fd)
+            }
         }?;
         Ok(())
     }
@@ -253,6 +264,10 @@ impl StacksMessageCodec for SignerMessage {
                 let state_machine_update = StacksMessageCodec::consensus_deserialize(fd)?;
                 SignerMessage::StateMachineUpdate(state_machine_update)
             }
+            SignerMessageTypePrefix::BlockPreCommit => {
+                let signer_signature_hash = StacksMessageCodec::consensus_deserialize(fd)?;
+                SignerMessage::BlockPreCommit(signer_signature_hash)
+            }
         };
         Ok(message)
     }
@@ -1145,6 +1160,18 @@ pub enum BlockResponse {
     Rejected(BlockRejection),
 }
 
+impl From<BlockRejection> for BlockResponse {
+    fn from(rejection: BlockRejection) -> Self {
+        BlockResponse::Rejected(rejection)
+    }
+}
+
+impl From<BlockAccepted> for BlockResponse {
+    fn from(accepted: BlockAccepted) -> Self {
+        BlockResponse::Accepted(accepted)
+    }
+}
+
 #[cfg_attr(test, mutants::skip)]
 impl std::fmt::Display for BlockResponse {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -2604,4 +2631,14 @@ mod test {
 
         assert_eq!(signer_message, signer_message_deserialized);
     }
+
+    #[test]
+    fn serde_block_signer_message_pre_commit() {
+        let pre_commit = SignerMessage::BlockPreCommit(Sha512Trunc256Sum([0u8; 32]));
+        let serialized_pre_commit = pre_commit.serialize_to_vec();
+        let deserialized_pre_commit =
+            read_next::<SignerMessage, _>(&mut &serialized_pre_commit[..])
+                .expect("Failed to deserialize pre-commit");
+        assert_eq!(pre_commit, deserialized_pre_commit);
+    }
 }

stacks-node/src/nakamoto_node/stackerdb_listener.rs

@@ -503,6 +503,9 @@ impl StackerDBListener {
                     SignerMessageV0::StateMachineUpdate(update) => {
                         self.update_global_state_evaluator(&signer_pubkey, update);
                     }
+                    SignerMessageV0::BlockPreCommit(_) => {
+                        debug!("Received block pre-commit message. Ignoring.");
+                    }
                 };
             }
         }

stacks-node/src/tests/signer/mod.rs

@@ -17,7 +17,7 @@ mod commands;
 pub mod multiversion;
 pub mod v0;
 
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 use std::fs::File;
 use std::path::PathBuf;
 use std::sync::atomic::{AtomicBool, Ordering};
@@ -72,6 +72,7 @@ use super::neon_integrations::{
     copy_dir_all, get_account, get_sortition_info_ch, submit_tx_fallible, Account,
 };
 use crate::burnchains::bitcoin::core_controller::BitcoinCoreController;
+use crate::nakamoto_node::miner::TEST_MINE_SKIP;
 use crate::neon::Counters;
 use crate::run_loop::boot_nakamoto;
 use crate::tests::nakamoto_integrations::{
@@ -81,6 +82,7 @@ use crate::tests::neon_integrations::{
     get_chain_info, next_block_and_wait, run_until_burnchain_height, test_observer,
     wait_for_runloop,
 };
+use crate::tests::signer::v0::wait_for_state_machine_update_by_miner_tenure_id;
 use crate::tests::to_addr;
 use crate::BitcoinRegtestController;
 
@@ -544,11 +546,16 @@ impl<Z: SpawnedSignerTrait> SignerTest<Z> {
     }
 
     pub fn mine_bitcoin_block(&self) {
+        let mined_btc_block_time = Instant::now();
         let info = self.get_peer_info();
         next_block_and(&self.running_nodes.btc_regtest_controller, 60, || {
             Ok(get_chain_info(&self.running_nodes.conf).burn_block_height > info.burn_block_height)
         })
         .unwrap();
+        info!(
+            "Bitcoin block mine time elapsed: {:?}",
+            mined_btc_block_time.elapsed()
+        );
     }
 
     /// Fetch the local signer state machine for all the signers,
@@ -1121,26 +1128,63 @@ impl<Z: SpawnedSignerTrait> SignerTest<Z> {
         output
     }
 
-    /// Mine a BTC block and wait for a new Stacks block to be mined
+    /// Mine a BTC block and wait for a new Stacks block to be mined, but do not wait for a commit
     /// Note: do not use nakamoto blocks mined heuristic if running a test with multiple miners
-    fn mine_nakamoto_block(&self, timeout: Duration, use_nakamoto_blocks_mined: bool) {
-        let mined_block_time = Instant::now();
-        let mined_before = self.running_nodes.counters.naka_mined_blocks.get();
-        let info_before = self.get_peer_info();
-
-        next_block_and(
-            &self.running_nodes.btc_regtest_controller,
+    fn mine_nakamoto_block_without_commit(
+        &self,
+        timeout: Duration,
+        use_nakamoto_blocks_mined: bool,
+    ) {
+        let info_before = get_chain_info(&self.running_nodes.conf);
+        info!("Pausing stacks block mining");
+        TEST_MINE_SKIP.set(true);
+        let mined_blocks = self.running_nodes.counters.naka_mined_blocks.clone();
+        let mined_before = mined_blocks.get();
+        self.mine_bitcoin_block();
+        wait_for_state_machine_update_by_miner_tenure_id(
             timeout.as_secs(),
-            || {
-                let info_after = self.get_peer_info();
-                let blocks_mined = self.running_nodes.counters.naka_mined_blocks.get();
-                Ok(info_after.stacks_tip_height > info_before.stacks_tip_height
-                    && (!use_nakamoto_blocks_mined || blocks_mined > mined_before))
-            },
+            &get_chain_info(&self.running_nodes.conf).pox_consensus,
+            &self.signer_addresses_versions_majority(),
         )
-        .unwrap();
-        let mined_block_elapsed_time = mined_block_time.elapsed();
-        info!("Nakamoto block mine time elapsed: {mined_block_elapsed_time:?}");
+        .expect("Failed to update signer state machine");
+
+        info!("Unpausing stacks block mining");
+        let mined_block_time = Instant::now();
+        TEST_MINE_SKIP.set(false);
+        // Do these wait for's in two steps not only for increased timeout but for easier debugging.
+        // Ensure that the tenure change transaction is mined
+        wait_for(timeout.as_secs(), || {
+            Ok(get_chain_info(&self.running_nodes.conf).stacks_tip_height
+                > info_before.stacks_tip_height
+                && (!use_nakamoto_blocks_mined || mined_blocks.get() > mined_before))
+        })
+        .expect("Failed to mine Tenure Change block");
+        info!(
+            "Nakamoto block mine time elapsed: {:?}",
+            mined_block_time.elapsed()
+        );
+    }
+
+    /// Mine a BTC block and wait for a new Stacks block to be mined and commit to be submitted
+    /// Note: do not use nakamoto blocks mined heuristic if running a test with multiple miners
+    fn mine_nakamoto_block(&self, timeout: Duration, use_nakamoto_blocks_mined: bool) {
+        let Counters {
+            naka_submitted_commits: commits_submitted,
+            naka_submitted_commit_last_burn_height: commits_last_burn_height,
+            naka_submitted_commit_last_stacks_tip: commits_last_stacks_tip,
+            ..
+        } = self.running_nodes.counters.clone();
+        let commits_before = commits_submitted.get();
+        let commit_burn_height_before = commits_last_burn_height.get();
+        self.mine_nakamoto_block_without_commit(timeout, use_nakamoto_blocks_mined);
+        // Ensure the subsequent block commit confirms the previous Tenure Change block
+        let stacks_tip_height = get_chain_info(&self.running_nodes.conf).stacks_tip_height;
+        wait_for(timeout.as_secs(), || {
+            Ok(commits_submitted.get() > commits_before
+                && commits_last_burn_height.get() > commit_burn_height_before
+                && commits_last_stacks_tip.get() >= stacks_tip_height)
+        })
+        .expect("Failed to update Block Commit");
     }
 
     fn mine_block_wait_on_processing(
@@ -1366,7 +1410,7 @@ impl<Z: SpawnedSignerTrait> SignerTest<Z> {
             .collect()
     }
 
-    /// Get the signer addresses and corresponding versions
+    /// Get the signer addresses and corresponding versions configured versions
     pub fn signer_addresses_versions(&self) -> Vec<(StacksAddress, u64)> {
         self.signer_stacks_private_keys
             .iter()
@@ -1380,6 +1424,33 @@ impl<Z: SpawnedSignerTrait> SignerTest<Z> {
             .collect()
     }
 
+    /// Get the signer addresses and corresponding majority versions
+    pub fn signer_addresses_versions_majority(&self) -> Vec<(StacksAddress, u64)> {
+        let mut signer_address_versions = self.signer_addresses_versions();
+        let majority = (signer_address_versions.len() * 7 / 10) as u64;
+        let mut protocol_versions = HashMap::new();
+        for (_, version) in &self.signer_addresses_versions() {
+            let entry = protocol_versions.entry(*version).or_insert_with(|| 0);
+            *entry += 1;
+        }
+
+        // find the highest version number supported by a threshold number of signers
+        let mut protocol_versions: Vec<_> = protocol_versions.into_iter().collect();
+        protocol_versions.sort_by_key(|(version, _)| *version);
+        let mut total_weight_support = 0;
+        for (version, weight_support) in protocol_versions.into_iter().rev() {
+            total_weight_support += weight_support;
+            if total_weight_support > majority {
+                // We need to actually overwrite the versions passed in since the signers will go with the majority value if they can
+                signer_address_versions
+                    .iter_mut()
+                    .for_each(|(_, v)| *v = version);
+                break;
+            }
+        }
+        signer_address_versions
+    }
+
     /// Get the signer public keys for the given reward cycle
     fn get_signer_public_keys(&self, reward_cycle: u64) -> Vec<StacksPublicKey> {
         let entries = self.get_reward_set_signers(reward_cycle);