test: Stricter checks in rpc_setban.py
What the developer wrote
Authored by MarcoFalke
test: Stricter checks in rpc_setban.py
Make the checks stricter and easier to follow:
* Fix a typo.
* After the first ban from node 1 wait until node 0 "sees" the ban.
* Move the restart_node out of the debug log context, to avoid bloat.
* Removed the timeout from the outer/lower exit stack to check "dropped
(banned)\n" on node 1, because the inner/top exit stack waits longer.
* The inner/top exit stack checks for the both disconnections peer=2 and
possibly peer=3 (for v2->v1 retry).
* And finally, add a redundant assert to confirm once more that node 0
is has "seen" the ban.
What changed, and why it matters
This commit only changes a test file that exercises the setban RPC. It makes the existing test stricter and easier to read by fixing a typo, adding waits for disconnections, moving a node restart outside a debug-log context, and adding an extra assertion. There is no change to production code, so it does not introduce or fix a security vulnerability in Bitcoin Core itself.
No security action required. This is a test-only quality improvement. Reviewers may optionally verify that the new assertions correctly cover the intended ban/reconnect behavior.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff modifies test/functional/rpc_setban.py. It tightens synchronization around banning behavior: it waits until node 0 observes the ban, moves restart_node out of the assert_debug_log context, removes an outer timeout in favor of a longer inner timeout, checks for both peer=2 and peer=3 disconnects when v2 transport retries with v1, and adds a final assert that node 0 is disconnected. No consensus, networking, or RPC production code is touched.
Changed components
test/functional/rpc_setban.pyInspect captured patch +15 / −6
diff --git a/test/functional/rpc_setban.py b/test/functional/rpc_setban.py
index 9b684ae4..7d1873c7 100755
--- a/test/functional/rpc_setban.py
+++ b/test/functional/rpc_setban.py
@@ -26,19 +26,28 @@ class SetBanTests(BitcoinTestFramework):
peerinfo = self.nodes[1].getpeerinfo()[0]
assert "noban" not in peerinfo["permissions"]
- # Node 0 get banned by Node 1
+ # Node 0 gets banned by Node 1
self.nodes[1].setban("127.0.0.1", "add")
+ self.wait_until(lambda: not self.nodes[0].is_connected_to(self.nodes[1]))
# Node 0 should not be able to reconnect
+ self.restart_node(1, [])
context = ExitStack()
- context.enter_context(self.nodes[1].assert_debug_log(expected_msgs=['dropped (banned)\n'], timeout=50))
+ context.enter_context(self.nodes[1].assert_debug_log(expected_msgs=["dropped (banned)\n"]))
# When disconnected right after connecting, a v2 node will attempt to reconnect with v1.
- # Wait for that to happen so that it cannot mess with later tests.
- if self.options.v2transport:
- context.enter_context(self.nodes[0].assert_debug_log(expected_msgs=['trying v1 connection'], timeout=50))
+ # Wait for all disconnects on node0, so that it cannot mess with later tests.
+ context.enter_context(self.nodes[0].assert_debug_log(
+ expected_msgs=[
+ "retrying with v1 transport protocol for peer=2",
+ "Cleared nodestate for peer=2",
+ "Cleared nodestate for peer=3",
+ ] if self.options.v2transport else [
+ "Cleared nodestate for peer=2", # Just one v1 disconnect to wait for
+ ],
+ timeout=8))
with context:
- self.restart_node(1, [])
self.nodes[0].addnode("127.0.0.1:" + str(p2p_port(1)), "onetry")
+ assert not self.nodes[0].is_connected_to(self.nodes[1])
# However, node 0 should be able to reconnect if it has noban permission
self.restart_node(1, ['-whitelist=127.0.0.1'])
Why this scored 14/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.