Skip to content

Commit

Permalink
revert: fix ban header syn peer if no headers provided (#3297) (#3306)
Browse files Browse the repository at this point in the history
Description
---
This reverts commit 570e222.

Motivation and Context
---
#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 committed Sep 6, 2021
1 parent 7e58845 commit bc95c18
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 bc95c18

Please sign in to comment.