Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
## [Unreleased]

### Added
- Added configuration option `connections.block_proposal_max_age_secs` 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

## [3.1.0.0.1]

Expand Down
2 changes: 1 addition & 1 deletion stacks-signer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
6 changes: 5 additions & 1 deletion stackslib/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -2240,6 +2240,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 {
Expand Down Expand Up @@ -2388,6 +2389,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
})
}
Expand Down
14 changes: 14 additions & 0 deletions stackslib/src/net/api/postblock_proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,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
Expand Down
40 changes: 35 additions & 5 deletions stackslib/src/net/api/tests/postblock_proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,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
Expand Down Expand Up @@ -382,16 +382,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
Expand All @@ -407,6 +433,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();
Expand Down
6 changes: 6 additions & 0 deletions stackslib/src/net/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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>>,

Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions testnet/stacks-node/src/tests/nakamoto_integrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2875,6 +2875,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",
Expand Down Expand Up @@ -2924,6 +2925,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
Expand Down
Loading