Merge branch 'develop' of https://github.com/stacks-network/stacks-core into feat/miner-continues-tenure-if-tenure-empty

Author: jferrant

.github/workflows/pr-differences-mutants.yml

@@ -1,14 +1,15 @@
 name: PR Differences Mutants
 
 on:
-  pull_request:
-    types:
-      - opened
-      - reopened
-      - synchronize
-      - ready_for_review
-    paths:
-      - '**.rs'
+  # Disabling PR mutants (issue #5806)
+  # pull_request:
+  #  types:
+  #    - opened
+  #    - reopened
+  #    - synchronize
+  #    - ready_for_review
+  #  paths:
+  #    - '**.rs'
   workflow_dispatch:
 
 concurrency:

CHANGELOG.md

@@ -13,6 +13,8 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ### Fixed
 
+- Error responses to /v2/transactions/fees are once again expressed as JSON ([#4145](https://github.com/stacks-network/stacks-core/issues/4145)).
+
 ## [3.1.0.0.5]
 
 ### Added

stacks-signer/CHANGELOG.md

@@ -7,10 +7,16 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ## [Unreleased]
 
-### Added
+## Added
+
+- Introduced the `reorg_attempts_activity_timeout_ms` configuration option for signers which is used to determine the length of time after the last block of a tenure is confirmed that an incoming miner's attempts to reorg it are considered valid miner activity. 
 
 ### Changed
 
+- Increase default `block_proposal_timeout_ms` from 10 minutes to 4 hours. Until #5729 is implemented, there is no value in rejecting a late block from a miner, since a late block is better than no block at all.
+
+- Signers no longer view any block proposal by a miner in their DB as indicative of valid miner activity.
+
 ## [3.1.0.0.5.0]
 
 ### Added

stacks-signer/src/chainstate.rs

@@ -89,23 +89,25 @@ impl SortitionState {
         if self.miner_status != SortitionMinerStatus::Valid {
             return Ok(false);
         }
-        // if we've already seen a proposed block from this miner. It cannot have timed out.
-        let has_blocks = signer_db.has_proposed_block_in_tenure(&self.consensus_hash)?;
-        if has_blocks {
+        // if we've already signed a block in this tenure, the miner can't have timed out.
+        let has_block = signer_db.has_signed_block_in_tenure(&self.consensus_hash)?;
+        if has_block {
             return Ok(false);
         }
         let Some(received_ts) = signer_db.get_burn_block_receive_time(&self.burn_block_hash)?
         else {
             return Ok(false);
         };
         let received_time = UNIX_EPOCH + Duration::from_secs(received_ts);
-        let Ok(elapsed) = std::time::SystemTime::now().duration_since(received_time) else {
+        let last_activity = signer_db
+            .get_last_activity_time(&self.consensus_hash)?
+            .map(|time| UNIX_EPOCH + Duration::from_secs(time))
+            .unwrap_or(received_time);
+
+        let Ok(elapsed) = std::time::SystemTime::now().duration_since(last_activity) else {
             return Ok(false);
         };
-        if elapsed > timeout {
-            return Ok(true);
-        }
-        Ok(false)
+        Ok(elapsed > timeout)
     }
 }
 
@@ -122,6 +124,9 @@ pub struct ProposalEvalConfig {
     pub tenure_last_block_proposal_timeout: Duration,
     /// How much idle time must pass before allowing a tenure extend
     pub tenure_idle_timeout: Duration,
+    /// Time following the last block of the previous tenure's global acceptance that a signer will consider an attempt by
+    /// the new miner to reorg it as valid towards miner activity
+    pub reorg_attempts_activity_timeout: Duration,
 }
 
 impl From<&SignerConfig> for ProposalEvalConfig {
@@ -131,6 +136,7 @@ impl From<&SignerConfig> for ProposalEvalConfig {
             block_proposal_timeout: value.block_proposal_timeout,
             tenure_last_block_proposal_timeout: value.tenure_last_block_proposal_timeout,
             tenure_idle_timeout: value.tenure_idle_timeout,
+            reorg_attempts_activity_timeout: value.reorg_attempts_activity_timeout,
         }
     }
 }
@@ -545,8 +551,10 @@ impl SortitionsView {
         signer_db: &mut SignerDb,
         client: &StacksClient,
         tenure_last_block_proposal_timeout: Duration,
+        reorg_attempts_activity_timeout: Duration,
     ) -> Result<bool, ClientError> {
         // If the tenure change block confirms the expected parent block, it should confirm at least one more block than the last accepted block in the parent tenure.
+        // NOTE: returns the locally accepted block if it is not timed out, otherwise it will return the last globally accepted block.
         let last_block_info = Self::get_tenure_last_block_info(
             &tenure_change.prev_tenure_consensus_hash,
             signer_db,
@@ -566,6 +574,21 @@ impl SortitionsView {
                     "proposed_chain_length" => block.header.chain_length,
                     "expected_at_least" => info.block.header.chain_length + 1,
                 );
+                if info.signed_group.map_or(true, |signed_time| {
+                    signed_time + reorg_attempts_activity_timeout.as_secs() > get_epoch_time_secs()
+                }) {
+                    // Note if there is no signed_group time, this is a locally accepted block (i.e. tenure_last_block_proposal_timeout has not been exceeded).
+                    // Treat any attempt to reorg a locally accepted block as valid miner activity.
+                    // If the call returns a globally accepted block, check its globally accepted time against a quarter of the block_proposal_timeout
+                    // to give the miner some extra buffer time to wait for its chain tip to advance
+                    // The miner may just be slow, so count this invalid block proposal towards valid miner activity.
+                    if let Err(e) = signer_db.update_last_activity_time(
+                        &tenure_change.tenure_consensus_hash,
+                        get_epoch_time_secs(),
+                    ) {
+                        warn!("Failed to update last activity time: {e}");
+                    }
+                }
                 return Ok(false);
             }
         }
@@ -631,6 +654,7 @@ impl SortitionsView {
             signer_db,
             client,
             self.config.tenure_last_block_proposal_timeout,
+            self.config.reorg_attempts_activity_timeout,
         )?;
         if !confirms_expected_parent {
             return Ok(false);

stacks-signer/src/client/mod.rs

@@ -417,6 +417,7 @@ pub(crate) mod tests {
             block_proposal_validation_timeout: config.block_proposal_validation_timeout,
             tenure_idle_timeout: config.tenure_idle_timeout,
             block_proposal_max_age_secs: config.block_proposal_max_age_secs,
+            reorg_attempts_activity_timeout: config.reorg_attempts_activity_timeout,
         }
     }
 

stacks-signer/src/config.rs

@@ -41,6 +41,7 @@ const DEFAULT_FIRST_PROPOSAL_BURN_BLOCK_TIMING_SECS: u64 = 60;
 const DEFAULT_TENURE_LAST_BLOCK_PROPOSAL_TIMEOUT_SECS: u64 = 30;
 const DEFAULT_DRY_RUN: bool = false;
 const TENURE_IDLE_TIMEOUT_SECS: u64 = 120;
+const DEFAULT_REORG_ATTEMPTS_ACTIVITY_TIMEOUT_MS: u64 = 200_000;
 
 #[derive(thiserror::Error, Debug)]
 /// An error occurred parsing the provided configuration
@@ -163,6 +164,9 @@ pub struct SignerConfig {
     pub tenure_idle_timeout: Duration,
     /// The maximum age of a block proposal in seconds that will be processed by the signer
     pub block_proposal_max_age_secs: u64,
+    /// Time following the last block of the previous tenure's global acceptance that a signer will consider an attempt by
+    /// the new miner to reorg it as valid towards miner activity
+    pub reorg_attempts_activity_timeout: Duration,
     /// The running mode for the signer (dry-run or normal)
     pub signer_mode: SignerConfigMode,
 }
@@ -205,6 +209,9 @@ pub struct GlobalConfig {
     pub tenure_idle_timeout: Duration,
     /// The maximum age of a block proposal that will be processed by the signer
     pub block_proposal_max_age_secs: u64,
+    /// Time following the last block of the previous tenure's global acceptance that a signer will consider an attempt by
+    /// the new miner to reorg it as valid towards miner activity
+    pub reorg_attempts_activity_timeout: Duration,
     /// Is this signer binary going to be running in dry-run mode?
     pub dry_run: bool,
 }
@@ -246,6 +253,9 @@ struct RawConfigFile {
     pub tenure_idle_timeout_secs: Option<u64>,
     /// The maximum age of a block proposal (in secs) that will be processed by the signer.
     pub block_proposal_max_age_secs: Option<u64>,
+    /// Time (in millisecs) following a block's global acceptance that a signer will consider an attempt by a miner
+    /// to reorg the block as valid towards miner activity
+    pub reorg_attempts_activity_timeout_ms: Option<u64>,
     /// Is this signer binary going to be running in dry-run mode?
     pub dry_run: Option<bool>,
 }
@@ -349,6 +359,12 @@ impl TryFrom<RawConfigFile> for GlobalConfig {
             .block_proposal_max_age_secs
             .unwrap_or(DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS);
 
+        let reorg_attempts_activity_timeout = Duration::from_millis(
+            raw_data
+                .reorg_attempts_activity_timeout_ms
+                .unwrap_or(DEFAULT_REORG_ATTEMPTS_ACTIVITY_TIMEOUT_MS),
+        );
+
         let dry_run = raw_data.dry_run.unwrap_or(DEFAULT_DRY_RUN);
 
         Ok(Self {
@@ -368,6 +384,7 @@ impl TryFrom<RawConfigFile> for GlobalConfig {
             block_proposal_validation_timeout,
             tenure_idle_timeout,
             block_proposal_max_age_secs,
+            reorg_attempts_activity_timeout,
             dry_run,
         })
     }

stacks-signer/src/runloop.rs

@@ -315,6 +315,7 @@ impl<Signer: SignerTrait<T>, T: StacksMessageCodec + Clone + Send + Debug> RunLo
             block_proposal_validation_timeout: self.config.block_proposal_validation_timeout,
             tenure_idle_timeout: self.config.tenure_idle_timeout,
             block_proposal_max_age_secs: self.config.block_proposal_max_age_secs,
+            reorg_attempts_activity_timeout: self.config.reorg_attempts_activity_timeout,
         }))
     }
 

