Skip to content

Commit

Permalink
feat: make prc errors ban-able for sync (#5884)
Browse files Browse the repository at this point in the history
Description
---
Made RPC errors ban-able during header-sync and horizon-sync ban-able
with a short ban duration.

Closes #5874

Motivation and Context
---
During sync, we establish a client-server connection and then execute
RPC methods from the client to the server, which we know should succeed,
so that any RPC errors afterwards are a banable offence. Something else
to consider is that sync only acts if better peer metadata is received
from a peer, translating to malicious behaviour should a peer not want
to play the protocol afterwards.

How Has This Been Tested?
---
Integration-level unit tests in `base_layer\core\tests\tests`:
- `block_sync.rs`
- `header_sync.rs`
- `horizon_sync.rs` (still in progress)

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

<!-- 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 -->
  • Loading branch information
hansieodendaal authored Oct 31, 2023
1 parent df9bc9a commit 4ca664e
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
6 changes: 3 additions & 3 deletions base_layer/core/src/base_node/sync/header_sync/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@ impl BlockHeaderSyncError {
match self {
// no ban
BlockHeaderSyncError::NoMoreSyncPeers(_) |
BlockHeaderSyncError::RpcError(_) |
BlockHeaderSyncError::RpcRequestError(_) |
BlockHeaderSyncError::SyncFailedAllPeers |
BlockHeaderSyncError::FailedToBan(_) |
BlockHeaderSyncError::AllSyncPeersExceedLatency |
Expand All @@ -109,7 +107,9 @@ impl BlockHeaderSyncError {
BlockHeaderSyncError::ChainStorageError(_) => None,

// short ban
err @ BlockHeaderSyncError::MaxLatencyExceeded { .. } => Some(BanReason {
err @ BlockHeaderSyncError::MaxLatencyExceeded { .. } |
err @ BlockHeaderSyncError::RpcError { .. } |
err @ BlockHeaderSyncError::RpcRequestError { .. } => Some(BanReason {
reason: format!("{}", err),
ban_duration: short_ban,
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,14 @@ impl HorizonSyncError {
HorizonSyncError::FailedSyncAllPeers |
HorizonSyncError::AllSyncPeersExceedLatency |
HorizonSyncError::ConnectivityError(_) |
HorizonSyncError::RpcError(_) |
HorizonSyncError::RpcStatus(_) |
HorizonSyncError::NoMoreSyncPeers(_) |
HorizonSyncError::PeerNotFound |
HorizonSyncError::JoinError(_) => None,

// short ban
err @ HorizonSyncError::MaxLatencyExceeded { .. } => Some(BanReason {
err @ HorizonSyncError::MaxLatencyExceeded { .. } |
err @ HorizonSyncError::RpcError { .. } |
err @ HorizonSyncError::RpcStatus { .. } => Some(BanReason {
reason: format!("{}", err),
ban_duration: short_ban,
}),
Expand Down

0 comments on commit 4ca664e

Please sign in to comment.