Feat: add `Manual` type to `ConnectionKinds` by Davidson-Souza · Pull Request #866 · getfloresta/Floresta

Davidson-Souza · 2026-03-03T20:28:12Z

Description and Notes

All peers that are added by the user — those added with the RPC
addnode or the CLI option --connect, should have kind Manual,
rather than Regular. This exempts them from:

Max peers quota — if we already have 10 peers and the user tries
to addnode another one, we will now have 11 peers
They whitelisted and shouldn't be banned by misbehaving
They don't need to have any required service

This commit adds this new variant, and makes sure we are following
the above rules.

This is a small subset of #636, and will be useful for getting #865 unstuck (potentially can help with some edge cases as mentioned in #852 (comment))

JoseSK999 · 2026-03-03T20:35:42Z

 }

 #[derive(Debug, PartialEq, Clone, Copy)]
+/// The kind of connection we see this peer as.


Add a link to the Core counterpart, useful

Davidson-Souza · 2026-03-03T21:21:22Z

Fixed a lint error

JoseSK999 · 2026-03-04T13:00:00Z

+    /// A feeler connection is a short-lived connection used to check whether this peer is alive.
+    ///
+    /// After handshake, we ask for addresses and when we receive an answer we just disconnect,
+    /// marking this peer as alive in our address manager


Suggested change

/// marking this peer as alive in our address manager

/// marking this peer as alive in our address manager.

We don't apply punctuation very evenly. Perhaps we should add this to our coding style, and update all docs as we go (same we are doing with structs)

For single-line comments it is fine to not punctuate IMO, but for these longer docstrings better to do it

JoseSK999 · 2026-03-04T13:00:37Z

+    /// marking this peer as alive in our address manager
    Feeler,
+
+    /// A regular peer, used to send requests to and learn about transactions and blocks


Suggested change

/// A regular peer, used to send requests to and learn about transactions and blocks

/// A regular peer, used to send requests to and learn about transactions and blocks.

JoseSK999 · 2026-03-04T13:00:56Z

+    /// A regular peer, used to send requests to and learn about transactions and blocks
    Regular(ServiceFlags),
+
+    /// An extra peer specially created if our tip haven't moved for too long


Suggested change

/// An extra peer specially created if our tip haven't moved for too long

/// An extra peer specially created if our tip hasn't moved for too long.

JoseSK999 · 2026-03-04T13:03:16Z

+    /// If more than [`NodeContext::ASSUME_STALE`] seconds has passed since the
+    /// processed block. We use this to make sure we are not in a partitioned subnet,
+    /// unable to learn about new blocks


Suggested change

/// If more than [`NodeContext::ASSUME_STALE`] seconds has passed since the

/// processed block. We use this to make sure we are not in a partitioned subnet,

/// unable to learn about new blocks

/// If more than [`NodeContext::ASSUME_STALE`] seconds have passed since the

/// last processed block, we use this to make sure we are not in a partitioned subnet,

/// unable to learn about new blocks.

Davidson-Souza · 2026-03-04T15:26:23Z

Pushed b7a39cc applying minor docs fixes by @JoseSK999

