Merge branch 'develop' into chore/remove-unused-dependencies

Author: hugocaillard

.gitattributes

@@ -1,2 +1,6 @@
 legacy/* linguist-vendored
-* text=lf
+# Enforcing 'lf' eol mainly for:
+# - 'stx-genesis' package, where txt files need hash computation and comparison
+# - 'clarity' package, where clarity language is sensitive to line endings for .clar files
+# anyhow, setting eol for all text files to have a homogeneous management over the whole code base
+* text eol=lf

.github/workflows/bitcoin-tests.yml

@@ -106,6 +106,13 @@ jobs:
           - test-name: tests::epoch_24::verify_auto_unlock_behavior
           # Disable this flaky test. We don't need continue testing Epoch 2 -> 3 transition
           - test-name: tests::nakamoto_integrations::flash_blocks_on_epoch_3_FLAKY
+          # These mempool tests take a long time to run, and are meant to be run manually
+          - test-name: tests::nakamoto_integrations::large_mempool_original_constant_fee
+          - test-name: tests::nakamoto_integrations::large_mempool_original_random_fee
+          - test-name: tests::nakamoto_integrations::large_mempool_next_constant_fee
+          - test-name: tests::nakamoto_integrations::large_mempool_next_random_fee
+          - test-name: tests::nakamoto_integrations::larger_mempool
+          - test-name: tests::signer::v0::larger_mempool
 
     steps:
       ## Setup test environment

.vscode/settings.json

@@ -7,5 +7,6 @@
     },
     "rust-analyzer.rustfmt.extraArgs": [
         "+nightly"
-    ]
+    ],
+    "files.eol": "\n"
 }

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 ### Added
 
 - Add fee information to transaction log ending with "success" or "skipped", while building a new block
+- Add `max_execution_time_secs` to miner config for limiting duration of contract calls
 - When a miner's config file is updated (ie with a new fee rate), a new block commit is issued using
   the new values ([#5924](https://github.com/stacks-network/stacks-core/pull/5924))
 

Cargo.lock

@@ -77,6 +77,8 @@ _Warning, this typically takes a few minutes_
 cargo nextest run
 ```
 
+_On Windows, many tests will fail, mainly due to parallelism. To mitigate the issue you may need to run the tests individually._
+
 ## Run the testnet
 
 You can observe the state machine in action locally by running:
@@ -85,8 +87,6 @@ You can observe the state machine in action locally by running:
 cargo run --bin stacks-node -- start --config ./sample/conf/testnet-follower-conf.toml
 ```
 
-_On Windows, many tests will fail if the line endings aren't `LF`. Please ensure that you have git's `core.autocrlf` set to `input` when you clone the repository to avoid any potential issues. This is due to the Clarity language currently being sensitive to line endings._
-
 Additional testnet documentation is available [here](./docs/testnet.md) and [here](https://docs.stacks.co/docs/nodes-and-miners/miner-testnet)
 
 ## Release Process

README.md

@@ -194,6 +194,9 @@ pub enum CheckErrors {
 
     WriteAttemptedInReadOnly,
     AtBlockClosureMustBeReadOnly,
+
+    // time checker errors
+    ExecutionTimeExpired,
 }
 
 #[derive(Debug, PartialEq)]
@@ -277,6 +280,7 @@ impl From<CostErrors> for CheckErrors {
                 CheckErrors::Expects("Unexpected interpreter failure in cost computation".into())
             }
             CostErrors::Expect(s) => CheckErrors::Expects(s),
+            CostErrors::ExecutionTimeExpired => CheckErrors::ExecutionTimeExpired,
         }
     }
 }
@@ -466,6 +470,7 @@ impl DiagnosableError for CheckErrors {
             CheckErrors::UncheckedIntermediaryResponses => "intermediary responses in consecutive statements must be checked".into(),
             CheckErrors::CostComputationFailed(s) => format!("contract cost computation failed: {}", s),
             CheckErrors::CouldNotDetermineSerializationType => "could not determine the input type for the serialization function".into(),
+            CheckErrors::ExecutionTimeExpired => "execution time expired".into(),
         }
     }
 

clarity/src/vm/analysis/errors.rs

@@ -92,6 +92,8 @@ pub enum ParseErrors {
     UnexpectedParserFailure,
     /// Should be an unreachable failure which invalidates the transaction
     InterpreterFailure,
+
+    ExecutionTimeExpired,
 }
 
 #[derive(Debug, PartialEq)]
@@ -173,6 +175,7 @@ impl From<CostErrors> for ParseError {
             CostErrors::InterpreterFailure | CostErrors::Expect(_) => {
                 ParseError::new(ParseErrors::InterpreterFailure)
             }
+            CostErrors::ExecutionTimeExpired => ParseError::new(ParseErrors::ExecutionTimeExpired),
         }
     }
 }
@@ -299,6 +302,7 @@ impl DiagnosableError for ParseErrors {
             ParseErrors::NoteToMatchThis(token) => format!("to match this '{}'", token),
             ParseErrors::UnexpectedParserFailure => "unexpected failure while parsing".to_string(),
             ParseErrors::InterpreterFailure => "unexpected failure while parsing".to_string(),
+            ParseErrors::ExecutionTimeExpired => "max execution time expired".to_string(),
         }
     }
 

clarity/src/vm/ast/errors.rs

@@ -61,6 +61,9 @@ impl From<CheckError> for Error {
             CheckErrors::MemoryBalanceExceeded(_a, _b) => {
                 Error::CostError(ExecutionCost::max_value(), ExecutionCost::max_value())
             }
+            CheckErrors::ExecutionTimeExpired => {
+                Error::CostError(ExecutionCost::max_value(), ExecutionCost::max_value())
+            }
             _ => Error::Analysis(e),
         }
     }
@@ -75,6 +78,9 @@ impl From<InterpreterError> for Error {
             InterpreterError::Unchecked(CheckErrors::CostOverflow) => {
                 Error::CostError(ExecutionCost::max_value(), ExecutionCost::max_value())
             }
+            InterpreterError::Unchecked(CheckErrors::ExecutionTimeExpired) => {
+                Error::CostError(ExecutionCost::max_value(), ExecutionCost::max_value())
+            }
             _ => Error::Interpreter(e),
         }
     }
@@ -90,6 +96,9 @@ impl From<ParseError> for Error {
             ParseErrors::MemoryBalanceExceeded(_a, _b) => {
                 Error::CostError(ExecutionCost::max_value(), ExecutionCost::max_value())
             }
+            ParseErrors::ExecutionTimeExpired => {
+                Error::CostError(ExecutionCost::max_value(), ExecutionCost::max_value())
+            }
             _ => Error::Parse(e),
         }
     }
@@ -284,6 +293,7 @@ pub trait TransactionConnection: ClarityConnection {
     /// 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).
+    #[allow(clippy::too_many_arguments)]
     fn run_contract_call<F>(
         &mut self,
         sender: &PrincipalData,
@@ -292,6 +302,7 @@ pub trait TransactionConnection: ClarityConnection {
         public_function: &str,
         args: &[Value],
         abort_call_back: F,
+        max_execution_time: Option<std::time::Duration>,
     ) -> Result<(Value, AssetMap, Vec<StacksTransactionEvent>), Error>
     where
         F: FnOnce(&AssetMap, &mut ClarityDatabase) -> bool,
@@ -303,6 +314,11 @@ pub trait TransactionConnection: ClarityConnection {
 
         self.with_abort_callback(
             |vm_env| {
+                if let Some(max_execution_time_duration) = max_execution_time {
+                    vm_env
+                        .context
+                        .set_max_execution_time(max_execution_time_duration);
+                }
                 vm_env
                     .execute_transaction(
                         sender.clone(),
@@ -329,6 +345,7 @@ pub trait TransactionConnection: ClarityConnection {
     /// 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).
+    #[allow(clippy::too_many_arguments)]
     fn initialize_smart_contract<F>(
         &mut self,
         identifier: &QualifiedContractIdentifier,
@@ -337,12 +354,18 @@ pub trait TransactionConnection: ClarityConnection {
         contract_str: &str,
         sponsor: Option<PrincipalData>,
         abort_call_back: F,
+        max_execution_time: Option<std::time::Duration>,
     ) -> Result<(AssetMap, Vec<StacksTransactionEvent>), Error>
     where
         F: FnOnce(&AssetMap, &mut ClarityDatabase) -> bool,
     {
         let (_, asset_map, events, aborted) = self.with_abort_callback(
             |vm_env| {
+                if let Some(max_execution_time_duration) = max_execution_time {
+                    vm_env
+                        .context
+                        .set_max_execution_time(max_execution_time_duration);
+                }
                 vm_env
                     .initialize_contract_from_ast(
                         identifier.clone(),

clarity/src/vm/clarity.rs

@@ -17,6 +17,7 @@
 use std::collections::BTreeMap;
 use std::fmt;
 use std::mem::replace;
+use std::time::{Duration, Instant};
 
 use hashbrown::{HashMap, HashSet};
 use serde::Serialize;
@@ -181,6 +182,17 @@ pub struct EventBatch {
     pub events: Vec<StacksTransactionEvent>,
 }
 
+/** ExecutionTimeTracker keeps track of how much time a contract call is taking.
+   It is checked at every eval call.
+*/
+pub enum ExecutionTimeTracker {
+    NoTracking,
+    MaxTime {
+        start_time: Instant,
+        max_duration: Duration,
+    },
+}
+
 /** GlobalContext represents the outermost context for a single transaction's
      execution. It tracks an asset changes that occurred during the
      processing of the transaction, whether or not the current context is read_only,
@@ -199,6 +211,7 @@ pub struct GlobalContext<'a, 'hooks> {
     /// This is the chain ID of the transaction
     pub chain_id: u32,
     pub eval_hooks: Option<Vec<&'hooks mut dyn EvalHook>>,
+    pub execution_time_tracker: ExecutionTimeTracker,
 }
 
 #[derive(Serialize, Deserialize, Clone)]
@@ -1553,13 +1566,21 @@ impl<'a, 'hooks> GlobalContext<'a, 'hooks> {
             epoch_id,
             chain_id,
             eval_hooks: None,
+            execution_time_tracker: ExecutionTimeTracker::NoTracking,
         }
     }
 
     pub fn is_top_level(&self) -> bool {
         self.asset_maps.is_empty()
     }
 
+    pub fn set_max_execution_time(&mut self, max_execution_time: Duration) {
+        self.execution_time_tracker = ExecutionTimeTracker::MaxTime {
+            start_time: Instant::now(),
+            max_duration: max_execution_time,
+        }
+    }
+
     fn get_asset_map(&mut self) -> Result<&mut AssetMap> {
         self.asset_maps
             .last_mut()