stacks-signer/src/signerdb.rs

@@ -480,6 +480,12 @@ CREATE TABLE IF NOT EXISTS block_validations_pending (
     PRIMARY KEY (signer_signature_hash)
 ) STRICT;"#;
 
+static CREATE_TENURE_ACTIVTY_TABLE: &str = r#"
+CREATE TABLE IF NOT EXISTS tenure_activity (
+    consensus_hash TEXT NOT NULL PRIMARY KEY,
+    last_activity_time INTEGER NOT NULL
+) STRICT;"#;
+
 static SCHEMA_1: &[&str] = &[
     DROP_SCHEMA_0,
     CREATE_DB_CONFIG,
@@ -534,9 +540,14 @@ static SCHEMA_6: &[&str] = &[
     "INSERT OR REPLACE INTO db_config (version) VALUES (6);",
 ];
 
+static SCHEMA_7: &[&str] = &[
+    CREATE_TENURE_ACTIVTY_TABLE,
+    "INSERT OR REPLACE INTO db_config (version) VALUES (7);",
+];
+
 impl SignerDb {
     /// The current schema version used in this build of the signer binary.
-    pub const SCHEMA_VERSION: u32 = 6;
+    pub const SCHEMA_VERSION: u32 = 7;
 
     /// Create a new `SignerState` instance.
     /// This will create a new SQLite database at the given path
@@ -650,6 +661,20 @@ impl SignerDb {
         Ok(())
     }
 
+    /// Migrate from schema 6 to schema 7
+    fn schema_7_migration(tx: &Transaction) -> Result<(), DBError> {
+        if Self::get_schema_version(tx)? >= 7 {
+            // no migration necessary
+            return Ok(());
+        }
+
+        for statement in SCHEMA_7.iter() {
+            tx.execute_batch(statement)?;
+        }
+
+        Ok(())
+    }
+
     /// Register custom scalar functions used by the database
     fn register_scalar_functions(&self) -> Result<(), DBError> {
         // Register helper function for determining if a block is a tenure change transaction
@@ -689,7 +714,8 @@ impl SignerDb {
                 3 => Self::schema_4_migration(&sql_tx)?,
                 4 => Self::schema_5_migration(&sql_tx)?,
                 5 => Self::schema_6_migration(&sql_tx)?,
-                6 => break,
+                6 => Self::schema_7_migration(&sql_tx)?,
+                7 => break,
                 x => return Err(DBError::Other(format!(
                     "Database schema is newer than supported by this binary. Expected version = {}, Database version = {x}",
                     Self::SCHEMA_VERSION,
@@ -746,10 +772,10 @@ impl SignerDb {
         try_deserialize(result)
     }
 
-    /// Return whether a block proposal has been stored for a tenure (identified by its consensus hash)
-    /// Does not consider the block's state.
-    pub fn has_proposed_block_in_tenure(&self, tenure: &ConsensusHash) -> Result<bool, DBError> {
-        let query = "SELECT block_info FROM blocks WHERE consensus_hash = ? LIMIT 1";
+    /// Return whether there was signed block in a tenure (identified by its consensus hash)
+    pub fn has_signed_block_in_tenure(&self, tenure: &ConsensusHash) -> Result<bool, DBError> {
+        let query =
+            "SELECT block_info FROM blocks WHERE consensus_hash = ? AND signed_over = 1 LIMIT 1";
         let result: Option<String> = query_row(&self.db, query, [tenure])?;
 
         Ok(result.is_some())
@@ -1112,6 +1138,30 @@ impl SignerDb {
         self.remove_pending_block_validation(&block_info.signer_signature_hash())?;
         Ok(())
     }
+    /// Update the tenure (identified by consensus_hash) last activity timestamp
+    pub fn update_last_activity_time(
+        &mut self,
+        tenure: &ConsensusHash,
+        last_activity_time: u64,
+    ) -> Result<(), DBError> {
+        debug!("Updating last activity for tenure"; "consensus_hash" => %tenure, "last_activity_time" => last_activity_time);
+        self.db.execute("INSERT OR REPLACE INTO tenure_activity (consensus_hash, last_activity_time) VALUES (?1, ?2)", params![tenure, u64_to_sql(last_activity_time)?])?;
+        Ok(())
+    }
+
+    /// Get the last activity timestamp for a tenure (identified by consensus_hash)
+    pub fn get_last_activity_time(&self, tenure: &ConsensusHash) -> Result<Option<u64>, DBError> {
+        let query =
+            "SELECT last_activity_time FROM tenure_activity WHERE consensus_hash = ? LIMIT 1";
+        let Some(last_activity_time_i64) = query_row::<i64, _>(&self.db, query, &[tenure])? else {
+            return Ok(None);
+        };
+        let last_activity_time = u64::try_from(last_activity_time_i64).map_err(|e| {
+            error!("Failed to parse db last_activity_time as u64: {e}");
+            DBError::Corruption
+        })?;
+        Ok(Some(last_activity_time))
+    }
 }
 
 fn try_deserialize<T>(s: Option<String>) -> Result<Option<T>, DBError>
@@ -1903,7 +1953,7 @@ mod tests {
     }
 
     #[test]
-    fn has_proposed_block() {
+    fn has_signed_block() {
         let db_path = tmp_db_path();
         let consensus_hash_1 = ConsensusHash([0x01; 20]);
         let consensus_hash_2 = ConsensusHash([0x02; 20]);
@@ -1913,16 +1963,59 @@ mod tests {
             b.block.header.chain_length = 1;
         });
 
-        assert!(!db.has_proposed_block_in_tenure(&consensus_hash_1).unwrap());
-        assert!(!db.has_proposed_block_in_tenure(&consensus_hash_2).unwrap());
+        assert!(!db.has_signed_block_in_tenure(&consensus_hash_1).unwrap());
+        assert!(!db.has_signed_block_in_tenure(&consensus_hash_2).unwrap());
 
+        block_info.signed_over = true;
         db.insert_block(&block_info).unwrap();
 
+        assert!(db.has_signed_block_in_tenure(&consensus_hash_1).unwrap());
+        assert!(!db.has_signed_block_in_tenure(&consensus_hash_2).unwrap());
+
+        block_info.block.header.consensus_hash = consensus_hash_2;
         block_info.block.header.chain_length = 2;
+        block_info.signed_over = false;
 
         db.insert_block(&block_info).unwrap();
 
-        assert!(db.has_proposed_block_in_tenure(&consensus_hash_1).unwrap());
-        assert!(!db.has_proposed_block_in_tenure(&consensus_hash_2).unwrap());
+        assert!(db.has_signed_block_in_tenure(&consensus_hash_1).unwrap());
+        assert!(!db.has_signed_block_in_tenure(&consensus_hash_2).unwrap());
+
+        block_info.signed_over = true;
+
+        db.insert_block(&block_info).unwrap();
+
+        assert!(db.has_signed_block_in_tenure(&consensus_hash_1).unwrap());
+        assert!(db.has_signed_block_in_tenure(&consensus_hash_2).unwrap());
+    }
+
+    #[test]
+    fn update_last_activity() {
+        let db_path = tmp_db_path();
+        let consensus_hash_1 = ConsensusHash([0x01; 20]);
+        let consensus_hash_2 = ConsensusHash([0x02; 20]);
+        let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
+
+        assert!(db
+            .get_last_activity_time(&consensus_hash_1)
+            .unwrap()
+            .is_none());
+        assert!(db
+            .get_last_activity_time(&consensus_hash_2)
+            .unwrap()
+            .is_none());
+
+        let time = get_epoch_time_secs();
+        db.update_last_activity_time(&consensus_hash_1, time)
+            .unwrap();
+        let retrieved_time = db
+            .get_last_activity_time(&consensus_hash_1)
+            .unwrap()
+            .unwrap();
+        assert_eq!(time, retrieved_time);
+        assert!(db
+            .get_last_activity_time(&consensus_hash_2)
+            .unwrap()
+            .is_none());
     }
 }