Skip to content

Commit

Permalink
feat: add consistent ban reason for sync (#5729)
Browse files Browse the repository at this point in the history
Description
---
Added consistent ban reason for sync/block_sync sync/header_sync and
sync/horizon_state_sync w.r.t. validation errors.

Motivation and Context
---
`ValidationError::get_ban_reason` will determine if a particular
validation error is ban-able.

How Has This Been Tested?
---
Pass existing tests

What process can a PR reviewer use to test or verify this change?
---
Code walk-through

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->

Co-authored-by: SW van Heerden <[email protected]>
  • Loading branch information
hansieodendaal and SWvheerden authored Sep 3, 2023
1 parent cc8573a commit 9564281
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 3 deletions.
3 changes: 3 additions & 0 deletions base_layer/core/src/base_node/sync/block_sync/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ impl BlockSyncError {
impl BlockSyncError {
pub fn get_ban_reason(&self, short_ban: Duration, long_ban: Duration) -> Option<BanReason> {
match self {
// no ban
BlockSyncError::AsyncTaskFailed(_) |
BlockSyncError::RpcError(_) |
BlockSyncError::RpcRequestError(_) |
Expand All @@ -107,11 +108,13 @@ impl BlockSyncError {
BlockSyncError::FailedToConstructChainBlock |
BlockSyncError::SyncRoundFailed => None,

// short ban
err @ BlockSyncError::MaxLatencyExceeded { .. } => Some(BanReason {
reason: format!("{}", err),
ban_duration: short_ban,
}),

// long ban
err @ BlockSyncError::BlockWithoutParent { .. } |
err @ BlockSyncError::UnknownHeaderHash(_) |
err @ BlockSyncError::InvalidBlockBody(_) |
Expand Down
3 changes: 2 additions & 1 deletion base_layer/core/src/base_node/sync/header_sync/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ impl BlockHeaderSyncError {

// long ban
err @ BlockHeaderSyncError::ReceivedInvalidHeader(_) |
err @ BlockHeaderSyncError::ValidationFailed(_) |
err @ BlockHeaderSyncError::FoundHashIndexOutOfRange(_, _) |
err @ BlockHeaderSyncError::StartHashNotFound(_) |
err @ BlockHeaderSyncError::InvalidBlockHeight { .. } |
Expand All @@ -126,6 +125,8 @@ impl BlockHeaderSyncError {
reason: format!("{}", err),
ban_duration: long_ban,
}),

BlockHeaderSyncError::ValidationFailed(err) => ValidationError::get_ban_reason(err, Some(long_ban)),
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,13 @@ impl HorizonSyncError {
err @ HorizonSyncError::InvalidMmrPosition { .. } |
err @ HorizonSyncError::ConversionError(_) |
err @ HorizonSyncError::MerkleMountainRangeError(_) |
err @ HorizonSyncError::ValidationError(_) |
err @ HorizonSyncError::FixedHashSizeError(_) |
err @ HorizonSyncError::TransactionError(_) => Some(BanReason {
reason: format!("{}", err),
ban_duration: long_ban,
}),

HorizonSyncError::ValidationError(err) => ValidationError::get_ban_reason(err, Some(long_ban)),
}
}
}
5 changes: 4 additions & 1 deletion comms/dht/src/store_forward/saf_handler/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,10 @@ where S: Service<DecryptedDhtMessage, Response = (), Error = PipelineError>
.validate_and_decrypt_incoming_stored_message(Arc::clone(&source_peer), msg)
.await;

let Some(result) = self.process_saf_message_validation_result(&source_peer.public_key, result).await else {
let Some(result) = self
.process_saf_message_validation_result(&source_peer.public_key, result)
.await
else {
// Logging of problems and banning are done inside process_saf_message. We can simply continue
continue;
};
Expand Down

0 comments on commit 9564281

Please sign in to comment.