From 8723ba28af4d404900eb4d01d3e75ed3d64ac8c2 Mon Sep 17 00:00:00 2001
From: ian <ian@nervos.org>
Date: Tue, 17 Jan 2023 16:00:10 +0800
Subject: [PATCH] fix: p2p alerts are not filterred in block chain info

The root cause is that when launcher passes client version to alert
notifier, it uses the full format such as `0.107.0-rc1 (1b3e6b1
2023-01-12)`. However, the notifier internal uses semver to parse the
version directly. If it fails, it will consider all alerts as effective.

The solution: The launcher pass the short version format to alert
notifier.
---
 util/launcher/src/lib.rs           |  2 +-
 util/network-alert/src/notifier.rs | 35 ++++++++++++++++++++++++------
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/util/launcher/src/lib.rs b/util/launcher/src/lib.rs
index 01e77ca381..ed8efc34ea 100644
--- a/util/launcher/src/lib.rs
+++ b/util/launcher/src/lib.rs
@@ -342,7 +342,7 @@ impl Launcher {
 
         let alert_signature_config = self.args.config.alert_signature.clone().unwrap_or_default();
         let alert_relayer = AlertRelayer::new(
-            self.version.to_string(),
+            self.version.short(),
             shared.notify_controller().clone(),
             alert_signature_config,
         );
diff --git a/util/network-alert/src/notifier.rs b/util/network-alert/src/notifier.rs
index 1210b8f9a0..dedab7598b 100644
--- a/util/network-alert/src/notifier.rs
+++ b/util/network-alert/src/notifier.rs
@@ -1,8 +1,9 @@
 //! notifier module
-use ckb_logger::debug;
+use ckb_logger::{debug, error};
 use ckb_notify::NotifyController;
 use ckb_types::{packed::Alert, prelude::*};
 use lru::LruCache;
+use semver::Version;
 use std::collections::HashMap;
 
 const CANCEL_FILTER_SIZE: usize = 128;
@@ -15,25 +16,35 @@ pub struct Notifier {
     received_alerts: HashMap<u32, Alert>,
     /// alerts that self node should notice
     noticed_alerts: Vec<Alert>,
-    client_version: String,
+    client_version: Option<Version>,
     notify_controller: NotifyController,
 }
 
 impl Notifier {
     /// Init
     pub fn new(client_version: String, notify_controller: NotifyController) -> Self {
+        let parsed_client_version = match Version::parse(&client_version) {
+            Ok(version) => Some(version),
+            Err(err) => {
+                error!(
+                    "Invalid version {} for alert notifier: {}",
+                    client_version, err
+                );
+                None
+            }
+        };
+
         Notifier {
             cancel_filter: LruCache::new(CANCEL_FILTER_SIZE),
             received_alerts: Default::default(),
             noticed_alerts: Vec::new(),
-            client_version,
+            client_version: parsed_client_version,
             notify_controller,
         }
     }
 
     fn is_version_effective(&self, alert: &Alert) -> bool {
-        use semver::Version;
-        if let Ok(client_version) = Version::parse(&self.client_version) {
+        if let Some(client_version) = &self.client_version {
             let test_min_ver_failed = alert
                 .as_reader()
                 .raw()
@@ -42,7 +53,12 @@ impl Notifier {
                 .and_then(|v| {
                     v.as_utf8()
                         .ok()
-                        .and_then(|v| Version::parse(v).map(|min_v| client_version < min_v).ok())
+                        .and_then(|v| {
+                            Version::parse(v)
+                                .as_ref()
+                                .map(|min_v| client_version < min_v)
+                                .ok()
+                        })
                         .or(Some(true))
                 })
                 .unwrap_or(false);
@@ -57,7 +73,12 @@ impl Notifier {
                 .and_then(|v| {
                     v.as_utf8()
                         .ok()
-                        .and_then(|v| Version::parse(v).map(|max_v| client_version > max_v).ok())
+                        .and_then(|v| {
+                            Version::parse(v)
+                                .as_ref()
+                                .map(|max_v| client_version > max_v)
+                                .ok()
+                        })
                         .or(Some(true))
                 })
                 .unwrap_or(false);