From 384ab0ceddd25b5b31722fe639229c0ecf554926 Mon Sep 17 00:00:00 2001 From: Stan Bondi Date: Thu, 24 Mar 2022 16:06:40 +0400 Subject: [PATCH] fix(sync): ban peer if sending invalid prev_header (#3955) 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 --- .../src/base_node/sync/header_sync/error.rs | 8 ++++++-- .../sync/header_sync/synchronizer.rs | 19 +++++++++++++++++++ .../base_node/sync/header_sync/validator.rs | 1 + 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/base_layer/core/src/base_node/sync/header_sync/error.rs b/base_layer/core/src/base_node/sync/header_sync/error.rs index a81948dbb8..c744c5e4c5 100644 --- a/base_layer/core/src/base_node/sync/header_sync/error.rs +++ b/base_layer/core/src/base_node/sync/header_sync/error.rs @@ -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( 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 860b52bfaa..da6a073dd8 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 @@ -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 { @@ -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 { diff --git a/base_layer/core/src/base_node/sync/header_sync/validator.rs b/base_layer/core/src/base_node/sync/header_sync/validator.rs index 84f3c263e8..ca7cdcc28c 100644 --- a/base_layer/core/src/base_node/sync/header_sync/validator.rs +++ b/base_layer/core/src/base_node/sync/header_sync/validator.rs @@ -129,6 +129,7 @@ impl BlockHeaderSyncValidator { } 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(), });