From 4ca664e5933f2266f594ecccf545d0eec3b18b40 Mon Sep 17 00:00:00 2001
From: Hansie Odendaal <39146854+hansieodendaal@users.noreply.github.com>
Date: Tue, 31 Oct 2023 07:33:36 +0200
Subject: [PATCH] feat: make prc errors ban-able for sync (#5884)

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 -->
---
 base_layer/core/src/base_node/sync/header_sync/error.rs     | 6 +++---
 .../core/src/base_node/sync/horizon_state_sync/error.rs     | 6 +++---
 2 files changed, 6 insertions(+), 6 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 998944f2c6..e83ee7c05a 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
@@ -98,8 +98,6 @@ impl BlockHeaderSyncError {
         match self {
             // no ban
             BlockHeaderSyncError::NoMoreSyncPeers(_) |
-            BlockHeaderSyncError::RpcError(_) |
-            BlockHeaderSyncError::RpcRequestError(_) |
             BlockHeaderSyncError::SyncFailedAllPeers |
             BlockHeaderSyncError::FailedToBan(_) |
             BlockHeaderSyncError::AllSyncPeersExceedLatency |
@@ -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,
             }),
diff --git a/base_layer/core/src/base_node/sync/horizon_state_sync/error.rs b/base_layer/core/src/base_node/sync/horizon_state_sync/error.rs
index d0fa0dfe7a..dc0211125e 100644
--- a/base_layer/core/src/base_node/sync/horizon_state_sync/error.rs
+++ b/base_layer/core/src/base_node/sync/horizon_state_sync/error.rs
@@ -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,
             }),