Merge branch 'develop' of https://github.com/stacks-network/stacks-core into feat/miner-constructs-replay-block-feature-gated

Author: jferrant

CHANGELOG.md

@@ -9,17 +9,23 @@ 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
+- Include a reason string in the transaction receipt when a transaction is rolled back due to a post-condition. This should help users in understanding what went wrong.
 - Updated `StackerDBListener` to monitor signer state machine updates and store replay information separately, enabling miners to perform transaction replays.
-- Added a `replay_transactions` flag to the miner configuration to feature-gate transaction replay. When enabled, the miner will construct a replay block if a threshold of signers signals that a transaction set requires replay.
+- Added a testnet `replay_transactions` flag to the miner configuration to feature-gate transaction replay. When enabled, the miner will construct a replay block if a threshold of signers signals that a transaction set requires replay.
 
 ### Changed
 
 - Reduce the default `block_rejection_timeout_steps` configuration so that miners will retry faster when blocks fail to reach 70% approved or 30% rejected.
 - 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

clarity/src/vm/clarity.rs

@@ -20,18 +20,30 @@ pub enum Error {
     Interpreter(InterpreterError),
     BadTransaction(String),
     CostError(ExecutionCost, ExecutionCost),
-    AbortedByCallback(Option<Value>, AssetMap, Vec<StacksTransactionEvent>),
+    AbortedByCallback {
+        /// What the output value of the transaction would have been.
+        /// This will be a Some for contract-calls, and None for contract initialization txs.
+        output: Option<Value>,
+        /// The asset map which was evaluated by the abort callback
+        assets_modified: AssetMap,
+        /// The events from the transaction processing
+        tx_events: Vec<StacksTransactionEvent>,
+        /// A human-readable explanation for aborting the transaction
+        reason: String,
+    },
 }
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match *self {
+        match self {
             Error::CostError(ref a, ref b) => {
-                write!(f, "Cost Error: {} cost exceeded budget of {} cost", a, b)
+                write!(f, "Cost Error: {a} cost exceeded budget of {b} cost")
             }
             Error::Analysis(ref e) => fmt::Display::fmt(e, f),
             Error::Parse(ref e) => fmt::Display::fmt(e, f),
-            Error::AbortedByCallback(..) => write!(f, "Post condition aborted transaction"),
+            Error::AbortedByCallback { reason, .. } => {
+                write!(f, "Post condition aborted transaction: {reason}")
+            }
             Error::Interpreter(ref e) => fmt::Display::fmt(e, f),
             Error::BadTransaction(ref s) => fmt::Display::fmt(s, f),
         }
@@ -42,7 +54,7 @@ impl std::error::Error for Error {
     fn cause(&self) -> Option<&dyn std::error::Error> {
         match *self {
             Error::CostError(ref _a, ref _b) => None,
-            Error::AbortedByCallback(..) => None,
+            Error::AbortedByCallback { .. } => None,
             Error::Analysis(ref e) => Some(e),
             Error::Parse(ref e) => Some(e),
             Error::Interpreter(ref e) => Some(e),
@@ -168,16 +180,17 @@ pub trait TransactionConnection: ClarityConnection {
     /// * the asset changes during `to_do` in an `AssetMap`
     /// * the Stacks events during the transaction
     ///
-    /// and a `bool` value which is `true` if the `abort_call_back` caused the changes to abort.
+    /// and an optional string value which is the result of `abort_call_back`,
+    /// containing a human-readable reason for aborting the transaction.
     ///
     /// If `to_do` returns an `Err` variant, then the changes are aborted.
     fn with_abort_callback<F, A, R, E>(
         &mut self,
         to_do: F,
         abort_call_back: A,
-    ) -> Result<(R, AssetMap, Vec<StacksTransactionEvent>, bool), E>
+    ) -> Result<(R, AssetMap, Vec<StacksTransactionEvent>, Option<String>), E>
     where
-        A: FnOnce(&AssetMap, &mut ClarityDatabase) -> bool,
+        A: FnOnce(&AssetMap, &mut ClarityDatabase) -> Option<String>,
         F: FnOnce(&mut OwnedEnvironment) -> Result<(R, AssetMap, Vec<StacksTransactionEvent>), E>,
         E: From<InterpreterError>;
 
@@ -283,16 +296,16 @@ pub trait TransactionConnection: ClarityConnection {
                     .stx_transfer(from, to, amount, memo)
                     .map_err(Error::from)
             },
-            |_, _| false,
+            |_, _| None,
         )
         .map(|(value, assets, events, _)| (value, assets, events))
     }
 
     /// Execute a contract call in the current block.
-    ///  If an error occurs while processing the transaction, its modifications will be rolled back.
-    /// abort_call_back is called with an AssetMap and a ClarityDatabase reference,
-    ///   if abort_call_back returns true, all modifications from this transaction will be rolled back.
-    ///      otherwise, they will be committed (though they may later be rolled back if the block itself is rolled back).
+    /// If an error occurs while processing the transaction, its modifications will be rolled back.
+    /// `abort_call_back` is called with an `AssetMap` and a `ClarityDatabase` reference,
+    /// If `abort_call_back` returns `Some(reason)`, all modifications from this transaction will be rolled back.
+    /// Otherwise, they will be committed (though they may later be rolled back if the block itself is rolled back).
     #[allow(clippy::too_many_arguments)]
     fn run_contract_call<F>(
         &mut self,
@@ -305,7 +318,7 @@ pub trait TransactionConnection: ClarityConnection {
         max_execution_time: Option<std::time::Duration>,
     ) -> Result<(Value, AssetMap, Vec<StacksTransactionEvent>), Error>
     where
-        F: FnOnce(&AssetMap, &mut ClarityDatabase) -> bool,
+        F: FnOnce(&AssetMap, &mut ClarityDatabase) -> Option<String>,
     {
         let expr_args: Vec<_> = args
             .iter()
@@ -331,20 +344,25 @@ pub trait TransactionConnection: ClarityConnection {
             },
             abort_call_back,
         )
-        .and_then(|(value, assets, events, aborted)| {
-            if aborted {
-                Err(Error::AbortedByCallback(Some(value), assets, events))
+        .and_then(|(value, assets_modified, tx_events, reason)| {
+            if let Some(reason) = reason {
+                Err(Error::AbortedByCallback {
+                    output: Some(value),
+                    assets_modified,
+                    tx_events,
+                    reason,
+                })
             } else {
-                Ok((value, assets, events))
+                Ok((value, assets_modified, tx_events))
             }
         })
     }
 
     /// Initialize a contract in the current block.
     ///  If an error occurs while processing the initialization, it's modifications will be rolled back.
-    /// abort_call_back is called with an AssetMap and a ClarityDatabase reference,
-    ///   if abort_call_back returns true, all modifications from this transaction will be rolled back.
-    ///      otherwise, they will be committed (though they may later be rolled back if the block itself is rolled back).
+    /// `abort_call_back` is called with an `AssetMap` and a `ClarityDatabase` reference,
+    /// If `abort_call_back` returns `Some(reason)`, all modifications from this transaction will be rolled back.
+    /// Otherwise, they will be committed (though they may later be rolled back if the block itself is rolled back).
     #[allow(clippy::too_many_arguments)]
     fn initialize_smart_contract<F>(
         &mut self,
@@ -357,9 +375,9 @@ pub trait TransactionConnection: ClarityConnection {
         max_execution_time: Option<std::time::Duration>,
     ) -> Result<(AssetMap, Vec<StacksTransactionEvent>), Error>
     where
-        F: FnOnce(&AssetMap, &mut ClarityDatabase) -> bool,
+        F: FnOnce(&AssetMap, &mut ClarityDatabase) -> Option<String>,
     {
-        let (_, asset_map, events, aborted) = self.with_abort_callback(
+        let (_, assets_modified, tx_events, reason) = self.with_abort_callback(
             |vm_env| {
                 if let Some(max_execution_time_duration) = max_execution_time {
                     vm_env
@@ -378,10 +396,15 @@ pub trait TransactionConnection: ClarityConnection {
             },
             abort_call_back,
         )?;
-        if aborted {
-            Err(Error::AbortedByCallback(None, asset_map, events))
+        if let Some(reason) = reason {
+            Err(Error::AbortedByCallback {
+                output: None,
+                assets_modified,
+                tx_events,
+                reason,
+            })
         } else {
-            Ok((asset_map, events))
+            Ok((assets_modified, tx_events))
         }
     }
 }

stacks-signer/src/monitoring/mod.rs

@@ -33,6 +33,21 @@ SignerAgreementStateChangeReason {
     InactiveMiner("inactive_miner"),
     /// Signer agreement protocol version has been upgraded
     ProtocolUpgrade("protocol_upgrade"),
+    /// An update related to the Miner view
+    MinerViewUpdate("miner_view_update"),
+    /// A specific Miner View update related to the parent tenure
+    MinerParentTenureUpdate("miner_parent_tenure_update"),
+});
+
+define_named_enum!(
+/// Represent different conflict types on signer agreement protocol
+SignerAgreementStateConflict {
+    /// Waiting for burn block propagation to be aligned with the signer set
+    BurnBlockDelay("burn_block_delay"),
+    /// Waiting for stacks block propagation to be aligned with the signer set
+    StacksBlockDelay("stacks_block_delay"),
+    /// No agreement on miner view with the signer set
+    MinerView("miner_view"),
 });
 
 /// Actions for updating metrics
@@ -44,7 +59,7 @@ pub mod actions {
 
     use crate::config::GlobalConfig;
     use crate::monitoring::prometheus::*;
-    use crate::monitoring::SignerAgreementStateChangeReason;
+    use crate::monitoring::{SignerAgreementStateChangeReason, SignerAgreementStateConflict};
     use crate::v0::signer_state::LocalStateMachine;
 
     /// Update stacks tip height gauge
@@ -134,6 +149,21 @@ pub mod actions {
             .inc();
     }
 
+    /// Increment signer agreement state conflict counter
+    pub fn increment_signer_agreement_state_conflict(conflict: SignerAgreementStateConflict) {
+        let label_value = conflict.get_name();
+        SIGNER_AGREEMENT_STATE_CONFLICTS
+            .with_label_values(&[&label_value])
+            .inc();
+    }
+
+    /// Record the time (seconds) taken for a signer to agree with the signer set
+    pub fn record_signer_agreement_capitulation_latency(latency_s: u64) {
+        SIGNER_AGREEMENT_CAPITULATION_LATENCIES_HISTOGRAM
+            .with_label_values(&[])
+            .observe(latency_s as f64);
+    }
+
     /// Start serving monitoring metrics.
     /// This will only serve the metrics if the `monitoring_prom` feature is enabled.
     pub fn start_serving_monitoring_metrics(config: GlobalConfig) -> Result<(), String> {
@@ -157,7 +187,7 @@ pub mod actions {
     use blockstack_lib::chainstate::nakamoto::NakamotoBlock;
     use stacks_common::info;
 
-    use crate::monitoring::SignerAgreementStateChangeReason;
+    use crate::monitoring::{SignerAgreementStateChangeReason, SignerAgreementStateConflict};
     use crate::v0::signer_state::LocalStateMachine;
     use crate::GlobalConfig;
 
@@ -212,6 +242,12 @@ pub mod actions {
     ) {
     }
 
+    /// Increment signer agreement state conflict counter
+    pub fn increment_signer_agreement_state_conflict(_conflict: SignerAgreementStateConflict) {}
+
+    /// Record the time (seconds) taken for a signer to agree with the signer set
+    pub fn record_signer_agreement_capitulation_latency(_latency_s: u64) {}
+
     /// Start serving monitoring metrics.
     /// This will only serve the metrics if the `monitoring_prom` feature is enabled.
     pub fn start_serving_monitoring_metrics(config: GlobalConfig) -> Result<(), String> {

stacks-signer/src/monitoring/prometheus.rs

@@ -81,10 +81,22 @@ lazy_static! {
 
     pub static ref SIGNER_AGREEMENT_STATE_CHANGE_REASONS: IntCounterVec = register_int_counter_vec!(
         "stacks_signer_agreement_state_change_reasons",
-        "The number of state machine changes in signer agreement protocol. `reason` can be one of: 'burn_block_arrival', 'stacks_block_arrival', 'inactive_miner', 'protocol_upgrade'",
+        "The number of state machine changes in signer agreement protocol. `reason` can be one of: 'burn_block_arrival', 'stacks_block_arrival', 'inactive_miner', 'protocol_upgrade', 'miner_view_update', 'miner_parent_tenure_update'",
         &["reason"]
     ).unwrap();
 
+    pub static ref SIGNER_AGREEMENT_STATE_CONFLICTS: IntCounterVec = register_int_counter_vec!(
+        "stacks_signer_agreement_state_conflicts",
+        "The number of state machine conflicts in signer agreement protocol. `conflict` can be one of: 'burn_block_delay', 'stacks_block_delay', 'miner_view'",
+        &["conflict"]
+    ).unwrap();
+
+    pub static ref SIGNER_AGREEMENT_CAPITULATION_LATENCIES_HISTOGRAM: HistogramVec = register_histogram_vec!(histogram_opts!(
+        "stacks_signer_agreement_capitulation_latencies_histogram",
+        "Measuring the time (in seconds) for the signer to agree (capitulate) with the signer set",
+        vec![0.0, 1.0, 3.0, 5.0, 10.0, 20.0, 30.0, 60.0, 120.0]
+    ), &[]).unwrap();
+
     pub static ref SIGNER_LOCAL_STATE_MACHINE: Mutex<Option<LocalStateMachine>> = Mutex::new(None);
 }