Skip to content

Commit

Permalink
Merge pull request #2524 from blockstack/hotfix/2518+2491
Browse files Browse the repository at this point in the history
Hotfix patch for Fix/2518+2491
  • Loading branch information
kantai authored Mar 18, 2021
2 parents 5f33a97 + f9da01c commit 59363a2
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 23 deletions.
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,21 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [2.0.10]

This is a low-priority hotfix release to address two bugs in the block downloader. The
chainstate directory of 2.0.10 is compatible with 2.0.9. If booting up a node from genesis, or
an existing node has stalled in downloading blocks, this hotfix is necessary for your
node.

## Fixed

- Bug in microblocks inventory vector calculation that included invalidated microblocks
as present bit. This bug will impact nodes booting up from genesis, but not affect nodes
currently running at the chain tip (#2518).
- Bug in microblocks downloader logic that would cause the stacks-node to fail to wake-up
to process newly arrived microblocks in certain instances (#2491).

## [2.0.9]

This is a hotfix release for improved handling of arriving Stacks blocks through
Expand Down
2 changes: 1 addition & 1 deletion src/chainstate/stacks/db/blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1865,7 +1865,7 @@ impl StacksChainState {
) -> Result<bool, Error> {
StacksChainState::read_i64s(self.db(), "SELECT staging_microblocks.processed
FROM staging_blocks JOIN staging_microblocks ON staging_blocks.parent_anchored_block_hash = staging_microblocks.anchored_block_hash AND staging_blocks.parent_consensus_hash = staging_microblocks.consensus_hash
WHERE staging_blocks.index_block_hash = ?1 AND staging_microblocks.microblock_hash = ?2", &[child_index_block_hash, &parent_microblock_hash])
WHERE staging_blocks.index_block_hash = ?1 AND staging_microblocks.microblock_hash = ?2 AND staging_microblocks.orphaned = 0", &[child_index_block_hash, &parent_microblock_hash])
.and_then(|processed| {
if processed.len() == 0 {
Ok(false)
Expand Down
60 changes: 41 additions & 19 deletions src/net/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,9 +655,11 @@ impl BlockDownloader {
info!("Remote neighbor {:?} ({:?}) does not have microblock stream indexed at {}", &block_key.neighbor, &block_key.data_url, &block_key.index_block_hash);

// the fact that we asked this peer means that it's block inv indicated
// it was present, so the absence is the mark of a broken peer
self.broken_peers.push(event_id);
self.broken_neighbors.push(block_key.neighbor.clone());
// it was present, so the absence is the mark of a broken peer.
// HOWEVER, there has been some bugs recently about nodes reporting
// invalid microblock streams as present, even though they are
// truly absent. Don't punish these peers with a ban; just don't
// talk to them for a while.
}
_ => {
// wrong message response
Expand Down Expand Up @@ -1306,7 +1308,11 @@ impl PeerNetwork {
)? {
Some(hdr) => hdr,
None => {
test_debug!("No such block: {:?}", &index_block_hash);
test_debug!(
"{:?}: No such parent block: {:?}",
&self.local_peer,
&index_block_hash
);
continue;
}
};
Expand All @@ -1321,7 +1327,7 @@ impl PeerNetwork {
}
Err(chainstate_error::DBError(db_error::NotFoundError)) => {
// we don't know about this parent block yet
debug!("{:?}: Do not have parent of anchored block {}/{} yet, so cannot ask for the microblocks it produced", &self.local_peer, &consensus_hash, &block_hash);
test_debug!("{:?}: Do not have parent of anchored block {}/{} yet, so cannot ask for the microblocks it produced", &self.local_peer, &consensus_hash, &block_hash);
continue;
}
Err(e) => {
Expand Down Expand Up @@ -1359,13 +1365,14 @@ impl PeerNetwork {
neighbors.clear();
neighbors.append(&mut microblock_stream_neighbors);

test_debug!(
"{:?}: Get microblocks produced by {}/{}, confirmed by {}/{}",
debug!(
"{:?}: Get microblocks produced by {}/{}, confirmed by {}/{}, from up to {} neighbors",
&self.local_peer,
&parent_consensus_hash,
&parent_header.block_hash(),
&consensus_hash,
&block_hash
&block_hash,
neighbors.len()
);

parent_block_header_opt = Some(parent_header);
Expand Down Expand Up @@ -1535,6 +1542,8 @@ impl PeerNetwork {
}
};

let scan_batch_size = self.burnchain.pox_constants.reward_cycle_length as u64;

if need_blocks {
PeerNetwork::with_downloader_state(self, |ref mut network, ref mut downloader| {
test_debug!("{:?}: needs blocks", &network.local_peer);
Expand Down Expand Up @@ -1599,8 +1608,14 @@ impl PeerNetwork {
}

if max_mblock_height == 0 && next_microblocks_to_try.len() == 0 {
// have no microblocks to try in the first place
max_mblock_height = max_height;
// have no microblocks to try in the first place, so just advance to the
// next batch
debug!(
"No microblocks to try; advance max_mblock_height to {}",
mblock_height
);
max_mblock_height = mblock_height;
mblock_height += scan_batch_size;
}

test_debug!("{:?}: at {},{}: {} blocks to get, {} microblock streams to get (up to {},{})",
Expand All @@ -1619,8 +1634,8 @@ impl PeerNetwork {
test_debug!("{:?}: End microblock requests", &network.local_peer);

debug!(
"{:?}: create block, microblock requests up to heights ({},{})",
&network.local_peer, &max_height, &max_mblock_height
"{:?}: create block, microblock requests from heights ({},{}) up to heights ({},{}) (so far: {} blocks, {} microblocks queued)",
&network.local_peer, height, mblock_height, max_height, max_mblock_height, downloader.blocks_to_try.len(), downloader.microblocks_to_try.len()
);

let now = get_epoch_time_secs();
Expand Down Expand Up @@ -1707,6 +1722,7 @@ impl PeerNetwork {
"BUG: hashmap both contains and does not contain sortition height",
);
if requests.len() == 0 {
debug!("No microblock requests for {}", mblock_height);
mblock_height += 1;
continue;
}
Expand Down Expand Up @@ -1747,12 +1763,14 @@ impl PeerNetwork {
}

debug!(
"{:?}: block download scan now at ({},{}) (was ({},{}))",
"{:?}: block download scan now at ({},{}) (was ({},{})), trying {} blocks and {} microblocks",
&network.local_peer,
height,
mblock_height,
block_sortition_height,
microblock_sortition_height
microblock_sortition_height,
downloader.blocks_to_try.len(),
downloader.microblocks_to_try.len(),
);

if max_height <= next_block_sortition_height
Expand All @@ -1779,12 +1797,16 @@ impl PeerNetwork {
}
}

if downloader.blocks_to_try.len() == 0 && downloader.microblocks_to_try.len() == 0 {
if downloader.blocks_to_try.len() == 0 || downloader.microblocks_to_try.len() == 0 {
// nothing in this range, so advance sortition range to try for next time
next_block_sortition_height = next_block_sortition_height
+ (network.burnchain.pox_constants.reward_cycle_length as u64);
next_microblock_sortition_height = next_microblock_sortition_height
+ (network.burnchain.pox_constants.reward_cycle_length as u64);
if downloader.blocks_to_try.len() == 0 {
next_block_sortition_height = next_block_sortition_height
+ (network.burnchain.pox_constants.reward_cycle_length as u64);
}
if downloader.microblocks_to_try.len() == 0 {
next_microblock_sortition_height = next_microblock_sortition_height
+ (network.burnchain.pox_constants.reward_cycle_length as u64);
}

debug!("{:?}: Pessimistically increase block and microblock sortition heights to ({},{})", &network.local_peer, next_block_sortition_height, next_microblock_sortition_height);
}
Expand Down
8 changes: 5 additions & 3 deletions src/net/relay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -993,11 +993,13 @@ impl Relayer {
Relayer::preprocess_pushed_microblocks(network_result, chainstate)?;
bad_neighbors.append(&mut new_bad_neighbors);

if new_blocks.len() > 0 || new_microblocks.len() > 0 {
if new_blocks.len() > 0 || new_microblocks.len() > 0 || new_confirmed_microblocks.len() > 0
{
info!(
"Processing newly received Stacks blocks: {}, microblocks: {}",
"Processing newly received Stacks blocks: {}, microblocks: {}, confirmed microblocks: {}",
new_blocks.len(),
new_microblocks.len()
new_microblocks.len(),
new_confirmed_microblocks.len()
);
if let Some(coord_comms) = coord_comms {
if !coord_comms.announce_new_stacks_block() {
Expand Down

0 comments on commit 59363a2

Please sign in to comment.