Added block_proposal_max_age_secs signer configuration to drop old proposals without processing

Signed-off-by: Jacinta Ferrant <[email protected]>

Author: jferrant

.github/workflows/bitcoin-tests.yml

@@ -127,6 +127,7 @@ jobs:
           - tests::signer::v0::continue_after_fast_block_no_sortition
           - tests::signer::v0::block_validation_response_timeout
           - tests::signer::v0::tenure_extend_after_bad_commit
+          - tests::signer::v0::block_proposal_max_age_rejections
           - tests::nakamoto_integrations::burn_ops_integration_test
           - tests::nakamoto_integrations::check_block_heights
           - tests::nakamoto_integrations::clarity_burn_state

stacks-signer/CHANGELOG.md

@@ -7,6 +7,8 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ## [Unreleased]
 
+- Introduced the `block_proposal_max_age_secs` configuration option for signers, enabling them to automatically ignore block proposals that exceed the specified age in seconds.
+
 ### Added
 
 ### Changed

stacks-signer/src/client/mod.rs

@@ -413,6 +413,7 @@ pub(crate) mod tests {
             block_proposal_timeout: config.block_proposal_timeout,
             tenure_last_block_proposal_timeout: config.tenure_last_block_proposal_timeout,
             block_proposal_validation_timeout: config.block_proposal_validation_timeout,
+            block_proposal_max_age_secs: config.block_proposal_max_age_secs,
         }
     }
 

stacks-signer/src/config.rs

@@ -38,6 +38,7 @@ const BLOCK_PROPOSAL_TIMEOUT_MS: u64 = 600_000;
 const BLOCK_PROPOSAL_VALIDATION_TIMEOUT_MS: u64 = 120_000;
 const DEFAULT_FIRST_PROPOSAL_BURN_BLOCK_TIMING_SECS: u64 = 60;
 const DEFAULT_TENURE_LAST_BLOCK_PROPOSAL_TIMEOUT_SECS: u64 = 30;
+const DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS: u64 = 600;
 
 #[derive(thiserror::Error, Debug)]
 /// An error occurred parsing the provided configuration
@@ -135,6 +136,8 @@ pub struct SignerConfig {
     pub tenure_last_block_proposal_timeout: Duration,
     /// How much time to wait for a block proposal validation response before marking the block invalid
     pub block_proposal_validation_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,
 }
 
 /// The parsed configuration for the signer
@@ -171,6 +174,8 @@ pub struct GlobalConfig {
     /// How long to wait for a response from a block proposal validation response from the node
     /// before marking that block as invalid and rejecting it
     pub block_proposal_validation_timeout: Duration,
+    /// The maximum age of a block proposal that will be processed by the signer
+    pub block_proposal_max_age_secs: u64,
 }
 
 /// Internal struct for loading up the config file
@@ -206,6 +211,8 @@ struct RawConfigFile {
     /// How long to wait (in millisecs) for a response from a block proposal validation response from the node
     /// before marking that block as invalid and rejecting it
     pub block_proposal_validation_timeout_ms: 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>,
 }
 
 impl RawConfigFile {
@@ -297,6 +304,10 @@ impl TryFrom<RawConfigFile> for GlobalConfig {
                 .unwrap_or(BLOCK_PROPOSAL_VALIDATION_TIMEOUT_MS),
         );
 
+        let block_proposal_max_age_secs = raw_data
+            .block_proposal_max_age_secs
+            .unwrap_or(DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS);
+
         Ok(Self {
             node_host: raw_data.node_host,
             endpoint,
@@ -312,6 +323,7 @@ impl TryFrom<RawConfigFile> for GlobalConfig {
             chain_id: raw_data.chain_id,
             tenure_last_block_proposal_timeout,
             block_proposal_validation_timeout,
+            block_proposal_max_age_secs,
         })
     }
 }

stacks-signer/src/runloop.rs

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

stacks-signer/src/v0/signer.rs

