Skip to content

Commit

Permalink
fix(sync): ban peer if sending invalid prev_header (#3955)
Browse files Browse the repository at this point in the history
Description
---
- bans peer for ChainLinkBroken error

Motivation and Context
---
Node should not continue syncing from a peer that does not properly link headers

How Has This Been Tested?
---
Manually, my node banned peers that sent an incorrect prev header
  • Loading branch information
sdbondi authored Mar 24, 2022
1 parent 48eb86e commit 384ab0c
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 2 deletions.
8 changes: 6 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 @@ -67,8 +67,12 @@ pub enum BlockHeaderSyncError {
NetworkSilence,
#[error("Invalid protocol response: {0}")]
InvalidProtocolResponse(String),
#[error("Headers did not form a chain. Expected {actual} to equal the previous hash {expected}")]
ChainLinkBroken { actual: String, expected: String },
#[error("Header at height {height} did not form a chain. Expected {actual} to equal the previous hash {expected}")]
ChainLinkBroken {
height: u64,
actual: String,
expected: String,
},
#[error("Block error: {0}")]
BlockError(#[from] BlockError),
#[error(
Expand Down
19 changes: 19 additions & 0 deletions base_layer/core/src/base_node/sync/header_sync/synchronizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,19 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> {
})
.await?;
},
Err(BlockHeaderSyncError::ChainLinkBroken {
height,
actual,
expected,
}) => {
let reason = BanReason::ChainLinkBroken {
height,
actual: actual.to_string(),
expected: expected.to_string(),
};
warn!(target: LOG_TARGET, "Chain link broken: {}", reason);
self.ban_peer_long(node_id, reason).await?;
},
Err(ref err @ BlockHeaderSyncError::WeakerChain { claimed, actual, local }) => {
warn!(target: LOG_TARGET, "{}", err);
self.ban_peer_long(node_id, BanReason::PeerCouldNotProvideStrongerChain {
Expand Down Expand Up @@ -794,6 +807,12 @@ enum BanReason {
"Peer claimed an accumulated difficulty of {claimed} but validated difficulty was {actual} <= local: {local}"
)]
PeerCouldNotProvideStrongerChain { claimed: u128, actual: u128, local: u128 },
#[error("Header at height {height} did not form a chain. Expected {actual} to equal the previous hash {expected}")]
ChainLinkBroken {
height: u64,
actual: String,
expected: String,
},
}

struct ChainSplitInfo {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ impl<B: BlockchainBackend + 'static> BlockHeaderSyncValidator<B> {
}
if header.prev_hash != state.previous_accum.hash {
return Err(BlockHeaderSyncError::ChainLinkBroken {
height: header.height,
actual: header.prev_hash.to_hex(),
expected: state.previous_accum.hash.to_hex(),
});
Expand Down

0 comments on commit 384ab0c

Please sign in to comment.