From 18eb212d8ed1a3a11c1e895249ac42f6d84e3244 Mon Sep 17 00:00:00 2001 From: Henry de Valence Date: Mon, 22 Jun 2020 22:31:26 -0700 Subject: [PATCH] Set the new tips to be the last, not first, hash. --- zebrad/src/commands/start/sync.rs | 130 +++++++++++++----------------- 1 file changed, 54 insertions(+), 76 deletions(-) diff --git a/zebrad/src/commands/start/sync.rs b/zebrad/src/commands/start/sync.rs index 3d444b17c..c8fa04f10 100644 --- a/zebrad/src/commands/start/sync.rs +++ b/zebrad/src/commands/start/sync.rs @@ -6,9 +6,24 @@ use tower::{Service, ServiceExt}; use tracing_futures::Instrument; use zebra_chain::{block::BlockHeaderHash, types::BlockHeight}; +use zebra_network as zn; +use zebra_state as zs; + +/// Get the heights of the blocks for constructing a block_locator list +#[allow(dead_code)] +pub fn block_locator_heights(tip_height: BlockHeight) -> impl Iterator { + iter::successors(Some(1u32), |h| h.checked_mul(2)) + .flat_map(move |step| tip_height.0.checked_sub(step)) + .map(BlockHeight) + .chain(iter::once(BlockHeight(0))) +} + +type Error = Box; +type NumReq = u32; + pub struct Syncer where - ZN: Service, + ZN: Service, { pub peer_set: ZN, // TODO(jlusby): add validator @@ -22,15 +37,9 @@ where impl Syncer where - ZN: Service - + Send - + Clone - + 'static, + ZN: Service + Send + Clone + 'static, ZN::Future: Send, - ZS: Service - + Send - + Clone - + 'static, + ZS: Service + Send + Clone + 'static, ZS::Future: Send, { pub async fn run(&mut self) -> Result<(), Report> { @@ -71,7 +80,7 @@ where // fanout parameter, to get resp1, ..., respF for _ in 0..self.fanout { let req = self.peer_set.ready_and().await.map_err(|e| eyre!(e))?.call( - zebra_network::Request::FindBlocks { + zn::Request::FindBlocks { known_blocks: block_locator.clone(), stop: None, }, @@ -82,62 +91,51 @@ where let mut download_set = HashSet::new(); while let Some(res) = tip_futs.next().await { match res.map_err::(|e| eyre!(e)) { - Ok(zebra_network::Response::BlockHeaderHashes(hashes)) => { - info!( - new_hashes = hashes.len(), - in_flight = self.block_requests.len(), - downloaded = self.downloaded.len(), - "requested more hashes" - ); - - // TODO(jlusby): reject both main and test net genesis blocks - if hashes.last() == Some(&super::GENESIS) { + Ok(zn::Response::BlockHeaderHashes(hashes)) => { + if hashes.is_empty() { + tracing::debug!("skipping empty response"); continue; } - let mut hashes = hashes.into_iter().peekable(); - let new_tip = if let Some(tip) = hashes.next() { - tip - } else { - continue; - }; - // ObtainTips Step 3 // // For each response, starting from the beginning of the // list, prune any block hashes already included in the // state, stopping at the first unknown hash to get resp1', // ..., respF'. (These lists may be empty). - while let Some(&next) = hashes.peek() { - let resp = self + let mut first_unknown = 0; + for (i, &hash) in hashes.iter().enumerate() { + let depth = self .state .ready_and() .await .map_err(|e| eyre!(e))? - .call(zebra_state::Request::GetDepth { hash: next }) + .call(zebra_state::Request::GetDepth { hash }) .await .map_err(|e| eyre!(e))?; - - let should_download = matches!(resp, zebra_state::Response::Depth(None)); - - if should_download { - download_set.extend(hashes); + if let zs::Response::Depth(None) = depth { + first_unknown = i; break; - } else { - let _ = hashes.next(); } } + if first_unknown == hashes.len() { + tracing::debug!("no new hashes, even though we gave our tip?"); + continue; + } + let unknown_hashes = &hashes[first_unknown..]; + download_set.extend(unknown_hashes); // ObtainTips Step 4 // // Combine the last elements of each list into a set; this // is the set of prospective tips. + let new_tip = *unknown_hashes + .last() + .expect("already checked first_unknown < hashes.len()"); let _ = self.prospective_tips.insert(new_tip); } - Ok(_) => {} - Err(e) => { - error!("{:?}", e); - } + Ok(r) => tracing::error!("unexpected response {:?}", r), + Err(e) => tracing::error!("{:?}", e), } } @@ -169,19 +167,17 @@ where .ready_and() .await .map_err(|e| eyre!(e))? - .call(zebra_network::Request::FindBlocks { + .call(zn::Request::FindBlocks { known_blocks: vec![tip], stop: None, }) .await; match res.map_err::(|e| eyre!(e)) { - Ok(zebra_network::Response::BlockHeaderHashes(hashes)) => { - info!( - new_hashes = hashes.len(), - in_flight = self.block_requests.len(), - downloaded = self.downloaded.len(), - "requested more hashes" - ); + Ok(zn::Response::BlockHeaderHashes(hashes)) => { + if hashes.is_empty() { + tracing::debug!("skipping empty response"); + continue; + } // ExtendTips Step 3 // @@ -189,21 +185,17 @@ where // response is the genesis block; if so, discard the response. // It indicates that the remote peer does not have any blocks // following the prospective tip. - if hashes.last() == Some(&super::GENESIS) { + // TODO(jlusby): reject both main and test net genesis blocks + if hashes[0] == super::GENESIS { + tracing::debug!("skipping response that does not extend the tip"); continue; } - let mut hashes = hashes.into_iter(); - let new_tip = if let Some(tip) = hashes.next() { - tip - } else { - continue; - }; - // ExtendTips Step 4 // // Combine the last elements of the remaining responses into // a set, and add this set to the set of prospective tips. + let new_tip = *hashes.last().expect("already checked is_empty"); let _ = self.prospective_tips.insert(new_tip); // ExtendTips Step 5 @@ -212,10 +204,8 @@ where // set, and queue download and verification of those blocks download_set.extend(hashes); } - Ok(_) => {} - Err(e) => { - error!("{:?}", e); - } + Ok(r) => tracing::error!("unexpected response {:?}", r), + Err(e) => tracing::error!("{:?}", e), } } } @@ -244,7 +234,7 @@ where while let Some(res) = self.block_requests.next().await { match res.map_err::(|e| eyre!(e)) { - Ok(zebra_network::Response::Blocks(blocks)) => { + Ok(zn::Response::Blocks(blocks)) => { info!(count = blocks.len(), "received blocks"); for block in blocks { let hash = block.as_ref().into(); @@ -278,7 +268,7 @@ where .ready_and() .await .map_err(|e| eyre!(e))? - .call(zebra_state::Request::AddBlock { block }); + .call(zs::Request::AddBlock { block }); let _handle = tokio::spawn( async move { @@ -307,7 +297,7 @@ where .ready_and() .await .map_err(|e| eyre!(e))? - .call(zebra_network::Request::BlocksByHash(set)); + .call(zn::Request::BlocksByHash(set)); self.downloading.extend(chunk); self.block_requests.push(request); @@ -315,15 +305,3 @@ where Ok(()) } } - -/// Get the heights of the blocks for constructing a block_locator list -#[allow(dead_code)] -pub fn block_locator_heights(tip_height: BlockHeight) -> impl Iterator { - iter::successors(Some(1u32), |h| h.checked_mul(2)) - .flat_map(move |step| tip_height.0.checked_sub(step)) - .map(BlockHeight) - .chain(iter::once(BlockHeight(0))) -} - -type Error = Box; -type NumReq = u32;