From d25fa983ab94d4a100a17fe715db2da4a05d04dd Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Tue, 15 Aug 2023 18:12:56 +0200 Subject: [PATCH 1/3] fix header sync --- .../sync/header_sync/synchronizer.rs | 54 ++++++++++++------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs b/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs index 90a27ffa15..814e8afbe3 100644 --- a/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs +++ b/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs @@ -596,25 +596,28 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { ); self.switch_to_pending_chain(&split_info).await?; has_switched_to_new_chain = true; + } else { + if pending_len < NUM_INITIAL_HEADERS_TO_REQUEST { + // Peer returned less than the max number of requested headers. This indicates that we have all the available + // headers and the pow is less than the current chain + debug!(target: LOG_TARGET, "No further headers to download"); + if !has_better_pow { + return Err(BlockHeaderSyncError::PeerSentInaccurateChainMetadata { + claimed: sync_peer.claimed_chain_metadata().accumulated_difficulty(), + actual: Some(total_accumulated_difficulty), + local: split_info + .local_tip_header + .accumulated_data() + .total_accumulated_difficulty, + }); + } + } else + { + // Peer sent max requested headers, and we still have less pow than our chain. So we assume more is coming, and we continue downloading + } } - if pending_len < NUM_INITIAL_HEADERS_TO_REQUEST { - // Peer returned less than the number of requested headers. This indicates that we have all the available - // headers. - debug!(target: LOG_TARGET, "No further headers to download"); - if !has_better_pow { - return Err(BlockHeaderSyncError::PeerSentInaccurateChainMetadata { - claimed: sync_peer.claimed_chain_metadata().accumulated_difficulty(), - actual: Some(total_accumulated_difficulty), - local: split_info - .local_tip_header - .accumulated_data() - .total_accumulated_difficulty, - }); - } - return Ok(()); - } debug!( target: LOG_TARGET, @@ -639,6 +642,7 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { let mut last_total_accumulated_difficulty = 0; let mut avg_latency = RollingAverageTime::new(20); + let mut prev_height = None; while let Some(header) = header_stream.next().await { let latency = last_sync_timer.elapsed(); avg_latency.add_sample(latency); @@ -651,6 +655,17 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { header.hash().to_hex(), latency ); + if let Some(prev_header_height) = prev_height{ + if header.height != prev_header_height + 1 { + warn!( + target: LOG_TARGET, + "Received header #{} `{}` does not follow previous header", + header.height, + header.hash().to_hex() + ); + return Err(BlockHeaderSyncError::ReceivedInvalidHeader("Header does not follow previous header".to_string())) + } + } let existing_header = self.db.fetch_header_by_block_hash(header.hash()).await?; if let Some(h) = existing_header { warn!( @@ -659,7 +674,7 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { h.height, h.hash().to_hex() ); - continue; + return Err(BlockHeaderSyncError::ReceivedInvalidHeader("Header already in database".to_string())) } let current_height = header.height; last_total_accumulated_difficulty = self.header_validator.validate(header).await?; @@ -696,6 +711,7 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { } last_sync_timer = Instant::now(); + prev_height = Some(current_height); } if !has_switched_to_new_chain { @@ -784,9 +800,9 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { return false; } - // Check that the remote tip is stronger than the local tip + // Check that the remote tip is stronger than the local tip, equal should not have ended up here, so we treat equal as less let proposed_tip = chain_headers.last().unwrap(); - self.header_validator.compare_chains(current_tip, proposed_tip).is_le() + self.header_validator.compare_chains(current_tip, proposed_tip).is_lt() } async fn switch_to_pending_chain(&mut self, split_info: &ChainSplitInfo) -> Result<(), BlockHeaderSyncError> { From 3ae606b1e2263c7570aeeaab092c9c758105b9ab Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Wed, 16 Aug 2023 14:10:02 +0200 Subject: [PATCH 2/3] fmt --- .../sync/header_sync/synchronizer.rs | 55 ++++++++++--------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs b/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs index 814e8afbe3..3413903352 100644 --- a/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs +++ b/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs @@ -598,27 +598,23 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { has_switched_to_new_chain = true; } else { if pending_len < NUM_INITIAL_HEADERS_TO_REQUEST { - // Peer returned less than the max number of requested headers. This indicates that we have all the available - // headers and the pow is less than the current chain + // Peer returned less than the max number of requested headers. This indicates that we have all the + // available headers and the pow is less than the current chain debug!(target: LOG_TARGET, "No further headers to download"); - if !has_better_pow { - return Err(BlockHeaderSyncError::PeerSentInaccurateChainMetadata { - claimed: sync_peer.claimed_chain_metadata().accumulated_difficulty(), - actual: Some(total_accumulated_difficulty), - local: split_info - .local_tip_header - .accumulated_data() - .total_accumulated_difficulty, - }); - } - } else - { - // Peer sent max requested headers, and we still have less pow than our chain. So we assume more is coming, and we continue downloading + return Err(BlockHeaderSyncError::PeerSentInaccurateChainMetadata { + claimed: sync_peer.claimed_chain_metadata().accumulated_difficulty(), + actual: Some(total_accumulated_difficulty), + local: split_info + .local_tip_header + .accumulated_data() + .total_accumulated_difficulty, + }); + } else { + // Peer sent max requested headers, and we still have less pow than our chain. So we assume more is + // coming, and we continue downloading } } - - debug!( target: LOG_TARGET, "Download remaining headers starting from header #{} from peer `{}`", @@ -655,15 +651,17 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { header.hash().to_hex(), latency ); - if let Some(prev_header_height) = prev_height{ - if header.height != prev_header_height + 1 { + if let Some(prev_header_height) = prev_height { + if header.height != prev_header_height + 1 { warn!( - target: LOG_TARGET, - "Received header #{} `{}` does not follow previous header", - header.height, - header.hash().to_hex() - ); - return Err(BlockHeaderSyncError::ReceivedInvalidHeader("Header does not follow previous header".to_string())) + target: LOG_TARGET, + "Received header #{} `{}` does not follow previous header", + header.height, + header.hash().to_hex() + ); + return Err(BlockHeaderSyncError::ReceivedInvalidHeader( + "Header does not follow previous header".to_string(), + )); } } let existing_header = self.db.fetch_header_by_block_hash(header.hash()).await?; @@ -674,7 +672,9 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { h.height, h.hash().to_hex() ); - return Err(BlockHeaderSyncError::ReceivedInvalidHeader("Header already in database".to_string())) + return Err(BlockHeaderSyncError::ReceivedInvalidHeader( + "Header already in database".to_string(), + )); } let current_height = header.height; last_total_accumulated_difficulty = self.header_validator.validate(header).await?; @@ -800,7 +800,8 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { return false; } - // Check that the remote tip is stronger than the local tip, equal should not have ended up here, so we treat equal as less + // Check that the remote tip is stronger than the local tip, equal should not have ended up here, so we treat + // equal as less let proposed_tip = chain_headers.last().unwrap(); self.header_validator.compare_chains(current_tip, proposed_tip).is_lt() } From 9702ae81ca4b47c18cf0e65ff31a29f88ed6cbdd Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Wed, 16 Aug 2023 19:01:37 +0200 Subject: [PATCH 3/3] fix --- .../base_node/sync/header_sync/synchronizer.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs b/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs index 3413903352..a1ac770232 100644 --- a/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs +++ b/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs @@ -588,6 +588,7 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { // If we already have a stronger chain at this point, switch over to it. // just in case we happen to be exactly NUM_INITIAL_HEADERS_TO_REQUEST headers behind. let has_better_pow = self.pending_chain_has_higher_pow(&split_info.local_tip_header); + if has_better_pow { debug!( target: LOG_TARGET, @@ -596,10 +597,13 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { ); self.switch_to_pending_chain(&split_info).await?; has_switched_to_new_chain = true; - } else { - if pending_len < NUM_INITIAL_HEADERS_TO_REQUEST { - // Peer returned less than the max number of requested headers. This indicates that we have all the - // available headers and the pow is less than the current chain + } + + if pending_len < NUM_INITIAL_HEADERS_TO_REQUEST { + // Peer returned less than the max number of requested headers. This indicates that we have all the + // available headers from the peer. + if !has_better_pow { + // Because the pow is less or equal than the current chain the peer had to have lied about their pow debug!(target: LOG_TARGET, "No further headers to download"); return Err(BlockHeaderSyncError::PeerSentInaccurateChainMetadata { claimed: sync_peer.claimed_chain_metadata().accumulated_difficulty(), @@ -609,10 +613,10 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { .accumulated_data() .total_accumulated_difficulty, }); - } else { - // Peer sent max requested headers, and we still have less pow than our chain. So we assume more is - // coming, and we continue downloading } + // The pow is higher, we swapped to the higher chain, we have all the better chain headers, we can move on + // to block sync. + return Ok(()); } debug!(