Merge pull request #5552 from stacks-network/feat/ignore-old-proposals-node

[node] Add "connections.block_proposal_max_age_secs" and ignore block proposals more than the configured amount

Author: jferrant

CHANGELOG.md

@@ -10,8 +10,10 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 ### Added
 
 - Add `tenure_timeout_secs` to the miner for determining when a time-based tenure extend should be attempted.
+- Added configuration option `block_proposal_max_age_secs` under `[connection_options]` to prevent processing stale block proposals
 
 ### Changed
+- The RPC endpoint `/v3/block_proposal` no longer will evaluate block proposals more than `block_proposal_max_age_secs` old
 
 - When a transaction is dropped due to replace-by-fee, the `/drop_mempool_tx` event observer payload now includes `new_txid`, which is the transaction that replaced this dropped transaction. When a transaction is dropped for other reasons, `new_txid` is `null`. [#5381](https://github.com/stacks-network/stacks-core/pull/5381)
 - Nodes will assume that all PoX anchor blocks exist by default, and stall initial block download indefinitely to await their arrival (#5502)

stacks-signer/src/config.rs

@@ -21,6 +21,7 @@ use std::path::PathBuf;
 use std::time::Duration;
 
 use blockstack_lib::chainstate::stacks::TransactionVersion;
+use blockstack_lib::net::connection::DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS;
 use clarity::util::hash::to_hex;
 use libsigner::SignerEntries;
 use serde::Deserialize;
@@ -39,7 +40,6 @@ 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 TENURE_IDLE_TIMEOUT_SECS: u64 = 300;
-const DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS: u64 = 600;
 
 #[derive(thiserror::Error, Debug)]
 /// An error occurred parsing the provided configuration

stackslib/src/config/mod.rs

@@ -59,7 +59,7 @@ use crate::cost_estimates::fee_scalar::ScalarFeeRateEstimator;
 use crate::cost_estimates::metrics::{CostMetric, ProportionalDotProduct, UnitMetric};
 use crate::cost_estimates::{CostEstimator, FeeEstimator, PessimisticEstimator, UnitEstimator};
 use crate::net::atlas::AtlasConfig;
-use crate::net::connection::ConnectionOptions;
+use crate::net::connection::{ConnectionOptions, DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS};
 use crate::net::{Neighbor, NeighborAddress, NeighborKey};
 use crate::types::chainstate::BurnchainHeaderHash;
 use crate::types::EpochList;
@@ -2241,6 +2241,7 @@ pub struct ConnectionOptionsFile {
     pub antientropy_retry: Option<u64>,
     pub reject_blocks_pushed: Option<bool>,
     pub stackerdb_hint_replicas: Option<String>,
+    pub block_proposal_max_age_secs: Option<u64>,
 }
 
 impl ConnectionOptionsFile {
@@ -2389,6 +2390,9 @@ impl ConnectionOptionsFile {
                 .transpose()?
                 .map(HashMap::from_iter)
                 .unwrap_or(default.stackerdb_hint_replicas),
+            block_proposal_max_age_secs: self
+                .block_proposal_max_age_secs
+                .unwrap_or(DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS),
             ..default
         })
     }

stackslib/src/net/api/postblock_proposal.rs

@@ -747,6 +747,20 @@ impl RPCRequestHandler for RPCBlockProposalRequestHandler {
                     NetError::SendError("Proposal currently being evaluated".into()),
                 ));
             }
+
+            if block_proposal
+                .block
+                .header
+                .timestamp
+                .saturating_add(network.get_connection_opts().block_proposal_max_age_secs)
+                < get_epoch_time_secs()
+            {
+                return Err((
+                    422,
+                    NetError::SendError("Block proposal is too old to process.".into()),
+                ));
+            }
+
             let (chainstate, _) = chainstate.reopen().map_err(|e| (400, NetError::from(e)))?;
             let sortdb = sortdb.reopen().map_err(|e| (400, NetError::from(e)))?;
             let receiver = rpc_args

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

