Merge branch 'develop' into feat/naka-timestamp

.github/workflows/bitcoin-tests.yml

@@ -99,6 +99,7 @@ jobs:
           - tests::nakamoto_integrations::clarity_burn_state
           - tests::nakamoto_integrations::check_block_times
           - tests::nakamoto_integrations::check_block_info
+          - tests::nakamoto_integrations::continue_tenure_extend
           # Do not run this one until we figure out why it fails in CI
           # - tests::neon_integrations::bitcoin_reorg_flap
           # - tests::neon_integrations::bitcoin_reorg_flap_with_follower

libsigner/src/v1/messages.rs

@@ -1464,6 +1464,8 @@ mod test {
     use super::{StacksMessageCodecExtensions, *};
 
     #[test]
+    #[should_panic]
+    // V1 signer slots do not have enough slots in Epoch 2.5. Something will need to be updated!
     fn signer_slots_count_is_sane() {
         let slot_identifiers_len = MessageSlotID::ALL.len();
         assert!(

stacks-common/src/libcommon.rs

@@ -63,5 +63,20 @@ pub mod consts {
 
     /// The number of StackerDB slots each signing key needs
     ///  to use to participate in DKG and block validation signing.
-    pub const SIGNER_SLOTS_PER_USER: u32 = 14;
+    pub const SIGNER_SLOTS_PER_USER: u32 = 13;
+}
+
+/// This test asserts that the constant above doesn't change.
+/// This exists because the constant above is used by Epoch 2.5 instantiation code.
+///
+/// Adding more slots will require instantiating more .signers contracts through either
+///  consensus changes (i.e., a new epoch) or through non-consensus-critical contract
+///  deployments.
+#[test]
+fn signer_slots_count_2_5() {
+    assert_eq!(
+        consts::SIGNER_SLOTS_PER_USER,
+        13,
+        "The .signers-x-y contracts in Epoch 2.5 were instantiated with 13 slots"
+    );
 }

stacks-common/src/types/mod.rs

@@ -87,11 +87,31 @@ pub enum StacksEpochId {
     Epoch30 = 0x03000,
 }
 
+pub enum MempoolCollectionBehavior {
+    ByStacksHeight,
+    ByReceiveTime,
+}
+
 impl StacksEpochId {
     pub fn latest() -> StacksEpochId {
         StacksEpochId::Epoch30
     }
 
+    /// In this epoch, how should the mempool perform garbage collection?
+    pub fn mempool_garbage_behavior(&self) -> MempoolCollectionBehavior {
+        match self {
+            StacksEpochId::Epoch10
+            | StacksEpochId::Epoch20
+            | StacksEpochId::Epoch2_05
+            | StacksEpochId::Epoch21
+            | StacksEpochId::Epoch22
+            | StacksEpochId::Epoch23
+            | StacksEpochId::Epoch24
+            | StacksEpochId::Epoch25 => MempoolCollectionBehavior::ByStacksHeight,
+            StacksEpochId::Epoch30 => MempoolCollectionBehavior::ByReceiveTime,
+        }
+    }
+
     /// Returns whether or not this Epoch should perform
     ///  memory checks during analysis
     pub fn analysis_memory(&self) -> bool {

stacks-signer/src/client/stacks_client.rs

@@ -1205,7 +1205,7 @@ mod tests {
                 let principal_data = StacksAddress::from_string(signer).unwrap().into();
 
                 let data_map = [
-                    ("num-slots".into(), ClarityValue::UInt(14)),
+                    ("num-slots".into(), ClarityValue::UInt(13)),
                     (
                         "signer".into(),
                         ClarityValue::Principal(PrincipalData::Standard(principal_data)),

stackslib/src/chainstate/nakamoto/miner.rs

@@ -82,6 +82,7 @@ use crate::util_lib::boot::boot_code_id;
 use crate::util_lib::db::Error as DBError;
 
 /// Nakamaoto tenure information
+#[derive(Debug)]
 pub struct NakamotoTenureInfo {
     /// Coinbase tx, if this is a new tenure
     pub coinbase_tx: Option<StacksTransaction>,
@@ -568,6 +569,7 @@ impl NakamotoBlockBuilder {
             "execution_consumed" => %consumed,
             "%-full" => block_limit.proportion_largest_dimension(&consumed),
             "assembly_time_ms" => ts_end.saturating_sub(ts_start),
+            "consensus_hash" => %block.header.consensus_hash
         );
 
         Ok((block, consumed, size, tx_events))

stackslib/src/chainstate/nakamoto/mod.rs

@@ -865,7 +865,7 @@ impl NakamotoBlock {
             // discontinuous
             warn!(
                 "Invalid block -- discontiguous";
-                "previosu_tenure_end" => %tc_payload.previous_tenure_end,
+                "previous_tenure_end" => %tc_payload.previous_tenure_end,
                 "parent_block_id" => %self.header.parent_block_id
             );
             return Err(());
@@ -3086,44 +3086,30 @@ impl NakamotoChainState {
         let burn_header_height = tenure_block_snapshot.block_height;
         let block_hash = block.header.block_hash();
 
-        let new_tenure = match block.is_wellformed_tenure_start_block() {
-            Ok(true) => true,
-            Ok(false) => {
-                // this block is mined in the ongoing tenure.
-                if !Self::check_tenure_continuity(
-                    chainstate_tx,
-                    burn_dbconn,
-                    &parent_ch,
-                    &block.header,
-                )? {
-                    // this block is not part of the ongoing tenure; it's invalid
-                    return Err(ChainstateError::ExpectedTenureChange);
-                }
-                false
-            }
-            Err(_) => {
-                return Err(ChainstateError::InvalidStacksBlock(
-                    "Invalid tenure changes in nakamoto block".into(),
-                ));
-            }
-        };
+        let new_tenure = block.is_wellformed_tenure_start_block().map_err(|_| {
+            ChainstateError::InvalidStacksBlock("Invalid tenure changes in nakamoto block".into())
+        })?;
+        // this block is mined in the ongoing tenure.
+        if !new_tenure
+            && !Self::check_tenure_continuity(
+                chainstate_tx,
+                burn_dbconn,
+                &parent_ch,
+                &block.header,
+            )?
+        {
+            // this block is not part of the ongoing tenure; it's invalid
+            return Err(ChainstateError::ExpectedTenureChange);
+        }
+        let tenure_extend = block.is_wellformed_tenure_extend_block().map_err(|_| {
+            ChainstateError::InvalidStacksBlock("Invalid tenure changes in nakamoto block".into())
+        })?;
 
-        let tenure_extend = match block.is_wellformed_tenure_extend_block() {
-            Ok(true) => {
-                if new_tenure {
-                    return Err(ChainstateError::InvalidStacksBlock(
-                        "Both started and extended tenure".into(),
-                    ));
-                }
-                true
-            }
-            Ok(false) => false,
-            Err(_) => {
-                return Err(ChainstateError::InvalidStacksBlock(
-                    "Invalid tenure extend in nakamoto block".into(),
-                ));
-            }
-        };
+        if tenure_extend && new_tenure {
+            return Err(ChainstateError::InvalidStacksBlock(
+                "Both started and extended tenure".into(),
+            ));
+        }
 
         let parent_coinbase_height = if block.is_first_mined() {
             0

stackslib/src/chainstate/nakamoto/tenure.rs

@@ -946,7 +946,7 @@ impl NakamotoChainState {
     }
 
     /// Check that this block is in the same tenure as its parent, and that this tenure is the
-    /// highest-seen tenure.  Use this to check blocks that do _not_ have tenure-changes.
+    /// highest-seen tenure.  Use this to check blocks that do _not_ have BlockFound tenure-changes.
     ///
     /// Returns Ok(bool) to indicate whether or not this block is in the same tenure as its parent.
     /// Returns Err(..) on DB error

stackslib/src/core/mempool.rs

@@ -21,7 +21,7 @@ use std::io::{Read, Write};
 use std::ops::{Deref, DerefMut};
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
-use std::time::Instant;
+use std::time::{Duration, Instant, SystemTime};
 use std::{fs, io};
 
 use clarity::vm::types::PrincipalData;
@@ -37,6 +37,7 @@ use stacks_common::codec::{
     read_next, write_next, Error as codec_error, StacksMessageCodec, MAX_MESSAGE_LEN,
 };
 use stacks_common::types::chainstate::{BlockHeaderHash, StacksAddress, StacksBlockId};
+use stacks_common::types::MempoolCollectionBehavior;
 use stacks_common::util::hash::{to_hex, Sha512Trunc256Sum};
 use stacks_common::util::retry::{BoundReader, RetryReader};
 use stacks_common::util::{get_epoch_time_ms, get_epoch_time_secs};
@@ -71,8 +72,10 @@ use crate::util_lib::db::{
 use crate::{cost_estimates, monitoring};
 
 // maximum number of confirmations a transaction can have before it's garbage-collected
-pub const MEMPOOL_MAX_TRANSACTION_AGE: u64 = 256;
-pub const MAXIMUM_MEMPOOL_TX_CHAINING: u64 = 25;
+pub static MEMPOOL_MAX_TRANSACTION_AGE: u64 = 256;
+pub static MAXIMUM_MEMPOOL_TX_CHAINING: u64 = 25;
+pub static MEMPOOL_NAKAMOTO_MAX_TRANSACTION_AGE: Duration =
+    Duration::from_secs(MEMPOOL_MAX_TRANSACTION_AGE * 10 * 60);
 
 // name of table for storing the counting bloom filter
 pub const BLOOM_COUNTER_TABLE: &'static str = "txid_bloom_counter";
@@ -2206,10 +2209,58 @@ impl MemPoolDB {
         Ok(())
     }
 
-    /// Garbage-collect the mempool.  Remove transactions that have a given number of
-    /// confirmations.
+    /// Garbage-collect the mempool according to the behavior specified in `behavior`.
     pub fn garbage_collect(
-        tx: &mut MemPoolTx,
+        &mut self,
+        chain_height: u64,
+        behavior: &MempoolCollectionBehavior,
+        event_observer: Option<&dyn MemPoolEventDispatcher>,
+    ) -> Result<(), db_error> {
+        let tx = self.tx_begin()?;
+        match behavior {
+            MempoolCollectionBehavior::ByStacksHeight => {
+                let Some(min_height) = chain_height.checked_sub(MEMPOOL_MAX_TRANSACTION_AGE) else {
+                    return Ok(());
+                };
+                Self::garbage_collect_by_height(&tx, min_height, event_observer)?;
+            }
+            MempoolCollectionBehavior::ByReceiveTime => {
+                Self::garbage_collect_by_time(
+                    &tx,
+                    &MEMPOOL_NAKAMOTO_MAX_TRANSACTION_AGE,
+                    event_observer,
+                )?;
+            }
+        };
+        tx.commit()
+    }
+
+    /// Garbage-collect the mempool. Remove transactions that were accepted more than `age` ago.
+    /// The granularity of this check is in seconds.
+    pub fn garbage_collect_by_time(
+        tx: &MemPoolTx,
+        age: &Duration,
+        event_observer: Option<&dyn MemPoolEventDispatcher>,
+    ) -> Result<(), db_error> {
+        let threshold_time = get_epoch_time_secs().saturating_sub(age.as_secs());
+        let args: &[&dyn ToSql] = &[&u64_to_sql(threshold_time)?];
+        if let Some(event_observer) = event_observer {
+            let sql = "SELECT txid FROM mempool WHERE accept_time < ?1";
+            let txids = query_rows(tx, sql, args)?;
+            event_observer.mempool_txs_dropped(txids, MemPoolDropReason::STALE_COLLECT);
+        }
+
+        let sql = "DELETE FROM mempool WHERE accept_time < ?1";
+
+        tx.execute(sql, args)?;
+        increment_stx_mempool_gc();
+        Ok(())
+    }
+
+    /// Garbage-collect the mempool.  Remove transactions that were received `min_height`
+    ///  blocks ago.
+    pub fn garbage_collect_by_height(
+        tx: &MemPoolTx,
         min_height: u64,
         event_observer: Option<&dyn MemPoolEventDispatcher>,
     ) -> Result<(), db_error> {
@@ -2230,10 +2281,9 @@ impl MemPoolDB {
 
     #[cfg(test)]
     pub fn clear_before_height(&mut self, min_height: u64) -> Result<(), db_error> {
-        let mut tx = self.tx_begin()?;
-        MemPoolDB::garbage_collect(&mut tx, min_height, None)?;
-        tx.commit()?;
-        Ok(())
+        let tx = self.tx_begin()?;
+        MemPoolDB::garbage_collect_by_height(&tx, min_height, None)?;
+        tx.commit()
     }
 
     /// Scan the chain tip for all available transactions (but do not remove them!)

stackslib/src/core/tests/mod.rs

@@ -15,6 +15,7 @@
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 use std::collections::{HashMap, HashSet};
+use std::time::Duration;
 use std::{cmp, io};
 
 use clarity::vm::costs::ExecutionCost;
@@ -31,7 +32,7 @@ use stacks_common::types::chainstate::{
     BlockHeaderHash, BurnchainHeaderHash, StacksAddress, StacksBlockId, StacksWorkScore, TrieHash,
     VRFSeed,
 };
-use stacks_common::types::StacksEpochId;
+use stacks_common::types::{MempoolCollectionBehavior, StacksEpochId};
 use stacks_common::util::hash::{hex_bytes, to_hex, Hash160, *};
 use stacks_common::util::secp256k1::{MessageSignature, *};
 use stacks_common::util::vrf::VRFProof;
@@ -1388,8 +1389,10 @@ fn mempool_do_not_replace_tx() {
     assert!(!MemPoolDB::db_has_tx(&mempool_tx, &txid).unwrap());
 }
 
-#[test]
-fn mempool_db_load_store_replace_tx() {
+#[rstest]
+#[case(MempoolCollectionBehavior::ByStacksHeight)]
+#[case(MempoolCollectionBehavior::ByReceiveTime)]
+fn mempool_db_load_store_replace_tx(#[case] behavior: MempoolCollectionBehavior) {
     let mut chainstate = instantiate_chainstate(false, 0x80000000, function_name!());
     let chainstate_path = chainstate_path(function_name!());
     let mut mempool = MemPoolDB::open_test(false, 0x80000000, &chainstate_path).unwrap();
@@ -1616,7 +1619,17 @@ fn mempool_db_load_store_replace_tx() {
 
     eprintln!("garbage-collect");
     let mut mempool_tx = mempool.tx_begin().unwrap();
-    MemPoolDB::garbage_collect(&mut mempool_tx, 101, None).unwrap();
+    match behavior {
+        MempoolCollectionBehavior::ByStacksHeight => {
+            MemPoolDB::garbage_collect_by_height(&mut mempool_tx, 101, None)
+        }
+        MempoolCollectionBehavior::ByReceiveTime => {
+            let test_max_age = Duration::from_secs(1);
+            std::thread::sleep(2 * test_max_age);
+            MemPoolDB::garbage_collect_by_time(&mut mempool_tx, &test_max_age, None)
+        }
+    }
+    .unwrap();
     mempool_tx.commit().unwrap();
 
     let txs = MemPoolDB::get_txs_after(