From 7097185c7f52161edd6aa7ddec7f4ab47449795f Mon Sep 17 00:00:00 2001
From: mrnaveira <47919901+mrnaveira@users.noreply.github.com>
Date: Fri, 29 Apr 2022 09:51:47 +0100
Subject: [PATCH] fix: weird behaviour of dates in base node banned peers
 (#4037)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Description
---
* Changed the peer display format to output `offline_at` and `banned_until` as local time instead of UTC, fixing the “time in the past” issue
* Improved handling and display of permanent bans (when invoking `ban-peer` with no params). Note that this makes _existing_ permabans still display weird times, changes affect only new permabans
* Scope of the changes does seem to only affect log messages in other applications

Motivation and Context
---
There is some weird behaviour regarding display of date times on banned peers.
- Times are shown in UTC but without the timezone, so if the user is in UTC+N the ban end time may look like it's "in the past" for some values
- The default ban time is permanent, and this makes the "banned until" display the max datetime.

How Has This Been Tested?
---
Manually tested on a base node, by banning peers with different values
---
 comms/core/src/peer_manager/peer.rs | 10 +++++++---
 comms/core/src/utils/datetime.rs    | 19 +++++++++++++------
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/comms/core/src/peer_manager/peer.rs b/comms/core/src/peer_manager/peer.rs
index b09f314f5b..39f8283a21 100644
--- a/comms/core/src/peer_manager/peer.rs
+++ b/comms/core/src/peer_manager/peer.rs
@@ -45,7 +45,7 @@ use crate::{
     peer_manager::identity_signature::IdentitySignature,
     protocol::ProtocolId,
     types::CommsPublicKey,
-    utils::datetime::safe_future_datetime_from_duration,
+    utils::datetime::{format_local_datetime, is_max_datetime, safe_future_datetime_from_duration},
 };
 
 bitflags! {
@@ -336,11 +336,15 @@ impl Display for Peer {
         let status_str = {
             let mut s = Vec::new();
             if let Some(offline_at) = self.offline_at.as_ref() {
-                s.push(format!("Offline since: {}", offline_at));
+                s.push(format!("Offline since: {}", format_local_datetime(offline_at)));
             }
 
             if let Some(dt) = self.banned_until() {
-                s.push(format!("Banned until: {}", dt));
+                if is_max_datetime(dt) {
+                    s.push("Banned permanently".to_string());
+                } else {
+                    s.push(format!("Banned until: {}", format_local_datetime(dt)));
+                }
                 s.push(format!("Reason: {}", self.banned_reason))
             }
             s.join(". ")
diff --git a/comms/core/src/utils/datetime.rs b/comms/core/src/utils/datetime.rs
index 53bfc93637..e61e2581ff 100644
--- a/comms/core/src/utils/datetime.rs
+++ b/comms/core/src/utils/datetime.rs
@@ -22,15 +22,13 @@
 
 use std::time::Duration;
 
-use chrono::{DateTime, NaiveTime, Utc};
+use chrono::{DateTime, Local, NaiveDateTime, Utc};
 
 pub fn safe_future_datetime_from_duration(duration: Duration) -> DateTime<Utc> {
     let old_duration = chrono::Duration::from_std(duration).unwrap_or_else(|_| chrono::Duration::max_value());
-    Utc::now().checked_add_signed(old_duration).unwrap_or_else(|| {
-        chrono::MAX_DATE
-            .and_time(NaiveTime::from_hms(0, 0, 0))
-            .expect("cannot fail")
-    })
+    Utc::now()
+        .checked_add_signed(old_duration)
+        .unwrap_or(chrono::MAX_DATETIME)
 }
 
 pub fn format_duration(duration: Duration) -> String {
@@ -48,6 +46,15 @@ pub fn format_duration(duration: Duration) -> String {
     }
 }
 
+pub fn format_local_datetime(datetime: &NaiveDateTime) -> String {
+    let local_datetime = DateTime::<Local>::from_utc(*datetime, Local::now().offset().to_owned());
+    local_datetime.format("%Y-%m-%d %H:%M:%S").to_string()
+}
+
+pub fn is_max_datetime(datetime: &NaiveDateTime) -> bool {
+    chrono::MAX_DATETIME.naive_utc() == *datetime
+}
+
 #[cfg(test)]
 mod test {
     use super::*;