Skip to content

Commit

Permalink
revert: fix ban header syn peer if no headers provided (tari-project#…
Browse files Browse the repository at this point in the history
…3297) (tari-project#3306)


Description
---
This reverts commit 570e222.

Motivation and Context
---
tari-project#3297 (comment)

Having thought about it a bit - header sync is designed to be self contained. It does not have the information required to ban a peer that "lied" in listening mode. 

Suggest that we pass through the chain metadata and peer that 'kicked' the base node into sync into header sync so 
that the peer can be banned if it fails to follow through. We should also not prevent mining when in header sync until a
split and better chain have been comfirmed by header sync.

How Has This Been Tested?
---
Existing tests pass
  • Loading branch information
sdbondi authored and Cifko committed Sep 10, 2021
1 parent 172987c commit 18bb00a
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 16 deletions.
2 changes: 0 additions & 2 deletions base_layer/core/src/base_node/sync/header_sync/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,4 @@ pub enum BlockHeaderSyncError {
InvalidProtocolResponse(String),
#[error("Headers did not form a chain. Expected {actual} to equal the previous hash {expected}")]
ChainLinkBroken { actual: String, expected: String },
#[error("Invalid remote peer behaviour: {0}")]
InvalidRemotePeerBehaviour(String),
}
14 changes: 0 additions & 14 deletions base_layer/core/src/base_node/sync/header_sync/synchronizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,6 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> {
warn!(target: LOG_TARGET, "Chain split not found for peer {}.", peer);
self.ban_peer_long(peer, BanReason::ChainSplitNotFound).await?;
},
Err(err @ BlockHeaderSyncError::InvalidRemotePeerBehaviour(_)) => {
warn!(target: LOG_TARGET, "{}", err);
self.ban_peer_long(node_id, BanReason::PeerMisbehaviour(err.to_string()))
.await?;
},
Err(err @ BlockHeaderSyncError::InvalidBlockHeight { .. }) => {
warn!(target: LOG_TARGET, "{}", err);
self.ban_peer_long(node_id, BanReason::GeneralHeaderSyncFailure(err))
Expand Down Expand Up @@ -387,13 +382,6 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> {
peer
);

if headers.is_empty() {
return Err(BlockHeaderSyncError::InvalidRemotePeerBehaviour(format!(
"Remote peer {} advertised a better chain but did not return headers",
peer
)));
}

if fork_hash_index >= block_hashes.len() as u32 {
let _ = self
.ban_peer_long(peer.clone(), BanReason::SplitHashGreaterThanHashes {
Expand Down Expand Up @@ -689,8 +677,6 @@ enum BanReason {
GeneralHeaderSyncFailure(BlockHeaderSyncError),
#[error("Peer did not respond timeously during RPC negotiation")]
RpcNegotiationTimedOut,
#[error("Peer misbehaviour: {0}")]
PeerMisbehaviour(String),
}

struct ChainSplitInfo {
Expand Down

0 comments on commit 18bb00a

Please sign in to comment.