Skip to content

Commit e5d3be3

Browse files
authored
Merge branch 'develop' into fix/remove-testing-feature
2 parents 6533115 + cbc9406 commit e5d3be3

File tree

40 files changed

+265
-279
lines changed

40 files changed

+265
-279
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
1010
### Added
1111

1212
- Add `tenure_timeout_secs` to the miner for determining when a time-based tenure extend should be attempted.
13+
- Added configuration option `block_proposal_max_age_secs` under `[connection_options]` to prevent processing stale block proposals
1314

1415
### Changed
16+
- The RPC endpoint `/v3/block_proposal` no longer will evaluate block proposals more than `block_proposal_max_age_secs` old
1517

1618
- 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)
1719
- 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/chainstate.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ impl SortitionsView {
539539
///
540540
/// The rationale here is that the signer DB can be out-of-sync with the node. For example,
541541
/// the signer may have been added to an already-running node.
542-
fn check_tenure_change_confirms_parent(
542+
pub fn check_tenure_change_confirms_parent(
543543
tenure_change: &TenureChangePayload,
544544
block: &NakamotoBlock,
545545
signer_db: &mut SignerDb,

stacks-signer/src/config.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use std::path::PathBuf;
2121
use std::time::Duration;
2222

2323
use blockstack_lib::chainstate::stacks::TransactionVersion;
24+
use blockstack_lib::net::connection::DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS;
2425
use clarity::util::hash::to_hex;
2526
use libsigner::SignerEntries;
2627
use serde::Deserialize;
@@ -39,7 +40,6 @@ const BLOCK_PROPOSAL_VALIDATION_TIMEOUT_MS: u64 = 120_000;
3940
const DEFAULT_FIRST_PROPOSAL_BURN_BLOCK_TIMING_SECS: u64 = 60;
4041
const DEFAULT_TENURE_LAST_BLOCK_PROPOSAL_TIMEOUT_SECS: u64 = 30;
4142
const TENURE_IDLE_TIMEOUT_SECS: u64 = 300;
42-
const DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS: u64 = 600;
4343

4444
#[derive(thiserror::Error, Debug)]
4545
/// An error occurred parsing the provided configuration

stacks-signer/src/signerdb.rs

Lines changed: 0 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -733,18 +733,6 @@ impl SignerDb {
733733
try_deserialize(result)
734734
}
735735

736-
/// Return the last accepted block the signer (highest stacks height). It will tie break a match based on which was more recently signed.
737-
pub fn get_signer_last_accepted_block(&self) -> Result<Option<BlockInfo>, DBError> {
738-
let query = "SELECT block_info FROM blocks WHERE state IN (?1, ?2) ORDER BY stacks_height DESC, signed_group DESC, signed_self DESC LIMIT 1";
739-
let args = params![
740-
&BlockState::GloballyAccepted.to_string(),
741-
&BlockState::LocallyAccepted.to_string()
742-
];
743-
let result: Option<String> = query_row(&self.db, query, args)?;
744-
745-
try_deserialize(result)
746-
}
747-
748736
/// Return the last accepted block in a tenure (identified by its consensus hash).
749737
pub fn get_last_accepted_block(
750738
&self,
@@ -1769,69 +1757,4 @@ mod tests {
17691757
< block_infos[0].proposed_time
17701758
);
17711759
}
1772-
1773-
#[test]
1774-
fn signer_last_accepted_block() {
1775-
let db_path = tmp_db_path();
1776-
let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
1777-
1778-
let (mut block_info_1, _block_proposal_1) = create_block_override(|b| {
1779-
b.block.header.miner_signature = MessageSignature([0x01; 65]);
1780-
b.block.header.chain_length = 1;
1781-
b.burn_height = 1;
1782-
});
1783-
1784-
let (mut block_info_2, _block_proposal_2) = create_block_override(|b| {
1785-
b.block.header.miner_signature = MessageSignature([0x02; 65]);
1786-
b.block.header.chain_length = 2;
1787-
b.burn_height = 1;
1788-
});
1789-
1790-
let (mut block_info_3, _block_proposal_3) = create_block_override(|b| {
1791-
b.block.header.miner_signature = MessageSignature([0x02; 65]);
1792-
b.block.header.chain_length = 2;
1793-
b.burn_height = 4;
1794-
});
1795-
block_info_3
1796-
.mark_locally_accepted(false)
1797-
.expect("Failed to mark block as locally accepted");
1798-
1799-
db.insert_block(&block_info_1)
1800-
.expect("Unable to insert block into db");
1801-
db.insert_block(&block_info_2)
1802-
.expect("Unable to insert block into db");
1803-
1804-
assert!(db.get_signer_last_accepted_block().unwrap().is_none());
1805-
1806-
block_info_1
1807-
.mark_globally_accepted()
1808-
.expect("Failed to mark block as globally accepted");
1809-
db.insert_block(&block_info_1)
1810-
.expect("Unable to insert block into db");
1811-
1812-
assert_eq!(
1813-
db.get_signer_last_accepted_block().unwrap().unwrap(),
1814-
block_info_1
1815-
);
1816-
1817-
block_info_2
1818-
.mark_globally_accepted()
1819-
.expect("Failed to mark block as globally accepted");
1820-
block_info_2.signed_self = Some(get_epoch_time_secs());
1821-
db.insert_block(&block_info_2)
1822-
.expect("Unable to insert block into db");
1823-
1824-
assert_eq!(
1825-
db.get_signer_last_accepted_block().unwrap().unwrap(),
1826-
block_info_2
1827-
);
1828-
1829-
db.insert_block(&block_info_3)
1830-
.expect("Unable to insert block into db");
1831-
1832-
assert_eq!(
1833-
db.get_signer_last_accepted_block().unwrap().unwrap(),
1834-
block_info_3
1835-
);
1836-
}
18371760
}

stacks-signer/src/v0/signer.rs

Lines changed: 57 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -582,60 +582,80 @@ impl Signer {
582582
}
583583
}
584584

585-
/// WARNING: Do NOT call this function PRIOR to check_proposal or block_proposal validation succeeds.
585+
/// WARNING: This is an incomplete check. Do NOT call this function PRIOR to check_proposal or block_proposal validation succeeds.
586586
///
587587
/// Re-verify a block's chain length against the last signed block within signerdb.
588588
/// This is required in case a block has been approved since the initial checks of the block validation endpoint.
589589
fn check_block_against_signer_db_state(
590-
&self,
590+
&mut self,
591+
stacks_client: &StacksClient,
591592
proposed_block: &NakamotoBlock,
592593
) -> Option<BlockResponse> {
593594
let signer_signature_hash = proposed_block.header.signer_signature_hash();
594595
let proposed_block_consensus_hash = proposed_block.header.consensus_hash;
596+
// If this is a tenure change block, ensure that it confirms the correct number of blocks from the parent tenure.
597+
if let Some(tenure_change) = proposed_block.get_tenure_change_tx_payload() {
598+
// Ensure that the tenure change block confirms the expected parent block
599+
match SortitionsView::check_tenure_change_confirms_parent(
600+
tenure_change,
601+
proposed_block,
602+
&mut self.signer_db,
603+
stacks_client,
604+
self.proposal_config.tenure_last_block_proposal_timeout,
605+
) {
606+
Ok(true) => {}
607+
Ok(false) => {
608+
return Some(
609+
self.create_block_rejection(
610+
RejectCode::SortitionViewMismatch,
611+
proposed_block,
612+
),
613+
)
614+
}
615+
Err(e) => {
616+
warn!("{self}: Error checking block proposal: {e}";
617+
"signer_sighash" => %signer_signature_hash,
618+
"block_id" => %proposed_block.block_id()
619+
);
620+
return Some(
621+
self.create_block_rejection(RejectCode::ConnectivityIssues, proposed_block),
622+
);
623+
}
624+
}
625+
}
595626

596-
match self.signer_db.get_signer_last_accepted_block() {
627+
// Ensure that the block is the last block in the chain of its current tenure.
628+
match self
629+
.signer_db
630+
.get_last_accepted_block(&proposed_block_consensus_hash)
631+
{
597632
Ok(Some(last_block_info)) => {
598633
if proposed_block.header.chain_length <= last_block_info.block.header.chain_length {
599-
// We do not allow reorgs at any time within the same consensus hash OR of globally accepted blocks
600-
let non_reorgable_block = last_block_info.block.header.consensus_hash
601-
== proposed_block_consensus_hash
602-
|| last_block_info.state == BlockState::GloballyAccepted;
603-
// Is the reorg timeout requirement exceeded?
604-
let reorg_timeout_exceeded = last_block_info
605-
.signed_self
606-
.map(|signed_over_time| {
607-
signed_over_time.saturating_add(
608-
self.proposal_config
609-
.tenure_last_block_proposal_timeout
610-
.as_secs(),
611-
) <= get_epoch_time_secs()
612-
})
613-
.unwrap_or(false);
614-
if non_reorgable_block || !reorg_timeout_exceeded {
615-
warn!(
616-
"Miner's block proposal does not confirm as many blocks as we expect";
617-
"proposed_block_consensus_hash" => %proposed_block_consensus_hash,
618-
"proposed_block_signer_sighash" => %signer_signature_hash,
619-
"proposed_chain_length" => proposed_block.header.chain_length,
620-
"expected_at_least" => last_block_info.block.header.chain_length + 1,
621-
);
622-
return Some(self.create_block_rejection(
623-
RejectCode::SortitionViewMismatch,
624-
proposed_block,
625-
));
626-
}
634+
warn!(
635+
"Miner's block proposal does not confirm as many blocks as we expect";
636+
"proposed_block_consensus_hash" => %proposed_block_consensus_hash,
637+
"proposed_block_signer_sighash" => %signer_signature_hash,
638+
"proposed_chain_length" => proposed_block.header.chain_length,
639+
"expected_at_least" => last_block_info.block.header.chain_length + 1,
640+
);
641+
return Some(self.create_block_rejection(
642+
RejectCode::SortitionViewMismatch,
643+
proposed_block,
644+
));
627645
}
628-
None
629646
}
630-
Ok(_) => None,
647+
Ok(_) => {}
631648
Err(e) => {
632649
warn!("{self}: Failed to check block against signer db: {e}";
633650
"signer_sighash" => %signer_signature_hash,
634651
"block_id" => %proposed_block.block_id()
635652
);
636-
Some(self.create_block_rejection(RejectCode::ConnectivityIssues, proposed_block))
653+
return Some(
654+
self.create_block_rejection(RejectCode::ConnectivityIssues, proposed_block),
655+
);
637656
}
638657
}
658+
None
639659
}
640660

641661
/// Handle the block validate ok response. Returns our block response if we have one
@@ -684,7 +704,9 @@ impl Signer {
684704
}
685705
};
686706

687-
if let Some(block_response) = self.check_block_against_signer_db_state(&block_info.block) {
707+
if let Some(block_response) =
708+
self.check_block_against_signer_db_state(stacks_client, &block_info.block)
709+
{
688710
// The signer db state has changed. We no longer view this block as valid. Override the validation response.
689711
if let Err(e) = block_info.mark_locally_rejected() {
690712
if !block_info.has_reached_consensus() {

stackslib/src/burnchains/bitcoin/blocks.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,7 @@ impl BitcoinMessageHandler for BitcoinBlockDownloader {
150150
None => panic!("No block header set"),
151151
Some(ref ipc_header) => {
152152
let block_hash = ipc_header.block_header.header.bitcoin_hash().clone();
153-
indexer
154-
.send_getdata(&vec![block_hash])
155-
.and_then(|_r| Ok(true))
153+
indexer.send_getdata(&vec![block_hash]).map(|_r| true)
156154
}
157155
}
158156
}

stackslib/src/burnchains/bitcoin/indexer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ impl BitcoinIndexer {
458458
}
459459
spv_client
460460
.run(self)
461-
.and_then(|_r| Ok(spv_client.end_block_height.unwrap()))
461+
.map(|_r| spv_client.end_block_height.unwrap())
462462
}
463463

464464
#[cfg(test)]

stackslib/src/burnchains/bitcoin/network.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,16 +127,16 @@ impl BitcoinIndexer {
127127
// classify the message here, so we can pass it along to the handler explicitly
128128
match message {
129129
btc_message::NetworkMessage::Version(..) => {
130-
return self.handle_version(message).and_then(|_r| Ok(true));
130+
return self.handle_version(message).map(|_r| true);
131131
}
132132
btc_message::NetworkMessage::Verack => {
133-
return self.handle_verack(message).and_then(|_r| Ok(true));
133+
return self.handle_verack(message).map(|_r| true);
134134
}
135135
btc_message::NetworkMessage::Ping(..) => {
136-
return self.handle_ping(message).and_then(|_r| Ok(true));
136+
return self.handle_ping(message).map(|_r| true);
137137
}
138138
btc_message::NetworkMessage::Pong(..) => {
139-
return self.handle_pong(message).and_then(|_r| Ok(true));
139+
return self.handle_pong(message).map(|_r| true);
140140
}
141141
_ => match handler {
142142
Some(custom_handler) => custom_handler.handle_message(self, message),

stackslib/src/burnchains/bitcoin/spv.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ impl SpvClient {
274274
tx.execute("UPDATE db_config SET version = ?1", &[version])
275275
.map_err(db_error::SqliteError)
276276
.map_err(|e| e.into())
277-
.and_then(|_| Ok(()))
277+
.map(|_| ())
278278
}
279279

280280
#[cfg(test)]
@@ -354,7 +354,7 @@ impl SpvClient {
354354
pub fn is_initialized(&self) -> Result<(), btc_error> {
355355
fs::metadata(&self.headers_path)
356356
.map_err(btc_error::FilesystemError)
357-
.and_then(|_m| Ok(()))
357+
.map(|_m| ())
358358
}
359359

360360
/// Get the block range to scan
@@ -762,7 +762,7 @@ impl SpvClient {
762762

763763
tx.execute(sql, args)
764764
.map_err(|e| btc_error::DBError(db_error::SqliteError(e)))
765-
.and_then(|_x| Ok(()))
765+
.map(|_x| ())
766766
}
767767

768768
/// Initialize the block headers file with the genesis block hash.
@@ -1231,7 +1231,7 @@ impl BitcoinMessageHandler for SpvClient {
12311231

12321232
indexer.runtime.last_getheaders_send_time = get_epoch_time_secs();
12331233
self.send_next_getheaders(indexer, start_height)
1234-
.and_then(|_r| Ok(true))
1234+
.map(|_r| true)
12351235
}
12361236

12371237
/// Trait message handler
@@ -1298,7 +1298,7 @@ impl BitcoinMessageHandler for SpvClient {
12981298
);
12991299
}
13001300
self.send_next_getheaders(indexer, block_height)
1301-
.and_then(|_| Ok(true))
1301+
.map(|_| true)
13021302
}
13031303
x => Err(btc_error::UnhandledMessage(x)),
13041304
}

stackslib/src/burnchains/tests/affirmation.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ pub fn make_reward_cycle_with_vote(
413413
new_commits.push(commits.clone());
414414
commits
415415
.into_iter()
416-
.filter_map(|cmt| cmt)
416+
.flatten()
417417
.map(|cmt| BlockstackOperationType::LeaderBlockCommit(cmt))
418418
.collect()
419419
};

0 commit comments

Comments
 (0)