@@ -92,6 +92,8 @@ pub struct Signer {
     pub block_proposal_validation_timeout: Duration,
     /// The current submitted block proposal and its submission time
     pub submitted_block_proposal: Option<(BlockProposal, Instant)>,
+    /// Maximum age of a block proposal in seconds before it is dropped without processing
+    pub block_proposal_max_age_secs: u64,
 }
 
 impl std::fmt::Display for Signer {
@@ -284,6 +286,7 @@ impl From<SignerConfig> for Signer {
             proposal_config,
             submitted_block_proposal: None,
             block_proposal_validation_timeout: signer_config.block_proposal_validation_timeout,
+            block_proposal_max_age_secs: signer_config.block_proposal_max_age_secs,
         }
     }
 }
@@ -331,6 +334,23 @@ impl Signer {
             return;
         }
 
+        if block_proposal
+            .block
+            .header
+            .timestamp
+            .saturating_add(self.block_proposal_max_age_secs)
+            < get_epoch_time_secs()
+        {
+            // Block is too old. Drop it with a warning. Don't even bother broadcasting to the node.
+            warn!("{self}: Received a block proposal that is more than {} secs old. Ignoring...", self.block_proposal_max_age_secs;
+                "block_id" => %block_proposal.block.block_id(),
+                "block_height" => block_proposal.block.header.chain_length,
+                "burn_height" => block_proposal.burn_height,
+                "timestamp" => block_proposal.block.header.timestamp,
+            );
+            return;
+        }
+
         // TODO: should add a check to ignore an old burn block height if we know its outdated. Would require us to store the burn block height we last saw on the side.
         //  the signer needs to be able to determine whether or not the block they're about to sign would conflict with an already-signed Stacks block
         let signer_signature_hash = block_proposal.block.header.signer_signature_hash();

testnet/stacks-node/src/tests/signer/v0.rs

