fix: logic around whether to clear replay set

Author: hstove

stacks-signer/src/signerdb.rs

@@ -605,6 +605,7 @@ static ADD_BLOCK_VALIDATED_BY_REPLAY_TXS_TABLE: &str = r#"
 CREATE TABLE IF NOT EXISTS block_validated_by_replay_txs (
     signer_signature_hash TEXT NOT NULL,
     replay_tx_hash TEXT NOT NULL,
+    replay_tx_exhausted INTEGER NOT NULL,
     PRIMARY KEY (signer_signature_hash, replay_tx_hash)
 ) STRICT;"#;
 
@@ -1549,10 +1550,15 @@ impl SignerDb {
         &self,
         signer_signature_hash: &Sha512Trunc256Sum,
         replay_tx_hash: u64,
+        replay_tx_exhausted: bool,
     ) -> Result<(), DBError> {
         self.db.execute(
-            "INSERT INTO block_validated_by_replay_txs (signer_signature_hash, replay_tx_hash) VALUES (?1, ?2)",
-            params![signer_signature_hash.to_string(), format!("{replay_tx_hash}")],
+            "INSERT INTO block_validated_by_replay_txs (signer_signature_hash, replay_tx_hash, replay_tx_exhausted) VALUES (?1, ?2, ?3)",
+            params![
+                signer_signature_hash.to_string(),
+                format!("{replay_tx_hash}"),
+                replay_tx_exhausted
+            ],
         )?;
         Ok(())
     }
@@ -1562,14 +1568,13 @@ impl SignerDb {
         &self,
         signer_signature_hash: &Sha512Trunc256Sum,
         replay_tx_hash: u64,
-    ) -> Result<bool, DBError> {
-        let query = "SELECT replay_tx_hash FROM block_validated_by_replay_txs WHERE signer_signature_hash = ? AND replay_tx_hash = ?";
+    ) -> Result<Option<BlockValidatedByReplaySet>, DBError> {
+        let query = "SELECT replay_tx_hash, replay_tx_exhausted FROM block_validated_by_replay_txs WHERE signer_signature_hash = ? AND replay_tx_hash = ?";
         let args = params![
             signer_signature_hash.to_string(),
             format!("{replay_tx_hash}")
         ];
-        let replay_tx_hash_opt: Option<String> = query_row(&self.db, query, args)?;
-        Ok(replay_tx_hash_opt.is_some())
+        query_row(&self.db, query, args)
     }
 }
 
@@ -1604,6 +1609,25 @@ impl FromRow<PendingBlockValidation> for PendingBlockValidation {
     }
 }
 
+/// A struct used to represent whether a block was validated by a transaction replay set
+pub struct BlockValidatedByReplaySet {
+    /// The hash of the transaction replay set that validated the block
+    pub replay_tx_hash: String,
+    /// Whether the transaction replay set exhausted the set of transactions
+    pub replay_tx_exhausted: bool,
+}
+
+impl FromRow<BlockValidatedByReplaySet> for BlockValidatedByReplaySet {
+    fn from_row(row: &rusqlite::Row) -> Result<Self, DBError> {
+        let replay_tx_hash = row.get_unwrap(0);
+        let replay_tx_exhausted = row.get_unwrap(1);
+        Ok(BlockValidatedByReplaySet {
+            replay_tx_hash,
+            replay_tx_exhausted,
+        })
+    }
+}
+
 #[cfg(any(test, feature = "testing"))]
 impl SignerDb {
     /// For tests, fetch all pending block validations
@@ -2851,13 +2875,37 @@ pub mod tests {
 
         let signer_signature_hash = Sha512Trunc256Sum([0; 32]);
         let replay_tx_hash = 15559610262907183370_u64;
+        let replay_tx_exhausted = true;
+
+        db.insert_block_validated_by_replay_tx(
+            &signer_signature_hash,
+            replay_tx_hash,
+            replay_tx_exhausted,
+        )
+        .expect("Failed to insert block validated by replay tx");
 
-        db.insert_block_validated_by_replay_tx(&signer_signature_hash, replay_tx_hash)
-            .expect("Failed to insert block validated by replay tx");
+        let result = db
+            .get_was_block_validated_by_replay_tx(&signer_signature_hash, replay_tx_hash)
+            .expect("Failed to get block validated by replay tx")
+            .expect("Expected block validation result to be stored");
+        assert_eq!(result.replay_tx_hash, format!("{replay_tx_hash}"));
+        assert!(result.replay_tx_exhausted);
+
+        let replay_tx_hash = 15559610262907183369_u64;
+        let replay_tx_exhausted = false;
+
+        db.insert_block_validated_by_replay_tx(
+            &signer_signature_hash,
+            replay_tx_hash,
+            replay_tx_exhausted,
+        )
+        .expect("Failed to insert block validated by replay tx");
 
         let result = db
             .get_was_block_validated_by_replay_tx(&signer_signature_hash, replay_tx_hash)
-            .expect("Failed to get block validated by replay tx");
-        assert!(result);
+            .expect("Failed to get block validated by replay tx")
+            .expect("Expected block validation result to be stored");
+        assert_eq!(result.replay_tx_hash, format!("{replay_tx_hash}"));
+        assert!(!result.replay_tx_exhausted);
     }
 }