@@ -340,9 +340,9 @@ fn test_try_make_response() {
     request.add_header("authorization".into(), "password".into());
     requests.push(request);
 
-    // Set the timestamp to a value in the past
+    // Set the timestamp to a value in the past (but NOT BEFORE timeout)
     let mut early_time_block = good_block.clone();
-    early_time_block.header.timestamp -= 10000;
+    early_time_block.header.timestamp -= 400;
     rpc_test
         .peer_1
         .miner
@@ -388,16 +388,42 @@ fn test_try_make_response() {
     request.add_header("authorization".into(), "password".into());
     requests.push(request);
 
+    // Set the timestamp to a value in the past (BEFORE the timeout)
+    let mut stale_block = good_block.clone();
+    stale_block.header.timestamp -= 10000;
+    rpc_test.peer_1.miner.sign_nakamoto_block(&mut stale_block);
+
+    // post the invalid block proposal
+    let proposal = NakamotoBlockProposal {
+        block: stale_block,
+        chain_id: 0x80000000,
+    };
+
+    let mut request = StacksHttpRequest::new_for_peer(
+        rpc_test.peer_1.to_peer_host(),
+        "POST".into(),
+        "/v3/block_proposal".into(),
+        HttpRequestContents::new().payload_json(serde_json::to_value(proposal).unwrap()),
+    )
+    .expect("failed to construct request");
+    request.add_header("authorization".into(), "password".into());
+    requests.push(request);
+
     // execute the requests
     let observer = ProposalTestObserver::new();
     let proposal_observer = Arc::clone(&observer.proposal_observer);
 
     info!("Run requests with observer");
-    let mut responses = rpc_test.run_with_observer(requests, Some(&observer));
+    let responses = rpc_test.run_with_observer(requests, Some(&observer));
 
-    let response = responses.remove(0);
+    for response in responses.iter().take(3) {
+        assert_eq!(response.preamble().status_code, 202);
+    }
+    let response = &responses[3];
+    assert_eq!(response.preamble().status_code, 422);
 
-    // Wait for the results of all 3 requests
+    // Wait for the results of all 3 PROCESSED requests
+    let start = std::time::Instant::now();
     loop {
         info!("Wait for results to be non-empty");
         if proposal_observer
@@ -413,6 +439,10 @@ fn test_try_make_response() {
         } else {
             break;
         }
+        assert!(
+            start.elapsed().as_secs() < 60,
+            "Timed out waiting for results"
+        );
     }
 
     let observer = proposal_observer.lock().unwrap();

stackslib/src/net/connection.rs

@@ -48,6 +48,9 @@ use crate::net::{
     StacksHttp, StacksP2P,
 };
 
+/// The default maximum age in seconds of a block that can be validated by the block proposal endpoint
+pub const DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS: u64 = 600;
+
 /// Receiver notification handle.
 /// When a message with the expected `seq` value arrives, send it to an expected receiver (possibly
 /// in another thread) via the given `receiver_input` channel.
@@ -434,6 +437,8 @@ pub struct ConnectionOptions {
     pub nakamoto_unconfirmed_downloader_interval_ms: u128,
     /// The authorization token to enable privileged RPC endpoints
     pub auth_token: Option<String>,
+    /// The maximum age in seconds of a block that can be validated by the block proposal endpoint
+    pub block_proposal_max_age_secs: u64,
     /// StackerDB replicas to talk to for a particular smart contract
     pub stackerdb_hint_replicas: HashMap<QualifiedContractIdentifier, Vec<NeighborAddress>>,
 
@@ -568,6 +573,7 @@ impl std::default::Default for ConnectionOptions {
             nakamoto_inv_sync_burst_interval_ms: 1_000, // wait 1 second after a sortition before running inventory sync
             nakamoto_unconfirmed_downloader_interval_ms: 5_000, // run unconfirmed downloader once every 5 seconds
             auth_token: None,
+            block_proposal_max_age_secs: DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS,
             stackerdb_hint_replicas: HashMap::new(),
 
             // no faults on by default

testnet/stacks-node/src/tests/nakamoto_integrations.rs

@@ -2873,6 +2873,7 @@ fn block_proposal_api_endpoint() {
     const HTTP_ACCEPTED: u16 = 202;
     const HTTP_TOO_MANY: u16 = 429;
     const HTTP_NOT_AUTHORIZED: u16 = 401;
+    const HTTP_UNPROCESSABLE: u16 = 422;
     let test_cases = [
         (
             "Valid Nakamoto block proposal",
@@ -2922,6 +2923,16 @@ fn block_proposal_api_endpoint() {
             Some(Err(ValidateRejectCode::ChainstateError)),
         ),
         ("Not authorized", sign(&proposal), HTTP_NOT_AUTHORIZED, None),
+        (
+            "Unprocessable entity",
+            {
+                let mut p = proposal.clone();
+                p.block.header.timestamp = 0;
+                sign(&p)
+            },
+            HTTP_UNPROCESSABLE,
+            None,
+        ),
     ];
 
     // Build HTTP client
@@ -6843,6 +6854,7 @@ fn continue_tenure_extend() {
     let prom_bind = "127.0.0.1:6000".to_string();
     naka_conf.node.prometheus_bind = Some(prom_bind.clone());
     naka_conf.miner.wait_on_interim_blocks = Duration::from_secs(1);
+    naka_conf.connection_options.block_proposal_max_age_secs = u64::MAX;
     let http_origin = naka_conf.node.data_url.clone();
     let sender_sk = Secp256k1PrivateKey::new();
     // setup sender + recipient for a test stx transfer

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

@@ -2956,7 +2956,10 @@ fn idle_tenure_extend_active_mining() {
         |config| {
             config.tenure_idle_timeout = idle_timeout;
         },
-        |_| {},
+        |config| {
+            // accept all proposals in the node
+            config.connection_options.block_proposal_max_age_secs = u64::MAX;
+        },
         None,
         None,
     );
@@ -2977,7 +2980,7 @@ fn idle_tenure_extend_active_mining() {
     // Add a delay to the block validation process
     TEST_VALIDATE_DELAY_DURATION_SECS.lock().unwrap().replace(3);
 
-    signer_test.mine_nakamoto_block(Duration::from_secs(30), true);
+    signer_test.mine_nakamoto_block(Duration::from_secs(60), true);
 
     info!("---- Getting current idle timeout ----");
 
@@ -6130,9 +6133,16 @@ fn miner_recovers_when_broadcast_block_delay_across_tenures_occurs() {
     let send_fee = 180;
     let nmb_txs = 3;
     let recipient = PrincipalData::from(StacksAddress::burn_address(false));
-    let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new(
+    let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new_with_config_modifications(
         num_signers,
         vec![(sender_addr, (send_amt + send_fee) * nmb_txs)],
+        |_config| {},
+        |config| {
+            // Accept all block proposals
+            config.connection_options.block_proposal_max_age_secs = u64::MAX;
+        },
+        None,
+        None,
     );
     let http_origin = format!("http://{}", &signer_test.running_nodes.conf.node.rpc_bind);
     signer_test.boot_to_epoch_3();