feat(wire): implement BanMan for centralized ban management by Micah-Shallom · Pull Request #892 · getfloresta/Floresta

Micah-Shallom · 2026-03-14T14:14:47Z

Summary

Add a BanMan struct to track banned IPs in one place instead of scattering ban state across AddressMan and peer_man. Banned IPs are now checked before opening connections.

This covers single-IP banning for #820. Subnet support, RPC endpoints, and persistence will follow in separate PRs.

Test plan

cargo test -p floresta-wire — all tests pass (6 new unit tests for BanMan)
cargo clippy --all-targets --all-features -- -D warnings — clean
cargo +nightly fmt — clean

Davidson-Souza · 2026-03-16T17:47:41Z

Bitcoin Core's banman also allows to ban a whole subnet (like 127.255.255.255/8). We can't really add one entry per address in this range (for the previous example that would be millions of addresses)

Micah-Shallom · 2026-03-16T23:10:42Z

Thanks for the feedback @Davidson-Souza ! I originally planned CIDR/subnet support as a follow-up PR, that way I could keep this one focused on the core BanMan strcuture and integration. Would you prefer i add subnet support in this PR??

Davidson-Souza · 2026-03-16T23:18:58Z

Thanks for the feedback @Davidson-Souza ! I originally planned CIDR/subnet support as a follow-up PR, that way I could keep this one focused on the core BanMan strcuture and integration. Would you prefer i add subnet support in this PR??

I think it's fine to do in another PR, but would be nice to have the follow-up as a draft PR. That way we could assess whether the current approach is in the right direction for that. See #836 and #837 as an example.

Micah-Shallom · 2026-03-17T03:02:59Z

Done @Davidson-Souza .... I just opened #899 as the draft PR for this... Looked and saw that Bitcoin Core has a CSubNet::Match in netaddress.cpp and so with the help of LLM i ported the bitwise AND matching to Rust. Single IPs are just /32 or /128 subnets so nothing breaks.

jaoleal · 2026-03-18T22:20:32Z

crates/floresta-wire/src/p2p_wire/ban_man.rs

+
+    /// Bans an IP address for `duration` seconds from now
+    ///
+    /// If `duration` is 0, the default ban time (24 hours) will be use


Suggested change

/// If `duration` is 0, the default ban time (24 hours) will be use

/// If `duration` is 0, the default ban time (24 hours) will be used instead.

jaoleal · 2026-03-18T22:20:57Z

crates/floresta-wire/src/p2p_wire/ban_man.rs

+}
+
+impl BanMan {
+    /// Creates a new empty Banman


Suggested change

/// Creates a new empty Banman

/// Creates a new empty `Banman`.

jaoleal · 2026-03-18T22:25:37Z

crates/floresta-wire/src/p2p_wire/ban_man.rs

+/// Centralized manager for tracking banned IP addresses.
+#[derive(Debug, Default)]
+pub struct BanMan {
+    banned: HashMap<IpAddr, u64>,


I think is good to document somewhere here in this module that is expected for this u64 to be an absolute timestamp that such a peer is banned

jaoleal · 2026-03-19T15:27:29Z

crates/floresta-wire/src/p2p_wire/node/conn.rs

+        let net_address = address.get_net_address();
+        if self.ban_man.is_banned(net_address) {
+            return Err(WireError::PeerBanned(net_address));
+        }
+


I think theres a better place for this check to live, this create_connection method is called to retrieve a random peer from the addrman that matches the required services, as it is here, this check would never be triggered because get_address_to_connect doesnt return banned peers.

Maybe would be better to place a periodic run that given the list on banman, it updates or creates a banned peer inside the peers list.

Maybe the boys can point you a better place to do that but I suggest for it to live on rearrange_buckets or similar.

thank you for your suggestions @jaoleal

i wasnt aware of the rearrange_buckets method before now...but then looking at it , i noticed there would be a problem... banman and addressman both track banned but do not talk to each other at all...

this means we will need to decide and based on what i think, banman should be the single source of truth for bans while addressman doesnt need to know about bans at all...but then making addressman fully unaware of bans would be a very large refactor..one of this refactor will see AddressState::banned becoming redundant

also...if banman is to be adopted as the main source of truth for banned addresses then i'd say checking for banned addresses at create_connection() wont be all that bad but this check will have to be inside the get_address_to_connect method

well these are what i think 🙂🙂....will like to know what you think @jaoleal @Davidson-Souza @luisschwab

Okay, after doing some analysis on the current ban structure, heres what I would do:

IMHO, yes, a small refactor is needed so both AddressMan and BanMan be in sync without more redundancies.

rearrange_buckets is a perfectly place to live a sync between those, it runs every 1 hour and would do the job to periodically unban peers, BanMan could export a list of banned peers so AddressMan catch up.

disconnect_and_ban would be the entry to ban such a peer, triggering immediately an new addition on both AddressMan and BanMan.

I think that the rest of the jobs is getting rid of AddressState::Banned (i think its fine since BanMan will now track which peers are banned and when to unban them) and other ban pipelines that can conflict with the new approach.

WYT ?

makes sense @jaoleal ..i am much aligned with this approach.... i just a raised a stacked PR #912 that implements this... kindly check it out

Introduce a BanMan struct that centralizes IP ban tracking, replacing the scattered ban logic across AddressMan and peer_man. - Add BanMan with add_ban() and is_banned() methods - Wire BanMan into NodeCommon alongside AddressMan - Check banned status before creating new connections - Register bans in both disconnect_and_ban() and increase_banscore() - Add PeerBanned variant to WireError Closes getfloresta#820 (partial — single-IP banning only)

Micah-Shallom force-pushed the feat/wire-implement-banman branch from 2bd3e54 to 911c577 Compare March 14, 2026 14:20

Micah-Shallom mentioned this pull request Mar 17, 2026

feat(wire): add subnet/CIDR support to BanMan #899

Draft

3 tasks

luisschwab assigned Micah-Shallom Mar 17, 2026

luisschwab added the enhancement New feature or request label Mar 17, 2026

luisschwab added this to the Q2/2026 milestone Mar 17, 2026

jaoleal requested review from Davidson-Souza, jaoleal and luisschwab March 17, 2026 20:10

jaoleal reviewed Mar 19, 2026

View reviewed changes

Micah-Shallom force-pushed the feat/wire-implement-banman branch from 911c577 to 1f36ff0 Compare March 20, 2026 00:58

This was referenced Mar 22, 2026

refactor(wire): make BanMan single source of truth for bans Micah-Shallom/Floresta#1

Closed

refactor(wire): make BanMan single source of truth for bans #912

Open

csgui added reliability Related to runtime reliability, stability and production readiness and removed enhancement New feature or request labels Mar 22, 2026

Micah-Shallom requested a review from jaoleal March 22, 2026 20:59

	/// If `duration` is 0, the default ban time (24 hours) will be use
	/// If `duration` is 0, the default ban time (24 hours) will be used instead.

	/// Creates a new empty Banman
	/// Creates a new empty `Banman`.

Conversation

Micah-Shallom commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Davidson-Souza commented Mar 16, 2026

Uh oh!

Micah-Shallom commented Mar 16, 2026

Uh oh!

Davidson-Souza commented Mar 16, 2026

Uh oh!

Micah-Shallom commented Mar 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaoleal Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Micah-Shallom Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Micah-Shallom commented Mar 14, 2026 •

edited

Loading

jaoleal Mar 20, 2026 •

edited

Loading

Micah-Shallom Mar 21, 2026 •

edited

Loading