Merge remote-tracking branch 'origin/develop' into feat/fork-detection-state-machine

Author: hstove

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ### Added
 
+- Added field `vm_error` to EventObserver transaction outputs
 - Added new `ValidateRejectCode` values to the `/v3/block_proposal` endpoint
 - Added `StateMachineUpdateContent::V1` to support a vector of `StacksTransaction` expected to be replayed in subsequent Stacks blocks
 
@@ -18,6 +19,10 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 - Added index for `next_ready_nakamoto_block()` which improves block processing performance.
 - Added a new field, `parent_burn_block_hash`, to the payload that is included in the `/new_burn_block` event observer payload.
 
+### Fixed
+
+- Fix regression in mock-mining, allowing the mock miner to continue mining blocks throughout a tenure instead of failing after mining the tenure change block.
+
 ## [3.1.0.0.8]
 
 ### Added

CONTRIBUTING.md

@@ -361,17 +361,31 @@ A non-exhaustive list of examples of consensus-critical changes include:
 
 - Every consensus-critical change needs an integration test to verify that the feature activates only when the hard fork activates.
 
-PRs must include test coverage. However, if your PR includes large tests or tests which cannot run in parallel
+- PRs must include test coverage. However, if your PR includes large tests or tests which cannot run in parallel
 (which is the default operation of the `cargo test` command), these tests should be decorated with `#[ignore]`.
-
 A test should be marked `#[ignore]` if:
 
-1. It does not _always_ pass `cargo test` in a vanilla environment
+  1. It does not _always_ pass `cargo test` in a vanilla environment
    (i.e., it does not need to run with `--test-threads 1`).
 
-2. Or, it runs for over a minute via a normal `cargo test` execution
+  2. Or, it runs for over a minute via a normal `cargo test` execution
    (the `cargo test` command will warn if this is not the case).
 
+- **Integration tests need to be properly tagged** using [pinny-rs](https://github.com/BitcoinL2-Labs/pinny-rs/) crate. Tagging requires two fundamental steps:
+  1. Define allowed tags in the package `Cargo.toml` file (if needed).
+  2. Apply relevant tags to the tests, picking from the allowed set.
+  
+  Then it will be possible to run tests with filtering based on the tags using `cargo test` and `cargo nextest` runner.
+  > For more information and examples on how tagging works, refer to the [pinny-rs](https://github.com/BitcoinL2-Labs/pinny-rs/) readme.
+  
+  Below the tag set currently defined with related purpose:
+
+  | Tag             | Description                                  |
+  |-----------------|----------------------------------------------|
+  | `slow`          | tests running over a minute                  |
+  | `bitcoind`      | tests requiring bitcoin daemon               |
+  | `flaky`         | tests that exhibit flaky behavior            |
+
 ## Formatting
 
 PRs will be checked against `rustfmt` and will _fail_ if not properly formatted.

Cargo.lock

@@ -17,6 +17,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 - The signer will now check if their associated stacks-node has processed the parent block for a block proposal before submitting that block proposal. If it cannot confirm that the parent block has been processed, it waits a default time of 15s before submitting, configurable via `proposal_wait_for_parent_time_secs` in the signer config.toml.
 
+
 ## [3.1.0.0.8.0]
 
 ### Changed

stacks-signer/CHANGELOG.md

@@ -43,6 +43,8 @@ use stacks_common::util::get_epoch_time_secs;
 use stacks_common::util::secp256k1::MessageSignature;
 use stacks_common::{debug, error, info, warn};
 
+#[cfg(not(any(test, feature = "testing")))]
+use super::signer_state::SUPPORTED_SIGNER_PROTOCOL_VERSION;
 use super::signer_state::{GlobalStateEvaluator, LocalStateMachine};
 use crate::chainstate::{ProposalEvalConfig, SortitionMinerStatus, SortitionsView};
 use crate::client::{ClientError, SignerSlotID, StackerDB, StacksClient};
@@ -301,8 +303,9 @@ impl SignerTrait<SignerMessage> for Signer {
         self.check_pending_block_validations(stacks_client);
 
         if prior_state != self.local_state_machine {
+            let version = self.get_signer_protocol_version();
             self.local_state_machine
-                .send_signer_update_message(&mut self.stackerdb);
+                .send_signer_update_message(&mut self.stackerdb, version);
         }
     }
 
@@ -731,10 +734,12 @@ impl Signer {
             .insert_update(address, update.clone());
 
         // See if this update means we should capitulate our viewpoint...
+        let version = self.get_signer_protocol_version();
         self.local_state_machine.capitulate_viewpoint(
             &mut self.signer_db,
             &mut self.global_state_evaluator,
             self.stacks_address,
+            version,
         );
     }
 
@@ -1683,6 +1688,16 @@ impl Signer {
             None
         }
     }
+
+    #[cfg(not(any(test, feature = "testing")))]
+    fn get_signer_protocol_version(&self) -> u64 {
+        SUPPORTED_SIGNER_PROTOCOL_VERSION
+    }
+
+    #[cfg(any(test, feature = "testing"))]
+    fn get_signer_protocol_version(&self) -> u64 {
+        self.test_get_signer_protocol_version()
+    }
 }
 
 /// Determine if a block should be re-evaluated based on its rejection reason˝

