No longer ban peers for BEEFY justifications (#1614)

* No longer ban peers for BEEFY justifications * PR link
smol-dot · Jan 26, 2024 · 422cf08 · 422cf08
1 parent d9d3ef7
commit 422cf08
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 25 deletions.
diff --git a/full-node/src/consensus_service.rs b/full-node/src/consensus_service.rs
@@ -2950,14 +2950,22 @@ impl SyncBackground {
                         (self, true)
                     }
                     (sync_out, all::FinalityProofVerifyOutcome::JustificationError(error)) => {
-                        self.network_service
-                            .ban_and_disconnect(
-                                sender.clone(),
-                                self.network_chain_id,
-                                network_service::BanSeverity::High,
-                                "bad-warp-sync-fragment",
-                            )
-                            .await;
+                        // Errors of type `JustificationEngineMismatch` indicate that the chain
+                        // uses a finality engine that smoldot doesn't recognize. This is a benign
+                        // error that shouldn't lead to a ban.
+                        if !matches!(
+                            error,
+                            all::JustificationVerifyError::JustificationEngineMismatch
+                        ) {
+                            self.network_service
+                                .ban_and_disconnect(
+                                    sender.clone(),
+                                    self.network_chain_id,
+                                    network_service::BanSeverity::High,
+                                    "bad-warp-sync-fragment",
+                                )
+                                .await;
+                        }
                         self.log_callback.log(
                             LogLevel::Warn,
                             format!(

diff --git a/lib/src/sync/all.rs b/lib/src/sync/all.rs
@@ -48,6 +48,7 @@ use core::{
 };
 
 pub use crate::executor::vm::ExecHint;
+pub use blocks_tree::{CommitVerifyError, JustificationVerifyError};
 pub use warp_sync::{
     BuildChainInformationError as WarpSyncBuildChainInformationError,
     BuildRuntimeError as WarpSyncBuildRuntimeError, ConfigCodeTrieNodeHint, VerifyFragmentError,
@@ -2046,9 +2047,9 @@ pub enum FinalityProofVerifyOutcome<TBl> {
     /// GrandPa commit cannot be verified yet and has been stored for later.
     GrandpaCommitPending,
     /// Problem while verifying justification.
-    JustificationError(blocks_tree::JustificationVerifyError),
+    JustificationError(JustificationVerifyError),
     /// Problem while verifying GrandPa commit.
-    GrandpaCommitError(blocks_tree::CommitVerifyError),
+    GrandpaCommitError(CommitVerifyError),
 }
 
 pub struct WarpSyncFragmentVerify<TRq, TSrc, TBl> {

diff --git a/light-base/src/sync_service/standalone.rs b/light-base/src/sync_service/standalone.rs
@@ -1364,22 +1364,28 @@ impl<TPlat: PlatformRef> Task<TPlat> {
                             sender,
                         );
 
-                        // TODO: don't print for consensusenginemismatch?
-                        log!(
-                            &self.platform,
-                            Warn,
-                            &self.log_target,
-                            format!("Error while verifying justification: {error}")
-                        );
+                        // Errors of type `JustificationEngineMismatch` indicate that the chain
+                        // uses a finality engine that smoldot doesn't recognize. This is a benign
+                        // error that shouldn't lead to a ban.
+                        if !matches!(
+                            error,
+                            all::JustificationVerifyError::JustificationEngineMismatch
+                        ) {
+                            log!(
+                                &self.platform,
+                                Warn,
+                                &self.log_target,
+                                format!("Error while verifying justification: {error}")
+                            );
 
-                        // TODO: don't ban for consensusenginemismatch?
-                        self.network_service
-                            .ban_and_disconnect(
-                                sender,
-                                network_service::BanSeverity::High,
-                                "bad-justification",
-                            )
-                            .await;
+                            self.network_service
+                                .ban_and_disconnect(
+                                    sender,
+                                    network_service::BanSeverity::High,
+                                    "bad-justification",
+                                )
+                                .await;
+                        }
                     }
 
                     (sync, all::FinalityProofVerifyOutcome::GrandpaCommitError(error)) => {

diff --git a/wasm-node/CHANGELOG.md b/wasm-node/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Fixed
+
+- Non-Grandpa justifications are now silently ignored, instead of leading to a ban of the peer that sent the justification. This makes smoldot work better on chains that use BEEFY. ([#1614](https://github.com/smol-dot/smoldot/pull/1614))
+
 ## 2.0.18 - 2024-01-24
 
 ### Changed