CRC: just send the update messages after processing an event

Signed-off-by: Jacinta Ferrant <[email protected]>

Author: jferrant

stacks-signer/src/v0/signer.rs

@@ -129,7 +129,7 @@ impl std::fmt::Display for Signer {
 impl SignerTrait<SignerMessage> for Signer {
     /// Create a new signer from the given configuration
     fn new(stacks_client: &StacksClient, signer_config: SignerConfig) -> Self {
-        let mut stackerdb = StackerDB::from(&signer_config);
+        let stackerdb = StackerDB::from(&signer_config);
         let mode = match signer_config.signer_mode {
             SignerConfigMode::DryRun => SignerMode::DryRun,
             SignerConfigMode::Normal { signer_id, .. } => SignerMode::Normal { signer_id },
@@ -141,12 +141,11 @@ impl SignerTrait<SignerMessage> for Signer {
             SignerDb::new(&signer_config.db_path).expect("Failed to connect to signer Db");
         let proposal_config = ProposalEvalConfig::from(&signer_config);
 
-        let signer_state =
-            LocalStateMachine::new(&signer_db, &mut stackerdb, stacks_client, &proposal_config)
-                .unwrap_or_else(|e| {
-                    warn!("Failed to initialize local state machine for signer: {e:?}");
-                    LocalStateMachine::Uninitialized
-                });
+        let signer_state = LocalStateMachine::new(&signer_db, stacks_client, &proposal_config)
+            .unwrap_or_else(|e| {
+                warn!("Failed to initialize local state machine for signer: {e:?}");
+                LocalStateMachine::Uninitialized
+            });
         Self {
             private_key: signer_config.stacks_private_key,
             stackerdb,
@@ -213,8 +212,9 @@ impl SignerTrait<SignerMessage> for Signer {
             return;
         }
 
+        let prior_state = self.local_state_machine.clone();
         if self.reward_cycle <= current_reward_cycle {
-            self.local_state_machine.handle_pending_update(&self.signer_db, &mut self.stackerdb, stacks_client, &self.proposal_config)
+            self.local_state_machine.handle_pending_update(&self.signer_db, stacks_client, &self.proposal_config)
                 .unwrap_or_else(|e| error!("{self}: failed to update local state machine for pending update"; "err" => ?e));
         }
 
@@ -338,7 +338,7 @@ impl SignerTrait<SignerMessage> for Signer {
                         panic!("{self} Failed to write burn block event to signerdb: {e}");
                     });
                 self.local_state_machine
-                    .bitcoin_block_arrival(&self.signer_db, &mut self.stackerdb, stacks_client, &self.proposal_config, Some(*burn_height))
+                    .bitcoin_block_arrival(&self.signer_db, stacks_client, &self.proposal_config, Some(*burn_height))
                     .unwrap_or_else(|e| error!("{self}: failed to update local state machine for latest bitcoin block arrival"; "err" => ?e));
                 *sortition_state = None;
             }
@@ -360,7 +360,7 @@ impl SignerTrait<SignerMessage> for Signer {
                     "block_height" => block_height
                 );
                 self.local_state_machine
-                    .stacks_block_arrival(&mut self.stackerdb, consensus_hash, *block_height, block_id)
+                    .stacks_block_arrival(consensus_hash, *block_height, block_id)
                     .unwrap_or_else(|e| error!("{self}: failed to update local state machine for latest stacks block arrival"; "err" => ?e));
 
                 if let Ok(Some(mut block_info)) = self
@@ -382,6 +382,10 @@ impl SignerTrait<SignerMessage> for Signer {
                 }
             }
         }
+        if prior_state != self.local_state_machine {
+            self.local_state_machine
+                .send_signer_update_message(&mut self.stackerdb);
+        }
     }
 
     fn has_unprocessed_blocks(&self) -> bool {

stacks-signer/src/v0/signer_state.rs

@@ -144,12 +144,11 @@ impl LocalStateMachine {
     ///  and signerdb for the current sortition information
     pub fn new(
         db: &SignerDb,
-        stackerdb: &mut StackerDB<MessageSlotID>,
         client: &StacksClient,
         proposal_config: &ProposalEvalConfig,
     ) -> Result<Self, SignerChainstateError> {
         let mut instance = Self::Uninitialized;
-        instance.bitcoin_block_arrival(db, stackerdb, client, proposal_config, None)?;
+        instance.bitcoin_block_arrival(db, client, proposal_config, None)?;
 
         Ok(instance)
     }
@@ -163,7 +162,7 @@ impl LocalStateMachine {
         }
     }
 
-    /// Send the local tate machine as an signer update message to stackerdb
+    /// Send the local state machine as a signer update message to stackerdb
     pub fn send_signer_update_message(&self, stackerdb: &mut StackerDB<MessageSlotID>) {
         let update: Result<StateMachineUpdateMessage, _> = self.try_into();
         match update {
@@ -182,21 +181,16 @@ impl LocalStateMachine {
     pub fn handle_pending_update(
         &mut self,
         db: &SignerDb,
-        stackerdb: &mut StackerDB<MessageSlotID>,
         client: &StacksClient,
         proposal_config: &ProposalEvalConfig,
     ) -> Result<(), SignerChainstateError> {
         let LocalStateMachine::Pending { update, .. } = self else {
-            return self.check_miner_inactivity(db, stackerdb, client, proposal_config);
+            return self.check_miner_inactivity(db, client, proposal_config);
         };
         match update.clone() {
-            StateMachineUpdate::BurnBlock(expected_burn_height) => self.bitcoin_block_arrival(
-                db,
-                stackerdb,
-                client,
-                proposal_config,
-                Some(expected_burn_height),
-            ),
+            StateMachineUpdate::BurnBlock(expected_burn_height) => {
+                self.bitcoin_block_arrival(db, client, proposal_config, Some(expected_burn_height))
+            }
         }
     }
 
@@ -237,7 +231,6 @@ impl LocalStateMachine {
     fn check_miner_inactivity(
         &mut self,
         db: &SignerDb,
-        stackerdb: &mut StackerDB<MessageSlotID>,
         client: &StacksClient,
         proposal_config: &ProposalEvalConfig,
     ) -> Result<(), SignerChainstateError> {
@@ -278,8 +271,6 @@ impl LocalStateMachine {
                 "inactive_tenure_ch" => %inactive_tenure_ch,
                 "new_active_tenure_ch" => %new_active_tenure_ch
             );
-            // We have updated our state, so let other signers know.
-            self.send_signer_update_message(stackerdb);
             Ok(())
         } else {
             warn!("Current miner timed out due to inactivity, but prior miner is not valid. Allowing current miner to continue");
@@ -347,7 +338,6 @@ impl LocalStateMachine {
     /// Handle a new stacks block arrival
     pub fn stacks_block_arrival(
         &mut self,
-        stackerdb: &mut StackerDB<MessageSlotID>,
         ch: &ConsensusHash,
         height: u64,
         block_id: &StacksBlockId,
@@ -403,8 +393,6 @@ impl LocalStateMachine {
         *parent_tenure_last_block = *block_id;
         *parent_tenure_last_block_height = height;
         *self = LocalStateMachine::Initialized(prior_state_machine);
-        // We updated the block id and/or the height. Let other signers know our view has changed
-        self.send_signer_update_message(stackerdb);
         Ok(())
     }
 
@@ -452,7 +440,6 @@ impl LocalStateMachine {
     pub fn bitcoin_block_arrival(
         &mut self,
         db: &SignerDb,
-        stackerdb: &mut StackerDB<MessageSlotID>,
         client: &StacksClient,
         proposal_config: &ProposalEvalConfig,
         mut expected_burn_height: Option<u64>,
@@ -538,11 +525,6 @@ impl LocalStateMachine {
             current_miner: miner_state,
             active_signer_protocol_version: prior_state_machine.active_signer_protocol_version,
         });
-
-        if prior_state != *self {
-            self.send_signer_update_message(stackerdb);
-        }
-
         Ok(())
     }
 }

testnet/stacks-node/src/tests/epoch_205.rs

@@ -14,8 +14,7 @@ use stacks::chainstate::stacks::{
 };
 use stacks::config::{EventKeyType, InitialBalance};
 use stacks::core::test_util::{
-    make_contract_call, make_contract_call_mblock_only, make_contract_publish,
-    make_contract_publish_microblock_only, to_addr,
+    make_contract_call, make_contract_call_mblock_only, make_contract_publish, to_addr,
 };
 use stacks::core::{
     self, EpochList, StacksEpoch, StacksEpochId, PEER_VERSION_EPOCH_1_0, PEER_VERSION_EPOCH_2_0,

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

@@ -1220,13 +1220,6 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
             .send_message_with_retry::<SignerMessage>(accepted.into())
             .expect("Failed to send accept signature");
     }
-
-    pub fn signer_public_keys(&self) -> Vec<StacksPublicKey> {
-        self.signer_stacks_private_keys
-            .iter()
-            .map(StacksPublicKey::from_private)
-            .collect()
-    }
 }
 
 fn setup_stx_btc_node<G: FnMut(&mut NeonConfig)>(

testnet/stacks-node/src/tests/signer/v0.rs

@@ -771,7 +771,6 @@ impl MultipleMinerTest {
         contract_src: &str,
     ) -> String {
         let http_origin = self.node_http();
-        let sender_addr = tests::to_addr(&self.sender_sk);
         let contract_tx = make_contract_publish(
             &self.sender_sk,
             sender_nonce,
@@ -6095,7 +6094,7 @@ fn reorg_locally_accepted_blocks_across_tenures_succeeds() {
         None,
         None,
     );
-    let all_signers = signer_test.signer_public_keys();
+    let all_signers = signer_test.signer_test_pks();
     let http_origin = format!("http://{}", &signer_test.running_nodes.conf.node.rpc_bind);
 
     let miner_sk = signer_test.running_nodes.conf.miner.mining_key.unwrap();
@@ -6265,7 +6264,7 @@ fn reorg_locally_accepted_blocks_across_tenures_fails() {
         None,
         None,
     );
-    let all_signers = signer_test.signer_public_keys();
+    let all_signers = signer_test.signer_test_pks();
     let http_origin = format!("http://{}", &signer_test.running_nodes.conf.node.rpc_bind);
 
     let miner_sk = signer_test.running_nodes.conf.miner.mining_key.unwrap();
@@ -6485,7 +6484,7 @@ fn miner_recovers_when_broadcast_block_delay_across_tenures_occurs() {
     info!("Submitted tx {tx} in to attempt to mine block N+1");
     let block_n_1 = wait_for_block_proposal(30, info_before.stacks_tip_height + 1, &miner_pk)
         .expect("Timed out waiting for block N+1 to be proposed");
-    let all_signers = signer_test.signer_public_keys();
+    let all_signers = signer_test.signer_test_pks();
     wait_for_block_global_acceptance_from_signers(
         30,
         &block_n_1.header.signer_signature_hash(),
@@ -6642,7 +6641,7 @@ fn continue_after_fast_block_no_sortition() {
     let burnchain = conf_1.get_burnchain();
     let sortdb = burnchain.open_sortition_db(true).unwrap();
 
-    let all_signers = miners.signer_test.signer_public_keys();
+    let all_signers = miners.signer_test.signer_test_pks();
     let get_burn_height = || {
         SortitionDB::get_canonical_burn_chain_tip(sortdb.conn())
             .unwrap()
@@ -6930,7 +6929,7 @@ fn signing_in_0th_tenure_of_reward_cycle() {
     info!("------------------------- Test Setup -------------------------");
     let num_signers = 5;
     let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new(num_signers, vec![]);
-    let signer_public_keys = signer_test.signer_public_keys();
+    let signer_public_keys = signer_test.signer_test_pks();
     let long_timeout = Duration::from_secs(200);
     signer_test.boot_to_epoch_3();
     let curr_reward_cycle = signer_test.get_current_reward_cycle();
@@ -7198,7 +7197,7 @@ fn block_commit_delay() {
     .expect("Timed out waiting for block commit after new Stacks block");
 
     // Prevent a block from being mined by making signers reject it.
-    let all_signers = signer_test.signer_public_keys();
+    let all_signers = signer_test.signer_test_pks();
     TEST_REJECT_ALL_BLOCK_PROPOSAL.set(all_signers);
 
     info!("------------------------- Test Mine Burn Block  -------------------------");
@@ -7464,7 +7463,7 @@ fn block_validation_check_rejection_timeout_heuristic() {
     );
     let miner_sk = signer_test.running_nodes.conf.miner.mining_key.unwrap();
     let miner_pk = StacksPublicKey::from_private(&miner_sk);
-    let all_signers = signer_test.signer_public_keys();
+    let all_signers = signer_test.signer_test_pks();
 
     signer_test.boot_to_epoch_3();
 
@@ -7682,7 +7681,7 @@ fn block_validation_pending_table() {
     .expect("Timed out waiting for pending block validation to be removed");
 
     // for test cleanup we need to wait for block rejections
-    let signer_keys = signer_test.signer_public_keys();
+    let signer_keys = signer_test.signer_test_pks();
     wait_for_block_rejections_from_signers(30, &block.header.signer_signature_hash(), &signer_keys)
         .expect("Timed out waiting for block rejections");
 
@@ -8411,7 +8410,7 @@ fn global_acceptance_depends_on_block_announcement() {
         None,
     );
 
-    let all_signers = signer_test.signer_public_keys();
+    let all_signers = signer_test.signer_test_pks();
     let miner_sk = signer_test.running_nodes.conf.miner.mining_key.unwrap();
     let miner_pk = StacksPublicKey::from_private(&miner_sk);
     let http_origin = format!("http://{}", &signer_test.running_nodes.conf.node.rpc_bind);
@@ -8914,7 +8913,7 @@ fn incoming_signers_ignore_block_proposals() {
     info!("------------------------- Test Attempt to Mine Invalid Block {signer_signature_hash_1} -------------------------");
 
     let short_timeout = Duration::from_secs(30);
-    let all_signers = signer_test.signer_public_keys();
+    let all_signers = signer_test.signer_test_pks();
     test_observer::clear();
 
     // Propose a block to the signers that passes initial checks but will be rejected by the stacks node
@@ -10711,7 +10710,7 @@ fn interrupt_miner_on_new_stacks_tip() {
     let (miner_pk_1, miner_pk_2) = miners.get_miner_public_keys();
     let (miner_pkh_1, miner_pkh_2) = miners.get_miner_public_key_hashes();
 
-    let all_signers = miners.signer_test.signer_public_keys();
+    let all_signers = miners.signer_test.signer_test_pks();
 
     // Pause Miner 2's commits to ensure Miner 1 wins the first sortition.
     skip_commit_op_rl2.set(true);
@@ -11936,7 +11935,7 @@ fn mark_miner_as_invalid_if_reorg_is_rejected() {
             config.miner.block_commit_delay = Duration::from_secs(0);
         },
     );
-    let all_signers = miners.signer_test.signer_public_keys();
+    let all_signers = miners.signer_test.signer_test_pks();
     let mut approving_signers = vec![];
     let mut rejecting_signers = vec![];
     for (i, signer_config) in miners.signer_test.signer_configs.iter().enumerate() {