@@ -43,6 +43,7 @@ use stacks::net::api::postblock_proposal::{ValidateRejectCode, TEST_VALIDATE_STA
 use stacks::net::relay::fault_injection::set_ignore_block;
 use stacks::types::chainstate::{StacksAddress, StacksBlockId, StacksPrivateKey, StacksPublicKey};
 use stacks::types::PublicKey;
+use stacks::util::get_epoch_time_secs;
 use stacks::util::hash::{hex_bytes, Hash160, MerkleHashFunc};
 use stacks::util::secp256k1::{Secp256k1PrivateKey, Secp256k1PublicKey};
 use stacks::util_lib::boot::boot_code_id;
@@ -811,14 +812,8 @@ fn reloads_signer_set_in() {
     let sender_addr = tests::to_addr(&sender_sk);
     let send_amt = 100;
     let send_fee = 180;
-    let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new_with_config_modifications(
-        num_signers,
-        vec![(sender_addr, send_amt + send_fee)],
-        |_config| {},
-        |_| {},
-        None,
-        None,
-    );
+    let mut signer_test: SignerTest<SpawnedSigner> =
+        SignerTest::new(num_signers, vec![(sender_addr, send_amt + send_fee)]);
 
     setup_epoch_3_reward_set(
         &signer_test.running_nodes.conf,
@@ -8574,3 +8569,115 @@ fn tenure_extend_after_2_bad_commits() {
     run_loop_2_thread.join().unwrap();
     signer_test.shutdown();
 }
+
+#[test]
+#[ignore]
+/// Test the block_proposal_max_age_secs signer configuration option. It should reject blocks that are
+/// invalid but within the max age window, otherwise it should simply drop the block without further processing.
+///
+/// Test Setup:
+/// The test spins up five stacks signers, one miner Nakamoto node, and a corresponding bitcoind.
+///
+/// Test Execution:
+/// The stacks node is advanced to epoch 3.0 reward set calculation to ensure the signer set is determined.
+/// An invalid block proposal with a recent timestamp is forcibly written to the miner's slot to simulate the miner proposing a block.
+/// The signers process the invalid block and broadcast a block response rejection to the respective .signers-XXX-YYY contract.
+/// A second block proposal with an outdated timestamp is then submitted to the miner's slot to simulate the miner proposing a very old block.
+/// The test confirms no further block rejection response is submitted to the .signers-XXX-YYY contract.
+///
+/// Test Assertion:
+/// - Each signer successfully rejects the recent invalid block proposal.
+/// - No signer submits a block proposal response for the outdated block proposal.
+/// - The stacks tip does not advance
+fn block_proposal_max_age_rejections() {
+    if env::var("BITCOIND_TEST") != Ok("1".into()) {
+        return;
+    }
+
+    tracing_subscriber::registry()
+        .with(fmt::layer())
+        .with(EnvFilter::from_default_env())
+        .init();
+
+    info!("------------------------- Test Setup -------------------------");
+    let num_signers = 5;
+    let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new_with_config_modifications(
+        num_signers,
+        vec![],
+        |config| {
+            config.block_proposal_max_age_secs = 30;
+        },
+        |_| {},
+        None,
+        None,
+    );
+    signer_test.boot_to_epoch_3();
+    let short_timeout = Duration::from_secs(30);
+
+    // Make sure no other block approvals are in the system.
+    test_observer::clear();
+    info!("------------------------- Send Block Proposal To Signers -------------------------");
+    let info_before = get_chain_info(&signer_test.running_nodes.conf);
+    let mut block = NakamotoBlock {
+        header: NakamotoBlockHeader::empty(),
+        txs: vec![],
+    };
+    // First propose a stale block that is older than the block_proposal_max_age_secs
+    block.header.timestamp = get_epoch_time_secs().saturating_sub(
+        signer_test.signer_configs[0]
+            .block_proposal_max_age_secs
+            .saturating_add(1),
+    );
+    let _block_signer_signature_hash_1 = block.header.signer_signature_hash();
+    signer_test.propose_block(block.clone(), short_timeout);
+
+    // Next propose a recent invalid block
+    block.header.timestamp = get_epoch_time_secs();
+    let block_signer_signature_hash_2 = block.header.signer_signature_hash();
+    signer_test.propose_block(block, short_timeout);
+
+    info!("------------------------- Test Block Proposal Rejected -------------------------");
+    // Verify the signers rejected only the SECOND block proposal. The first was not even processed.
+    wait_for(30, || {
+        let rejections: Vec<_> = test_observer::get_stackerdb_chunks()
+            .into_iter()
+            .flat_map(|chunk| chunk.modified_slots)
+            .map(|chunk| {
+                let Ok(message) = SignerMessage::consensus_deserialize(&mut chunk.data.as_slice())
+                else {
+                    return None;
+                };
+                assert!(matches!(
+                    message,
+                    SignerMessage::BlockResponse(BlockResponse::Rejected(_))
+                ));
+                let SignerMessage::BlockResponse(BlockResponse::Rejected(BlockRejection {
+                    reason_code,
+                    signer_signature_hash,
+                    signature,
+                    ..
+                })) = message
+                else {
+                    panic!("Received an unexpected block approval from the signer");
+                };
+                assert_eq!(
+                    signer_signature_hash, block_signer_signature_hash_2,
+                    "Received a rejection for an unexpected block: {signer_signature_hash}"
+                );
+                assert!(
+                    matches!(reason_code, RejectCode::SortitionViewMismatch),
+                    "Received a rejection for an unexpected reason: {reason_code}"
+                );
+                Some(signature)
+            })
+            .collect();
+        Ok(rejections.len() == num_signers)
+    })
+    .expect("Timed out waiting for block rejections");
+
+    info!("------------------------- Test Peer Info-------------------------");
+    assert_eq!(info_before, get_chain_info(&signer_test.running_nodes.conf));
+
+    info!("------------------------- Test Shutdown-------------------------");
+    signer_test.shutdown();
+}