Merge pull request #6091 from stacks-network/release/3.1.0.0.9

Merge release/3.1.0.0.9 to master

Author: wileyj

.github/workflows/clarity-js-sdk-pr.yml

@@ -5,6 +5,25 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to the versioning scheme outlined in the [README.md](README.md).
 
+## [3.1.0.0.9]
+
+### 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.
+
+### 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
@@ -19,7 +38,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 - When a miner times out waiting for signatures, it will re-propose the same block instead of building a new block ([#5877](https://github.com/stacks-network/stacks-core/pull/5877))
 - Improve tenure downloader trace verbosity applying proper logging level depending on the tenure state ("debug" if unconfirmed, "info" otherwise) ([#5871](https://github.com/stacks-network/stacks-core/issues/5871))
-- Remove warning log about missing UTXOs when a node is configured as `miner` with `mock_mining` mode enabled ([#5841](https://github.com/stacks-network/stacks-core/issues/5841)) 
+- Remove warning log about missing UTXOs when a node is configured as `miner` with `mock_mining` mode enabled ([#5841](https://github.com/stacks-network/stacks-core/issues/5841))
 - Deprecated the `wait_on_interim_blocks` option in the miner config file. This option is no longer needed, as the miner will always wait for interim blocks to be processed before mining a new block. To wait extra time in between blocks, use the `min_time_between_blocks_ms` option instead. ([#5979](https://github.com/stacks-network/stacks-core/pull/5979))
 - Added `empty_mempool_sleep_ms` to the miner config file to control the time to wait in between mining attempts when the mempool is empty. If not set, the default sleep time is 2.5s. ([#5997](https://github.com/stacks-network/stacks-core/pull/5997))
 

CHANGELOG.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.

CONTRIBUTING.md

@@ -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))
         }
     }
 }