stacks-signer/src/v0/signer.rs

@@ -30,7 +30,7 @@ use stacks_common::codec::Error as CodecError;
 use stacks_common::types::chainstate::{ConsensusHash, StacksBlockId, TrieHash};
 use stacks_common::util::hash::{Hash160, Sha512Trunc256Sum};
 use stacks_common::util::secp256k1::MessageSignature;
-use stacks_common::{info, warn};
+use stacks_common::{debug, info, warn};
 
 use crate::chainstate::{
     ProposalEvalConfig, SignerChainstateError, SortitionState, SortitionsView,
@@ -357,10 +357,25 @@ pub struct NewBurnBlock {
     pub consensus_hash: ConsensusHash,
 }
 
-impl TryInto<StateMachineUpdateMessage> for &LocalStateMachine {
-    type Error = CodecError;
+impl LocalStateMachine {
+    /// Initialize a local state machine by querying the local stacks-node
+    ///  and signerdb for the current sortition information
+    pub fn new(
+        db: &SignerDb,
+        client: &StacksClient,
+        proposal_config: &ProposalEvalConfig,
+    ) -> Result<Self, SignerChainstateError> {
+        let mut instance = Self::Uninitialized;
+        instance.bitcoin_block_arrival(db, client, proposal_config, None)?;
+
+        Ok(instance)
+    }
 
-    fn try_into(self) -> Result<StateMachineUpdateMessage, Self::Error> {
+    /// Convert the local state machine into update message with the specificed supported protocol version
+    pub fn try_into_update_message_with_version(
+        &self,
+        local_supported_signer_protocol_version: u64,
+    ) -> Result<StateMachineUpdateMessage, CodecError> {
         let LocalStateMachine::Initialized(state_machine) = self else {
             return Err(CodecError::SerializeError(
                 "Local state machine is not ready to be serialized into an update message".into(),
@@ -404,25 +419,10 @@ impl TryInto<StateMachineUpdateMessage> for &LocalStateMachine {
         };
         StateMachineUpdateMessage::new(
             state_machine.active_signer_protocol_version,
-            SUPPORTED_SIGNER_PROTOCOL_VERSION,
+            local_supported_signer_protocol_version,
             content,
         )
     }
-}
-
-impl LocalStateMachine {
-    /// Initialize a local state machine by querying the local stacks-node
-    ///  and signerdb for the current sortition information
-    pub fn new(
-        db: &SignerDb,
-        client: &StacksClient,
-        proposal_config: &ProposalEvalConfig,
-    ) -> Result<Self, SignerChainstateError> {
-        let mut instance = Self::Uninitialized;
-        instance.bitcoin_block_arrival(db, client, proposal_config, None)?;
-
-        Ok(instance)
-    }
 
     fn place_holder() -> SignerStateMachine {
         SignerStateMachine {
@@ -435,10 +435,16 @@ impl LocalStateMachine {
     }
 
     /// 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();
+    pub fn send_signer_update_message(
+        &self,
+        stackerdb: &mut StackerDB<MessageSlotID>,
+        version: u64,
+    ) {
+        let update: Result<StateMachineUpdateMessage, _> =
+            self.try_into_update_message_with_version(version);
         match update {
             Ok(update) => {
+                debug!("Sending signer update message to stackerdb: {update:?}");
                 if let Err(e) = stackerdb.send_message_with_retry::<SignerMessage>(update.into()) {
                     warn!("Failed to send signer update to stacker-db: {e:?}",);
                 }
@@ -848,10 +854,12 @@ impl LocalStateMachine {
         signerdb: &mut SignerDb,
         eval: &mut GlobalStateEvaluator,
         local_address: StacksAddress,
+        local_supported_signer_protocol_version: u64,
     ) {
         // Before we ever access eval...we should make sure to include our own local state machine update message in the evaluation
-        let local_update: Result<StateMachineUpdateMessage, _> = (&*self).try_into();
-        let Ok(mut local_update) = local_update else {
+        let Ok(mut local_update) =
+            self.try_into_update_message_with_version(local_supported_signer_protocol_version)
+        else {
             return;
         };
 
@@ -892,8 +900,9 @@ impl LocalStateMachine {
                 tx_replay_set: tx_replay_set.cloned(),
             });
             // Because we updated our active signer protocol version, update local_update so its included in the subsequent evaluations
-            let update: Result<StateMachineUpdateMessage, _> = (&*self).try_into();
-            let Ok(update) = update else {
+            let Ok(update) =
+                self.try_into_update_message_with_version(local_supported_signer_protocol_version)
+            else {
                 return;
             };
             local_update = update;

stacks-signer/src/v0/signer_state.rs

@@ -13,6 +13,7 @@
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+use std::collections::HashMap;
 use std::sync::LazyLock;
 
 use blockstack_lib::chainstate::nakamoto::NakamotoBlock;
@@ -24,8 +25,14 @@ use stacks_common::util::tests::TestFlag;
 use stacks_common::{info, warn};
 
 use super::signer::Signer;
+use super::signer_state::SUPPORTED_SIGNER_PROTOCOL_VERSION;
 use crate::signerdb::BlockInfo;
 
+/// A global variable that can be used to pin a signer's highest supported protocol version if the signer's public key is in the provided list
+pub static TEST_PIN_SUPPORTED_SIGNER_PROTOCOL_VERSION: LazyLock<
+    TestFlag<HashMap<StacksPublicKey, u64>>,
+> = LazyLock::new(TestFlag::default);
+
 /// A global variable that can be used to reject all block proposals if the signer's public key is in the provided list
 pub static TEST_REJECT_ALL_BLOCK_PROPOSAL: LazyLock<TestFlag<Vec<StacksPublicKey>>> =
     LazyLock::new(TestFlag::default);
@@ -151,4 +158,16 @@ impl Signer {
             warn!("{self}: Block validation submission is no longer stalled due to testing directive. Continuing...");
         }
     }
+
+    /// Get the pinned signer version for the signer
+    pub fn test_get_signer_protocol_version(&self) -> u64 {
+        let public_keys = TEST_PIN_SUPPORTED_SIGNER_PROTOCOL_VERSION.get();
+        if let Some(version) = public_keys.get(
+            &stacks_common::types::chainstate::StacksPublicKey::from_private(&self.private_key),
+        ) {
+            warn!("{self}: signer version is pinned to {version}");
+            return *version;
+        }
+        SUPPORTED_SIGNER_PROTOCOL_VERSION
+    }
 }