JoseSK999 · 2026-03-04T16:23:43Z

    pub(crate) fn create_connection(&mut self, kind: ConnectionKind) -> Result<(), WireError> {
+        // Connection with fixed peers should be marked as `manual`, rather than `regular`
+        let connection_kind = match (self.fixed_peer.as_ref(), kind) {
+            (Some(_), ConnectionKind::Regular(_)) => ConnectionKind::Manual,
+            _ => kind,
+        };


What if we change the signature to mut kind: ConnectionKind, and mutate it to Manual if needed. To avoid having two kind variables within scope.

I think this is nice, so you can reuse the is_fixed variable at L120:

pub(crate) fn create_connection(&mut self, mut kind: ConnectionKind) -> Result<(), WireError> { let is_fixed = self.fixed_peer.is_some(); // Connection with fixed peers should be marked as `manual`, rather than `regular` if is_fixed && matches!(kind, ConnectionKind::Regular(_)) { kind = ConnectionKind::Manual; }

JoseSK999

Last nits I think

JoseSK999 · 2026-03-04T18:21:02Z

Remove line, not needed now

Wait, the is_fixed one? I think it's still relevant, we need to allow fallback for manual peers. We would also need to change peer.rs to always fallback on manual peers

You have that variable defined twice after applying my suggestion, I mean. You can remove this one.

JoseSK999 · 2026-03-04T18:23:14Z

        match kind {
            ConnectionKind::Feeler => self.last_feeler = Instant::now(),
            ConnectionKind::Regular(_) => self.last_connection = Instant::now(),
+            // Note: Crating a manual peer intentionally don't prevent us from checking our peers


Suggested change

// Note: Crating a manual peer intentionally don't prevent us from checking our peers

// Note: creating a manual peer intentionally doesn't prevent us from checking our peers

But I don't understand the point you are trying to make in this comment

We still call check_connections in the same moment we would, regardless of already having a new connection

Ahh, perhaps a rephrase to say manual peers don’t affect the check connection waiting time

Rephrased to mention the timer

JoseSK999 · 2026-03-04T18:28:42Z

+                // Connections that will continue open for as long as we are running (and the other
+                // peer don't die)


Suggested change

// Connections that will continue open for as long as we are running (and the other

// peer don't die)

// Connections expected to remain open if the peer doesn't die

JoseSK999 · 2026-03-04T18:29:51Z

+                    || matches!(p.kind, ConnectionKind::Manual);
+                p.state == PeerStatus::Ready && long_lived_connection


JoseSK999 · 2026-03-04T18:31:09Z

+                || matches!(p.kind, ConnectionKind::Manual);
+            if long_lived && p.state == PeerStatus::Ready {


JoseSK999 · 2026-03-04T18:46:40Z

@@ -137,12 +138,11 @@
        // peer and create a utreexo and CBS connection
        if !self.has_utreexo_peers() {
            if self.peer_ids.len() == 10 {


Maybe we can do this now that you touch it:

Suggested change

if self.peer_ids.len() == 10 {

if self.peer_ids.len() >= RunningNode::MAX_OUTGOING_PEERS {

Or should we use connected_peers to not count feelers?

JoseSK999 · 2026-03-04T18:47:37Z

@@ -153,12 +153,11 @@
                return Ok(());
            }
            if self.peer_ids.len() == 10 {


and here

Suggested change

if self.peer_ids.len() == 10 {

if self.peer_ids.len() >= RunningNode::MAX_OUTGOING_PEERS {

JoseSK999 · 2026-03-04T18:53:34Z

-                            info.kind = ConnectionKind::Regular(peer_info.services);
-                        });
-                    }
+                   }


Why were we updating peer info here 🤨

I think that was actually a bug 👀

If a feeler connection sent us headers, they would be promoted to Regular

jaoleal · 2026-03-04T19:59:44Z


                            // disconnect the peer with the lowest score
                            if let Some(peer) = peer_to_disconnect {
                                self.send_to_peer(peer, NodeRequest::Shutdown)?;


Just a comment:

I saw a lot of these cases where the connection kind where checked to see if a peer should be disconnected, would be good to have unified methods for the each ConnectionKind exception.

I've added some helper methods in LocalPeerView and used them. I think we can't trivially remove anything else

Davidson-Souza · 2026-03-04T22:47:59Z

Pushed a4ba5f6b355f34394a539748d555c316e8c4906d

Minor style and comments changes @JoseSK999
Used MAX_OUTGOING_PEERS instead of magic values in check_connections
Used connected_peers instead of self.peers.len() in check_connections
Added some utility methods to LocalPeerVew that wraps some matches! invocations to make the code cleaner

JoseSK999

LAST nits

JoseSK999 · 2026-03-05T17:55:38Z

        match kind {
            ConnectionKind::Feeler => self.last_feeler = Instant::now(),
            ConnectionKind::Regular(_) => self.last_connection = Instant::now(),
+            // Note: Creating a manual peer intentionally doesn't affect the `last_connection


Suggested change

// Note: Creating a manual peer intentionally doesn't affect the `last_connection

// Note: Creating a manual peer intentionally doesn't affect the `last_connection`

JoseSK999 · 2026-03-05T18:01:12Z

+    pub(crate) const fn is_long_lived(&self) -> bool {
+        matches!(self.kind, ConnectionKind::Regular(_))
+            || matches!(self.kind, ConnectionKind::Manual)
+    }


Tiny nit

Suggested change

pub(crate) const fn is_long_lived(&self) -> bool {

matches!(self.kind, ConnectionKind::Regular(_))

|| matches!(self.kind, ConnectionKind::Manual)

}

pub(crate) const fn is_long_lived(&self) -> bool {

self.is_regular_peer() || self.is_manual_peer()

}

Tiny fix pushed

JoseSK999 · 2026-03-05T18:15:49Z

Since you are at it too

Suggested change

let total_peers = self.connected_peers();

Davidson-Souza · 2026-03-05T20:56:21Z

Pushed 525cc0c fixing nits by @JoseSK999

JoseSK999

ACK 525cc0c

JoseSK999 · 2026-03-05T21:36:05Z

Note there's a case where we would disconnect the manual peer, which is when it sends an invalid block, and we call disconnect_and_ban.

But probably we want to call disconnect_and_ban inside increase_banscore tho. The latter doesn't update the address manager, so we may connect again with the banned peer. disconnect_and_ban does update the address manager which we want, I think.

EDIT: I think we can just add a return here to fix it and then in a followup merge a bit these two functions.

JoseSK999 · 2026-03-05T21:49:10Z

        if let Some(peer) = self.peers.get(&peer) {
            let ban_state = AddressState::Banned(T::BAN_TIME);


Add early return to fix it

if let Some(peer) = self.peers.get(&peer) { // Manual connections are exempt from being punished if peer.is_manual_peer() { return Ok(()); } let ban_state = AddressState::Banned(T::BAN_TIME);

Davidson-Souza · 2026-03-05T22:06:27Z

Note there's a case where we would disconnect the manual peer, which is when it sends an invalid block, and we call disconnect_and_ban.

But probably we want to call disconnect_and_ban inside increase_banscore tho. The latter doesn't update the address manager, so we may connect again with the banned peer. disconnect_and_ban does update the address manager which we want, I think.

EDIT: I think we can just add a return here to fix it and then in a followup merge a bit these two functions.

Oh shoot, right! Pushed the temporary fix, ACK on refactoring those functions.

JoseSK999

ACK e0d9a85

All peers that are added by the user — those added with the RPC `addnode` or the CLI option `--connect`, should have kind `Manual`, rather than `Regular`. This excempts them from: - Max peers quota — if we already have 10 peers and the user tries to addnode another one, we will now have 11 peers - They whitelisted and shouldn't be banned by misbehaving - They don't need to have any required service This commit adds this new variant, and makes sure we are following the above rules.

Manual connections are whitelisted and should not be banned, even if they misbehave. This commit makes sure we are not doing so. I've removed all the times we just disconnect a misbehaving peer with a call to `increase_banscore` with the maximum `banscore` as the increase amount (so they will be banned right away)

Davidson-Souza · 2026-03-06T14:58:30Z

Sorry, @JoseSK999. But I've fixed that annoying but where we would get nine peers and get stuck.

JoseSK999

re-ACK 6de9fa1

jaoleal · 2026-03-06T17:21:21Z

ACK 6de9fa1

JoseSK999 · 2026-03-06T17:27:07Z

Next on: batch connections 👨🏻‍🍳

Davidson-Souza added this to the v0.9.0 milestone Mar 3, 2026

Davidson-Souza requested a review from JoseSK999 March 3, 2026 20:28

Davidson-Souza self-assigned this Mar 3, 2026

Davidson-Souza added enhancement New feature or request code quality Generally improves code readability and maintainability refactor labels Mar 3, 2026

Davidson-Souza changed the title ~~Feat/add manual peers~~ Feat: add Manual type to ConnectionKinds Mar 3, 2026

Davidson-Souza mentioned this pull request Mar 3, 2026

0.9.0 priorities #850

Closed

20 tasks

JoseSK999 reviewed Mar 3, 2026

View reviewed changes

Davidson-Souza force-pushed the feat/add-manual-peers branch from 52d1361 to e0027b9 Compare March 3, 2026 21:05

Davidson-Souza mentioned this pull request Mar 3, 2026

feat(wire): create connections in batch #865

Merged

Davidson-Souza force-pushed the feat/add-manual-peers branch from e0027b9 to 27b5f92 Compare March 3, 2026 21:20

JoseSK999 reviewed Mar 4, 2026

View reviewed changes

Davidson-Souza force-pushed the feat/add-manual-peers branch from 27b5f92 to b7a39cc Compare March 4, 2026 15:25

JoseSK999 reviewed Mar 4, 2026

View reviewed changes

Davidson-Souza force-pushed the feat/add-manual-peers branch from b7a39cc to 2821fd5 Compare March 4, 2026 18:19

JoseSK999 reviewed Mar 4, 2026

View reviewed changes

jaoleal approved these changes Mar 4, 2026

View reviewed changes

Davidson-Souza force-pushed the feat/add-manual-peers branch 2 times, most recently from 2459abc to a4ba5f6 Compare March 4, 2026 22:44

JoseSK999 reviewed Mar 5, 2026

View reviewed changes

Davidson-Souza force-pushed the feat/add-manual-peers branch from a4ba5f6 to 525cc0c Compare March 5, 2026 20:55

Davidson-Souza requested review from JoseSK999 and jaoleal March 5, 2026 20:56

JoseSK999 approved these changes Mar 5, 2026

View reviewed changes

JoseSK999 reviewed Mar 5, 2026

View reviewed changes

Davidson-Souza force-pushed the feat/add-manual-peers branch from 525cc0c to e0d9a85 Compare March 5, 2026 22:04

JoseSK999 approved these changes Mar 5, 2026

View reviewed changes

Davidson-Souza added 2 commits March 6, 2026 11:36

Davidson-Souza force-pushed the feat/add-manual-peers branch from e0d9a85 to 6de9fa1 Compare March 6, 2026 14:57

JoseSK999 approved these changes Mar 6, 2026

View reviewed changes

Davidson-Souza merged commit fe045d8 into getfloresta:master Mar 6, 2026
11 checks passed

csgui added this to Floresta Mar 12, 2026

github-project-automation Bot moved this to Done in Floresta Mar 12, 2026

csgui added reliability Related to runtime reliability, stability and production readiness and removed code quality Generally improves code readability and maintainability labels Mar 12, 2026

csgui mentioned this pull request Mar 13, 2026

[Enhancement] Do not automatically disconnect or add (manual) peer to our discouragement filter #646

Closed

	/// marking this peer as alive in our address manager
	/// marking this peer as alive in our address manager.

	/// A regular peer, used to send requests to and learn about transactions and blocks
	/// A regular peer, used to send requests to and learn about transactions and blocks.

	/// An extra peer specially created if our tip haven't moved for too long
	/// An extra peer specially created if our tip hasn't moved for too long.

	// Note: Crating a manual peer intentionally don't prevent us from checking our peers
	// Note: creating a manual peer intentionally doesn't prevent us from checking our peers

		// Connections that will continue open for as long as we are running (and the other
		// peer don't die)

	// Connections that will continue open for as long as we are running (and the other
	// peer don't die)
	// Connections expected to remain open if the peer doesn't die

		\|\| matches!(p.kind, ConnectionKind::Manual);
		p.state == PeerStatus::Ready && long_lived_connection

		\|\| matches!(p.kind, ConnectionKind::Manual);
		if long_lived && p.state == PeerStatus::Ready {

	if self.peer_ids.len() == 10 {
	if self.peer_ids.len() >= RunningNode::MAX_OUTGOING_PEERS {

		if let Some(peer) = self.peers.get(&peer) {
		let ban_state = AddressState::Banned(T::BAN_TIME);

Uh oh!

Conversation

Davidson-Souza commented Mar 3, 2026

Description and Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Davidson-Souza commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Davidson-Souza commented Mar 4, 2026

Uh oh!

JoseSK999 Mar 4, 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!

Choose a reason for hiding this comment

Uh oh!

JoseSK999 left a comment

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!

JoseSK999 Mar 4, 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaoleal Mar 4, 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!

Choose a reason for hiding this comment

Uh oh!

Davidson-Souza commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoseSK999 left a comment • 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!

Davidson-Souza commented Mar 3, 2026 •

edited

Loading

JoseSK999 Mar 4, 2026 •

edited

Loading

JoseSK999 Mar 4, 2026 •

edited

Loading

jaoleal Mar 4, 2026 •

edited

Loading

Davidson-Souza commented Mar 4, 2026 •

edited

Loading

JoseSK999 left a comment •

edited

Loading

JoseSK999 commented Mar 5, 2026 •

edited

Loading

JoseSK999 Mar 5, 2026 •

edited

Loading

Davidson-Souza commented Mar 6, 2026 •

edited

Loading