Merge bitcoin/bitcoin#26822: p2p, rpc: don't allow past absolute timestamp in `setban`

abccb27466c5a240a59f3f7c86f2bc60d51e9fee test: add coverage for absolute timestamp in `setban` (brunoerg)
b99f1f20f773eb55c870a033b3f2e8f13d55d0c8 p2p, rpc: don't allow past absolute timestamp in `setban` (brunoerg)

Pull request description:

  We shouldn't allow call `setban` with past absolute timestamp. First, because doesn't make sense to ban a node until ~ past ~. Besides that, it could make an unnecessary write to the DB since `BanMan::Ban` calls `DumpBanlist` and it calls `SweepBanned` which will remove this new ban (because of the timestamp) of the array.

ACKs for top commit:
  1440000bytes:
    ACK https://github.com/bitcoin/bitcoin/pull/26822/commits/abccb27466c5a240a59f3f7c86f2bc60d51e9fee

Tree-SHA512: 6d0cadf99fc4f78d77d3bafd6f5c85ac56e243ebc376210fdb2bee751e7b139ec7d6f5f346317fd0b10051b685f2d0ee1d8e40f4bc10f4dbdbbddd5e1ee84de5

2 files changed, 17 insertions, 2 deletions

diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index 4611e03270..d430087358 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -741,6 +741,10 @@ static RPCHelpMan setban()
 
         const bool absolute{request.params[3].isNull() ? false : request.params[3].get_bool()};
 
+        if (absolute && banTime < GetTime()) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: Absolute timestamp is in the past");
+        }
+
         if (isSubnet) {
             node.banman->Ban(subNet, banTime, absolute);
             if (node.connman) {
diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py
index 91c2a43932..7169cc8937 100755
--- a/test/functional/p2p_disconnect_ban.py
+++ b/test/functional/p2p_disconnect_ban.py
@@ -46,6 +46,9 @@ class DisconnectBanTest(BitcoinTestFramework):
         assert_raises_rpc_error(-30, "Error: Invalid IP/Subnet", self.nodes[1].setban, "127.0.0.1/42", "add")
         assert_equal(len(self.nodes[1].listbanned()), 1)  # still only one banned ip because 127.0.0.1 is within the range of 127.0.0.0/24
 
+        self.log.info("setban: fail to ban with past absolute timestamp")
+        assert_raises_rpc_error(-8, "Error: Absolute timestamp is in the past", self.nodes[1].setban, "127.27.0.1", "add", 123, True)
+
         self.log.info("setban remove: fail to unban a non-banned subnet")
         assert_raises_rpc_error(-30, "Error: Unban failed", self.nodes[1].setban, "127.0.0.1", "remove")
         assert_equal(len(self.nodes[1].listbanned()), 1)
@@ -66,9 +69,13 @@ class DisconnectBanTest(BitcoinTestFramework):
         self.nodes[1].setban("2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19", "add", 1000)  # ban for 1000 seconds
         listBeforeShutdown = self.nodes[1].listbanned()
         assert_equal("192.168.0.1/32", listBeforeShutdown[2]['address'])
+
+        self.log.info("setban: test banning with absolute timestamp")
+        self.nodes[1].setban("192.168.0.2", "add", old_time + 120, True)
+
         # Move time forward by 3 seconds so the third ban has expired
         self.nodes[1].setmocktime(old_time + 3)
-        assert_equal(len(self.nodes[1].listbanned()), 3)
+        assert_equal(len(self.nodes[1].listbanned()), 4)
 
         self.log.info("Test ban_duration and time_remaining")
         for ban in self.nodes[1].listbanned():
@@ -78,13 +85,17 @@ class DisconnectBanTest(BitcoinTestFramework):
             elif ban["address"] == "2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19":
                 assert_equal(ban["ban_duration"], 1000)
                 assert_equal(ban["time_remaining"], 997)
+            elif ban["address"] == "192.168.0.2/32":
+                assert_equal(ban["ban_duration"], 120)
+                assert_equal(ban["time_remaining"], 117)
 
         self.restart_node(1)
 
         listAfterShutdown = self.nodes[1].listbanned()
         assert_equal("127.0.0.0/24", listAfterShutdown[0]['address'])
         assert_equal("127.0.0.0/32", listAfterShutdown[1]['address'])
-        assert_equal("/19" in listAfterShutdown[2]['address'], True)
+        assert_equal("192.168.0.2/32", listAfterShutdown[2]['address'])
+        assert_equal("/19" in listAfterShutdown[3]['address'], True)
 
         # Clear ban lists
         self.nodes[1].clearbanned()