stacks-signer/src/v0/signer.rs

@@ -20,7 +20,6 @@ use std::sync::LazyLock;
 use std::time::{Duration, Instant, SystemTime};
 
 use blockstack_lib::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader};
-use blockstack_lib::chainstate::stacks::TransactionPayload;
 use blockstack_lib::net::api::postblock_proposal::{
     BlockValidateOk, BlockValidateReject, BlockValidateResponse, ValidateRejectCode,
     TOO_MANY_REQUESTS_STATUS,
@@ -1043,7 +1042,11 @@ impl Signer {
                 "replay_tx_hash" => replay_tx_hash
             );
             self.signer_db
-                .insert_block_validated_by_replay_tx(&signer_signature_hash, replay_tx_hash)
+                .insert_block_validated_by_replay_tx(
+                    &signer_signature_hash,
+                    replay_tx_hash,
+                    block_validate_ok.replay_tx_exhausted,
+                )
                 .unwrap_or_else(|e| {
                     warn!("{self}: Failed to insert block validated by replay tx: {e:?}")
                 });
@@ -1614,21 +1617,15 @@ impl Signer {
                 debug!("{self}: Cannot confirm that we have processed parent, but we've waited proposal_wait_for_parent_time, will submit proposal");
             }
         }
-        let is_block_found = block.txs.iter().all(|tx| {
-            matches!(
-                tx.payload,
-                TransactionPayload::Coinbase(..) | TransactionPayload::TenureChange(..)
-            )
-        });
         match stacks_client.submit_block_for_validation(
             block.clone(),
-            if is_block_found || !self.validate_with_replay_tx {
-                None
-            } else {
+            if self.validate_with_replay_tx {
                 self.global_state_evaluator
                     .get_global_tx_replay_set()
                     .unwrap_or_default()
                     .into_optional()
+            } else {
+                None
             },
         ) {
             Ok(_) => {

stacks-signer/src/v0/signer_state.rs

@@ -20,9 +20,7 @@ use std::time::{Duration, UNIX_EPOCH};
 
 use blockstack_lib::chainstate::burn::ConsensusHashExtensions;
 use blockstack_lib::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader};
-use blockstack_lib::chainstate::stacks::{
-    StacksTransaction, TenureChangeCause, TenureChangePayload, TransactionPayload,
-};
+use blockstack_lib::chainstate::stacks::{StacksTransaction, TransactionPayload};
 use blockstack_lib::net::api::postblock_proposal::NakamotoBlockProposal;
 use clarity::types::chainstate::StacksAddress;
 #[cfg(any(test, feature = "testing"))]
@@ -48,7 +46,7 @@ use crate::chainstate::{
     ProposalEvalConfig, SignerChainstateError, SortitionState, SortitionsView,
 };
 use crate::client::{ClientError, CurrentAndLastSortition, StackerDB, StacksClient};
-use crate::signerdb::SignerDb;
+use crate::signerdb::{BlockValidatedByReplaySet, SignerDb};
 
 /// This is the latest supported protocol version for this signer binary
 pub static SUPPORTED_SIGNER_PROTOCOL_VERSION: u64 = 1;
@@ -389,32 +387,24 @@ impl LocalStateMachine {
             &prior_state_machine.tx_replay_set.into_optional(),
         ) {
             match db.get_was_block_validated_by_replay_tx(signer_signature_hash, replay_set_hash) {
-                Ok(true) => {
-                    // This block was validated by our current state machine's replay set,
-                    // and the block exhausted the replay set. Therefore, clear the tx replay set.
-                    info!("Signer State: Incoming Stacks block exhausted the replay set, clearing the tx replay set";
-                        "signer_signature_hash" => %signer_signature_hash,
-                    );
-                    prior_state_machine.tx_replay_set = ReplayTransactionSet::none();
+                Ok(Some(BlockValidatedByReplaySet {
+                    replay_tx_exhausted,
+                    ..
+                })) => {
+                    if replay_tx_exhausted {
+                        // This block was validated by our current state machine's replay set,
+                        // and the block exhausted the replay set. Therefore, clear the tx replay set.
+                        info!("Signer State: Incoming Stacks block exhausted the replay set, clearing the tx replay set";
+                            "signer_signature_hash" => %signer_signature_hash,
+                        );
+                        prior_state_machine.tx_replay_set = ReplayTransactionSet::none();
+                    }
                 }
-                Ok(false) => {
-                    info!("Signer state: got a new block during replay that wasn't validated by our replay set.";
+                Ok(None) => {
+                    info!("Signer state: got a new block during replay that wasn't validated by our replay set. Clearing the local replay set.";
                         "txs" => ?txs,
                     );
-                    // If any of the transactions aren't TenureChange/Coinbase, we should
-                    // capitulate and clear the tx replay set.
-                    if txs.iter().any(|tx| {
-                        !matches!(
-                            tx.payload,
-                            TransactionPayload::TenureChange(TenureChangePayload {
-                                cause: TenureChangeCause::BlockFound,
-                                ..
-                            }) | TransactionPayload::Coinbase(..)
-                        )
-                    }) {
-                        info!("Signer state: clearing the tx replay set due to unrecognized transactions");
-                        prior_state_machine.tx_replay_set = ReplayTransactionSet::none();
-                    }
+                    prior_state_machine.tx_replay_set = ReplayTransactionSet::none();
                 }
                 Err(e) => {
                     warn!("Failed to check if block was validated by replay tx";

stackslib/src/net/api/postblock_proposal.rs

@@ -176,9 +176,11 @@ pub struct BlockValidateOk {
     pub size: u64,
     pub validation_time_ms: u64,
     /// If a block was validated by a transaction replay set,
-    /// and the block exhausted the set of transactions,
     /// then this return Some with the hash of the replay set.
     pub replay_tx_hash: Option<u64>,
+    /// If a block was validated by a transaction replay set,
+    /// then this is true if this block exhausted the set of transactions.
+    pub replay_tx_exhausted: bool,
 }
 
 /// This enum is used for serializing the response to block
@@ -582,10 +584,7 @@ impl NakamotoBlockProposal {
                 loop {
                     if matches!(
                         tx.payload,
-                        TransactionPayload::TenureChange(TenureChangePayload {
-                            cause: TenureChangeCause::Extended,
-                            ..
-                        })
+                        TransactionPayload::TenureChange(..) | TransactionPayload::Coinbase(..)
                     ) {
                         // Allow this to happen, tenure extend checks happen elsewhere.
                         break;
@@ -759,16 +758,15 @@ impl NakamotoBlockProposal {
             })
         );
 
+        let replay_tx_hash = Self::tx_replay_hash(&self.replay_txs);
+
         Ok(BlockValidateOk {
             signer_signature_hash: block.header.signer_signature_hash(),
             cost,
             size,
             validation_time_ms,
-            replay_tx_hash: if replay_tx_exhausted {
-                Self::tx_replay_hash(&self.replay_txs)
-            } else {
-                None
-            },
+            replay_tx_hash,
+            replay_tx_exhausted,
         })
     }
 

stackslib/src/net/api/tests/postblock_proposal.rs

@@ -442,6 +442,7 @@ fn test_try_make_response() {
             size,
             validation_time_ms,
             replay_tx_hash,
+            replay_tx_exhausted,
         }) => {
             assert_eq!(
                 signer_signature_hash,
@@ -451,6 +452,7 @@ fn test_try_make_response() {
             assert_eq!(size, 180);
             assert!(validation_time_ms > 0 && validation_time_ms < 60000);
             assert!(replay_tx_hash.is_none());
+            assert!(!replay_tx_exhausted);
         }
         _ => panic!("expected ok"),
     }
@@ -745,6 +747,7 @@ fn replay_validation_test_transaction_unmineable_match_2() {
             let replay_hash = hasher.finish();
 
             assert_eq!(block_validate_ok.replay_tx_hash, Some(replay_hash));
+            assert!(block_validate_ok.replay_tx_exhausted);
         }
         Err(rejection) => {
             panic!("Expected validation to be OK, but got {:?}", rejection);
@@ -1032,7 +1035,12 @@ fn replay_validation_test_budget_exhausted() {
 
     match result {
         Ok(block_validate_ok) => {
-            assert_eq!(block_validate_ok.replay_tx_hash, None);
+            let mut hasher = DefaultHasher::new();
+            replay_set.hash(&mut hasher);
+            let replay_hash = hasher.finish();
+
+            assert_eq!(block_validate_ok.replay_tx_hash, Some(replay_hash));
+            assert!(!block_validate_ok.replay_tx_exhausted);
         }
         Err(rejection) => {
             panic!(