Merge #21602: rpc: add additional ban time fields to listbanned

d3b0b08 doc: release notes for new listbanned fields (Jarol Rodriguez)
60290d3 test: increase listbanned unit test coverage (Jon Atack)
3e978d1 rpc: add time_remaining field to listbanned (Jarol Rodriguez)
5456b34 rpc: add ban_duration field to listbanned (Jarol Rodriguez)
c95c616 doc: improve listbanned help (Jarol Rodriguez)
dd3c8ea rpc: swap position of banned_until and ban_created fields (Jarol Rodriguez)

Pull request description:

  This PR adds a `ban_duration` and `time_remaining` field to the `listbanned` RPC command. Thanks to jonatack, this PR also expands the `listbanned` test coverage to include these new fields

  It's useful to keep track of `ban_duration` as this is another data point on which to sort banned peers. I found this helpful in adding additional context columns to the GUI `bantablemodel` as part of a follow-up PR. As [suggested](bitcoin/bitcoin#21602 (comment)) by jonatack, `time_remaining` is another useful user-centric data point.

  Since a ban always expires after its created, the `ban_created` field is now placed before the `banned_until` field. This new ordering is more logical.

  This PR also improves the `help listbanned` output by providing additional context to the descriptions of the `address`, `ban_created`, and `banned_until` fields.

  **Master: listbanned**
  ```
  [
    {
      "address": "1.2.3.4/32",
      "banned_until": 1617691101,
      "ban_created": 1617604701
    },
    {
      "address": "135.181.41.129/32",
      "banned_until": 1649140716,
      "ban_created": 1617604716
    }
  ]
  ```

  **PR: listbanned**
  ```
  [
    {
      "address": "1.2.3.4/32",
      "ban_created": 1617775773,
      "banned_until": 1617862173,
      "ban_duration": 86400,
      "time_remaining": 86392
    },
    {
      "address": "3.114.211.172/32",
      "ban_created": 1617753165,
      "banned_until": 1618357965,
      "ban_duration": 604800,
      "time_remaining": 582184
    }
  ]
  ```

ACKs for top commit:
  jonatack:
    re-ACK d3b0b08
  hebasto:
    ACK d3b0b08, tested on Linux Mint 20.1 (x86_64).
  MarcoFalke:
    review ACK d3b0b08 🕙

Tree-SHA512: 5b83ed2483344e546d57e43adc8a1ed7a1fff292124b14c86ca3a1aa2aec8b0f7198212fabff2c5145e7f726ca04ae567fe667b141254c7519df290cf63774e5

Author: MarcoFalke

doc/release-notes.md

@@ -104,6 +104,10 @@ Updated RPCs
   with the `-json` option set, the following fields: `addresses`, `reqSigs` are no longer
   returned in the tx output of the response. (#20286)
 
+- The `listbanned` RPC now returns two new numeric fields: `ban_duration` and `time_remaining`.
+  Respectively, these new fields indicate the duration of a ban and the time remaining until a ban expires,
+  both in seconds. Additionally, the `ban_created` field is repositioned to come before `banned_until`. (#21602)
+
 Changes to Wallet or GUI related RPCs can be found in the GUI or Wallet section below.
 
 New RPCs

src/rpc/net.cpp

@@ -749,9 +749,11 @@ static RPCHelpMan listbanned()
             {
                 {RPCResult::Type::OBJ, "", "",
                     {
-                        {RPCResult::Type::STR, "address", ""},
-                        {RPCResult::Type::NUM_TIME, "banned_until", ""},
-                        {RPCResult::Type::NUM_TIME, "ban_created", ""},
+                        {RPCResult::Type::STR, "address", "The IP/Subnet of the banned node"},
+                        {RPCResult::Type::NUM_TIME, "ban_created", "The " + UNIX_EPOCH_TIME + " the ban was created"},
+                        {RPCResult::Type::NUM_TIME, "banned_until", "The " + UNIX_EPOCH_TIME + " the ban expires"},
+                        {RPCResult::Type::NUM_TIME, "ban_duration", "The ban duration, in seconds"},
+                        {RPCResult::Type::NUM_TIME, "time_remaining", "The time remaining until the ban expires, in seconds"},
                     }},
             }},
                 RPCExamples{
@@ -767,15 +769,18 @@ static RPCHelpMan listbanned()
 
     banmap_t banMap;
     node.banman->GetBanned(banMap);
+    const int64_t current_time{GetTime()};
 
     UniValue bannedAddresses(UniValue::VARR);
     for (const auto& entry : banMap)
     {
         const CBanEntry& banEntry = entry.second;
         UniValue rec(UniValue::VOBJ);
         rec.pushKV("address", entry.first.ToString());
-        rec.pushKV("banned_until", banEntry.nBanUntil);
         rec.pushKV("ban_created", banEntry.nCreateTime);
+        rec.pushKV("banned_until", banEntry.nBanUntil);
+        rec.pushKV("ban_duration", (banEntry.nBanUntil - banEntry.nCreateTime));
+        rec.pushKV("time_remaining", (banEntry.nBanUntil - current_time));
 
         bannedAddresses.push_back(rec);
     }

src/test/rpc_tests.cpp

@@ -269,9 +269,9 @@ BOOST_AUTO_TEST_CASE(rpc_ban)
     ar = r.get_array();
     o1 = ar[0].get_obj();
     adr = find_value(o1, "address");
-    UniValue banned_until = find_value(o1, "banned_until");
+    int64_t banned_until{find_value(o1, "banned_until").get_int64()};
     BOOST_CHECK_EQUAL(adr.get_str(), "127.0.0.0/24");
-    BOOST_CHECK_EQUAL(banned_until.get_int64(), 9907731200); // absolute time check
+    BOOST_CHECK_EQUAL(banned_until, 9907731200); // absolute time check
 
     BOOST_CHECK_NO_THROW(CallRPC(std::string("clearbanned")));
 
@@ -280,11 +280,16 @@ BOOST_AUTO_TEST_CASE(rpc_ban)
     ar = r.get_array();
     o1 = ar[0].get_obj();
     adr = find_value(o1, "address");
-    banned_until = find_value(o1, "banned_until");
+    banned_until = find_value(o1, "banned_until").get_int64();
+    const int64_t ban_created{find_value(o1, "ban_created").get_int64()};
+    const int64_t ban_duration{find_value(o1, "ban_duration").get_int64()};
+    const int64_t time_remaining{find_value(o1, "time_remaining").get_int64()};
+    const int64_t now{GetTime()};
     BOOST_CHECK_EQUAL(adr.get_str(), "127.0.0.0/24");
-    int64_t now = GetTime();
-    BOOST_CHECK(banned_until.get_int64() > now);
-    BOOST_CHECK(banned_until.get_int64()-now <= 200);
+    BOOST_CHECK(banned_until > now);
+    BOOST_CHECK(banned_until - now <= 200);
+    BOOST_CHECK_EQUAL(ban_duration, banned_until - ban_created);
+    BOOST_CHECK_EQUAL(time_remaining, banned_until - now);
 
     // must throw an exception because 127.0.0.1 is in already banned subnet range
     BOOST_CHECK_THROW(r = CallRPC(std::string("setban 127.0.0.1 add")), std::runtime_error);