test: enable and rename IPv6 ban tests after PR #523 fixes - #537
Conversation
|
🤖 Claude Code Review Status: Complete Current Review: Found 1 minor issue: [Minor] Outdated comment on line 416 Summary: This PR successfully re-enables two test functions that were previously skipped due to bugs fixed in PR #523:
The test changes correctly reflect the implementation fixes in handlers.go where isIPOrSubnet now handles IPv6 CIDR notation correctly and handleIsBanned accepts both IP and PeerID formats. Test documentation accurately describes the previous bugs and their resolutions. |
|
Benchmark Comparison ReportBaseline: Current: Summary
All benchmark results (sec/op)
Threshold: >10% with p < 0.05 | Generated: 2026-05-22 15:05 UTC |
|
ordishs
left a comment
There was a problem hiding this comment.
LGTM. Verified the underlying fixes against services/rpc/handlers.go:
isIPOrSubnet(handlers.go:2715–2730) no longer port-strips beforeParseCIDR, so IPv6 CIDR works.handleIsBanned(handlers.go:2042–2088) callsp2pClient.IsBannedunconditionally (both IP and PeerID) and only invokes the legacy client whenisIPOrSubnetis true.
The renamed tests accurately describe current behaviour.
Suggestion (non-blocking)
There's a stale comment at test/e2e/daemon/ready/banlist_e2e_test.go:416 inside TestBanListRPCE2E that wasn't updated:
// Note: IPv6 subnet bans via RPC have a known bug - see TestBanListRPCIPv6SubnetBugIt still references the old test name and still claims the bug exists. Worth either updating to point at the renamed test, e.g.:
// IPv6 subnet bans via RPC are exercised by TestBanListRPCIPv6Subnet…or just deleting it, since the surrounding test already covers IPv6 single addresses.
Minor (optional)
TestIsBannedRPCWithPeerID only exercises the PeerID branch of handleIsBanned. The IP-input branch (which now fans out to both p2pClient and the legacy client) is only covered indirectly via TestBanListRPCE2E/listbanned. A small isbanned <ip> assertion would tighten coverage, but the test name is honest about its scope so this is not blocking.



No description provided.