Merge branch 'develop' of https://github.com/stacks-network/stacks-core into chore/add-vm-error-to-transaction-event-payload

Author: jferrant

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

@@ -254,16 +254,13 @@ impl SqliteConnection {
 
         Ok(())
     }
+
     pub fn memory() -> Result<Connection> {
         let contract_db = SqliteConnection::inner_open(":memory:")?;
         SqliteConnection::initialize_conn(&contract_db)?;
         Ok(contract_db)
     }
-    pub fn open(filename: &str) -> Result<Connection> {
-        let contract_db = SqliteConnection::inner_open(filename)?;
-        SqliteConnection::check_schema(&contract_db)?;
-        Ok(contract_db)
-    }
+
     pub fn check_schema(conn: &Connection) -> Result<()> {
         let sql = "SELECT sql FROM sqlite_master WHERE name=?";
         let _: String = conn
@@ -272,10 +269,13 @@ impl SqliteConnection {
         let _: String = conn
             .query_row(sql, params!["metadata_table"], |row| row.get(0))
             .map_err(|x| InterpreterError::SqliteError(IncomparableError { err: x }))?;
+        let _: String = conn
+            .query_row(sql, params!["md_blockhashes"], |row| row.get(0))
+            .map_err(|x| InterpreterError::SqliteError(IncomparableError { err: x }))?;
         Ok(())
     }
 
-    pub fn inner_open(filename: &str) -> Result<Connection> {
+    fn inner_open(filename: &str) -> Result<Connection> {
         let conn = Connection::open(filename)
             .map_err(|x| InterpreterError::SqliteError(IncomparableError { err: x }))?;
 

clarity/src/vm/database/sqlite.rs

@@ -20,7 +20,7 @@ use std::collections::{HashMap, HashSet};
 use std::io::{ErrorKind, Write};
 use std::ops::{Deref, DerefMut};
 use std::str::FromStr;
-use std::sync::{Arc, LazyLock, Mutex};
+use std::sync::{Arc, LazyLock, Mutex, MutexGuard};
 use std::{cmp, fmt, fs};
 
 use clarity::util::lru_cache::LruCache;
@@ -98,8 +98,9 @@ pub const REWARD_WINDOW_END: u64 = 144 * 90 + REWARD_WINDOW_START;
 
 pub type BlockHeaderCache = HashMap<ConsensusHash, (Option<BlockHeaderHash>, ConsensusHash)>;
 
+const DESCENDANCY_CACHE_SIZE: usize = 2000;
 static DESCENDANCY_CACHE: LazyLock<Arc<Mutex<LruCache<(SortitionId, BlockHeaderHash), bool>>>> =
-    LazyLock::new(|| Arc::new(Mutex::new(LruCache::new(2000))));
+    LazyLock::new(|| Arc::new(Mutex::new(LruCache::new(DESCENDANCY_CACHE_SIZE))));
 
 pub enum FindIter<R> {
     Found(R),
@@ -1091,6 +1092,38 @@ pub trait SortitionHandle {
         Ok(Some(StacksBlockId::new(&ch, &bhh)))
     }
 
+    /// Check if the descendancy cache has an entry for whether or not the winning block in `key.0`
+    ///  descends from `key.1`
+    ///
+    /// If it does, return the cached entry
+    fn descendancy_cache_get(
+        cache: &mut MutexGuard<'_, LruCache<(SortitionId, BlockHeaderHash), bool>>,
+        key: &(SortitionId, BlockHeaderHash),
+    ) -> Option<bool> {
+        match cache.get(key) {
+            Ok(result) => result,
+            // cache is broken, create a new one
+            Err(e) => {
+                error!("SortitionDB's descendant cache errored. Will continue operation with cleared cache"; "err" => %e);
+                **cache = LruCache::new(DESCENDANCY_CACHE_SIZE);
+                None
+            }
+        }
+    }
+
+    /// Cache the result of the descendancy check on whether or not the winning block in `key.0`
+    ///  descends from `key.1`
+    fn descendancy_cache_put(
+        cache: &mut MutexGuard<'_, LruCache<(SortitionId, BlockHeaderHash), bool>>,
+        key: (SortitionId, BlockHeaderHash),
+        is_descended: bool,
+    ) {
+        if let Err(e) = cache.insert_clean(key, is_descended) {
+            error!("SortitionDB's descendant cache errored. Will continue operation with cleared cache"; "err" => %e);
+            **cache = LruCache::new(DESCENDANCY_CACHE_SIZE);
+        }
+    }
+
     /// is the given block a descendant of `potential_ancestor`?
     ///  * block_at_burn_height: the burn height of the sortition that chose the stacks block to check
     ///  * potential_ancestor: the stacks block hash of the potential ancestor
@@ -1125,39 +1158,36 @@ pub trait SortitionHandle {
             .expect("FATAL: lock poisoned in SortitionDB");
 
         while sn.block_height >= earliest_block_height {
-            match cache.get(&(sn.sortition_id, potential_ancestor.clone())) {
-                Ok(Some(result)) => {
+            let cache_check_key = (sn.sortition_id, potential_ancestor.clone());
+            match Self::descendancy_cache_get(&mut cache, &cache_check_key) {
+                Some(result) => {
                     if sn.sortition_id != top_sortition_id {
-                        if let Err(_) = cache
-                            .insert_clean((top_sortition_id, potential_ancestor.clone()), result)
-                        {
-                            *cache = LruCache::new(2000);
-                        }
+                        Self::descendancy_cache_put(
+                            &mut cache,
+                            (top_sortition_id, cache_check_key.1),
+                            result,
+                        );
                     }
                     return Ok(result);
                 }
                 // not cached, don't need to do anything.
-                Ok(None) => {}
-                // cache is broken, create a new one
-                Err(_) => {
-                    *cache = LruCache::new(2000);
-                }
+                None => {}
             }
 
             if !sn.sortition {
-                if let Err(_) =
-                    cache.insert_clean((top_sortition_id, potential_ancestor.clone()), false)
-                {
-                    *cache = LruCache::new(2000);
-                }
+                Self::descendancy_cache_put(
+                    &mut cache,
+                    (top_sortition_id, cache_check_key.1),
+                    false,
+                );
                 return Ok(false);
             }
             if &sn.winning_stacks_block_hash == potential_ancestor {
-                if let Err(_) =
-                    cache.insert_clean((top_sortition_id, potential_ancestor.clone()), true)
-                {
-                    *cache = LruCache::new(2000);
-                }
+                Self::descendancy_cache_put(
+                    &mut cache,
+                    (top_sortition_id, cache_check_key.1),
+                    true,
+                );
                 return Ok(true);
             }
 
@@ -1193,9 +1223,11 @@ pub trait SortitionHandle {
                 }
             }
         }
-        if let Err(_) = cache.insert_clean((top_sortition_id, potential_ancestor.clone()), false) {
-            *cache = LruCache::new(2000);
-        }
+        Self::descendancy_cache_put(
+            &mut cache,
+            (top_sortition_id, potential_ancestor.clone()),
+            false,
+        );
         return Ok(false);
     }
 }

stackslib/src/chainstate/burn/db/sortdb.rs

@@ -1940,6 +1940,7 @@ impl ClarityTransactionConnection<'_, '_> {
 #[cfg(test)]
 mod tests {
     use std::fs;
+    use std::path::PathBuf;
 
     use clarity::vm::analysis::errors::CheckErrors;
     use clarity::vm::database::{ClarityBackingStore, STXBalance};
@@ -1950,10 +1951,57 @@ mod tests {
     use stacks_common::types::sqlite::NO_PARAMS;
 
     use super::*;
+    use crate::chainstate::stacks::index::marf::{MARFOpenOpts, MarfConnection as _};
     use crate::chainstate::stacks::index::ClarityMarfTrieId;
     use crate::clarity_vm::database::marf::MarfedKV;
     use crate::core::{PEER_VERSION_EPOCH_1_0, PEER_VERSION_EPOCH_2_0, PEER_VERSION_EPOCH_2_05};
 
+    #[test]
+    pub fn create_md_index() {
+        let path_db = "/tmp/stacks-node-tests/creat_md_index";
+        std::fs::remove_dir_all(path_db);
+        let mut path = PathBuf::from(path_db);
+
+        std::fs::create_dir_all(&path).unwrap();
+
+        path.push("marf.sqlite");
+        let marf_path = path.to_str().unwrap().to_string();
+
+        let mut marf_opts = MARFOpenOpts::default();
+        marf_opts.external_blobs = true;
+
+        let mut marf: MARF<StacksBlockId> = MARF::from_path(&marf_path, marf_opts).unwrap();
+
+        let tx = marf.storage_tx().unwrap();
+
+        tx.query_row("PRAGMA journal_mode = WAL;", NO_PARAMS, |_row| Ok(()))
+            .unwrap();
+
+        tx.execute(
+            "CREATE TABLE IF NOT EXISTS data_table
+                      (key TEXT PRIMARY KEY, value TEXT)",
+            NO_PARAMS,
+        )
+        .unwrap();
+
+        tx.execute(
+            "CREATE TABLE IF NOT EXISTS metadata_table
+                      (key TEXT NOT NULL, blockhash TEXT, value TEXT,
+                       UNIQUE (key, blockhash))",
+            NO_PARAMS,
+        )
+        .unwrap();
+
+        tx.commit().unwrap();
+
+        assert!(SqliteConnection::check_schema(marf.sqlite_conn()).is_err());
+
+        MarfedKV::open(path_db, None, None).unwrap();
+
+        // schema should be good now
+        assert!(SqliteConnection::check_schema(marf.sqlite_conn()).is_ok());
+    }
+
     #[test]
     pub fn bad_syntax_test() {
         let marf = MarfedKV::temporary();

stackslib/src/clarity_vm/clarity.rs

@@ -52,7 +52,7 @@ fn stacker_db_id(i: usize) -> QualifiedContractIdentifier {
 
 fn make_stacker_db_ids(i: usize) -> Vec<QualifiedContractIdentifier> {
     let mut dbs = vec![];
-    for j in 0..i {
+    for j in 0..i + 1 {
         dbs.push(stacker_db_id(j));
     }
     dbs
@@ -1053,6 +1053,45 @@ fn run_topology_test_ex<F>(
             (100.0 * (peer_counts as f64)) / ((peer_count * peer_count) as f64),
         );
 
+        // wait for stacker DBs to converge
+        for (i, peer) in peers.iter().enumerate() {
+            if i % 2 != 0 {
+                continue;
+            }
+            for (j, other_peer) in peers.iter().enumerate() {
+                if i == j {
+                    continue;
+                }
+
+                let all_neighbors =
+                    PeerDB::get_all_peers(other_peer.network.peerdb.conn()).unwrap();
+
+                if (all_neighbors.len() as u64) < ((peer_count - 1) as u64) {
+                    // this is a simulated-NAT'ed node -- it won't learn about other NAT'ed nodes'
+                    // DBs
+                    continue;
+                }
+
+                if j % 2 != 0 {
+                    continue; // this peer doesn't support Stacker DBs
+                }
+                let dbs = peer
+                    .network
+                    .peerdb
+                    .get_peer_stacker_dbs(&other_peer.config.to_neighbor())
+                    .unwrap();
+                if dbs.is_empty() {
+                    test_debug!(
+                        "waiting for peer {i} {} to learn about peer {j} {}'s stacker DBs",
+                        &peer.config.to_neighbor(),
+                        &other_peer.config.to_neighbor()
+                    );
+                    finished = false;
+                    break;
+                }
+            }
+        }
+
         if finished {
             break;
         }

stackslib/src/net/tests/convergence.rs

@@ -51,6 +51,7 @@ http-types = "2.12"
 tempfile = "3.3"
 mockito = "1.5"
 serial_test = "3.2.0"
+pinny = { git = "https://github.com/BitcoinL2-Labs/pinny-rs.git", rev = "54ba9d533a7b84525a5e65a3eae1a3ae76b9ea49" } #v0.0.2
 madhouse = { git = "https://github.com/stacks-network/madhouse-rs.git", rev = "fc651ddcbaf85e888b06d4a87aa788c4b7ba9309" }
 proptest = { git = "https://github.com/proptest-rs/proptest.git", rev = "c9bdf18c232665b2b740c667c81866b598d06dc7" }
 
@@ -68,3 +69,6 @@ slog_json = ["stacks/slog_json", "stacks-common/slog_json", "clarity/slog_json"]
 prod-genesis-chainstate = []
 default = []
 testing = []
+
+[package.metadata.pinny] 
+allowed = ["bitcoind", "flaky", "slow"]