p2p: Increase inbound capacity for block-relay only connections by mzumsande · Pull Request #28463 · bitcoin/bitcoin

mzumsande · 2023-09-12T20:36:39Z

This is joint work with amitiuttarwar.

See issue #28462 for a broader discussion on increasing the number of block-relay-only connections independent of this particular implementation proposal.

We suggest to increase the number of inbound slots allocated to block-relay-only peers by increasing the default maximum connections from 125 to 200, with 50% of inbound slots accessible for tx-relaying peers.
This is a prerequisite for being able to increase the default number of outgoing block-relay-only peers later, because the current inbound capacity of the network is not sufficient.
In order to account for incoming tx-relaying peers separately from incoming block-relay peers, changes to the inbound eviction logic are necessary.

See the next post in this thread for a more detailed explanation and motivation of the changes.

DrahtBot · 2023-09-12T20:36:42Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/28463.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	dergoegge
Concept ACK	naumenkogs, naiyoma, 0xB10C, marcofleon, instagibbs, ajtowns, brunoerg, darosior

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35600 (net: prevent duplication manual connections (take 2) by willcl-ark)
#35591 ([DO NOT MERGE] Erlay: bandwidth-efficient transaction relay protocol (Full implementation) by sr-gi)
#31260 (scripted-diff: Type-safe settings retrieval by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

mzumsande · 2023-09-12T20:37:10Z

Status Quo

We have a default maximum of 125 connections (not counting manual ones). Out of these,
11 (8 full outbounds, 2 block-relay-only, 1 feeler) are reserved for outbounds, leaving us with
114 inbound slots. There are currently no limits on how many of the inbounds can support transaction relay.

Problem

We want to increase the number of inbound slots specifically offered to block-relay-only connections, but don't want to significantly change the number of tx-relaying inbound peers, because we don’t want to radically change the memory and traffic requirements that come with running a full node with the default configuration.
This is complicated by a timing issue: When a new peer connects to us, we don't know yet whether it is block-relay-only before we have processed their version message.
Inbound peer eviction, however, happens right after a peer connects to us and before processing its version msg. This means that we don't yet know during inbound eviction, if the new peer wants tx relay.

Approach

We propose to introduce a separate maximum of tx-relaying inbound connections, in addition to the existing global maximum.
The new maximum for tx-relaying inbounds gets chosen such that 50% of the total inbound slots can be utilized by tx-relaying peers. We choose a ratio here instead of a fixed number here so that it scales with users changing their -maxconnections value from the default. The ratio of 50% is also made adjustable in the form of a new startup parameter.

Changes to the eviction logic

We don't change the eviction logic for total connections, which happens in CreateNodeFromAcceptedSocket().
We add a tx-relay-specific eviction check to the version message processing which runs if we don't have capacity for another tx-relaying peer: This check uses the same logic from AttemptToEvictConnection, but specifically evicts only tx-relaying peers (by protecting the rest of the peers). Only if we cannot find another peer to evict (e.g. all peers are protected, which should only happen if a low -maxconnections is used) we disconnect the new peer. This approach achieves that the number of tx-relaying peers can never exceed the limit.

Numbers

Specifically, we propose to increase the total number of connections from 125 to 200, with a suggested ratio of 50% tx-relay inbounds. With 11 connection slots reserved for outbound peers,
this results in (200 - 11) * 0.5 = 94.5 slots for tx-relaying inbound peers, which closely aligns with the typical tx-relaying inbounds seen in today's world, where typically ~20% of inbound slots are already taken by block-relay-only connections.
These numbers were chosen to be sufficient to raise the number of block-relay-only connections from 2 to 8 once this patch is widely deployed (see Issue #28462 for more details on this).
[Note: In a previous iteration, a ratio of 60% was suggested, see this post for more discussion.]

Handling bloom filter peers

The logic gets a bit complicated if a node supports BIP37 (bloom filters), because then an inbound peer can be switched to tx-relay after sending a FILTERLOAD or FILTERCLEAR message (but never back!), which could result in an excess of tx-relaying inbounds. We propose to solve this by triggering the eviction logic for a tx-relaying peer from net_processing whenever we switch a peer to tx-relay due to a BIP37-related message.

naumenkogs · 2023-09-13T08:28:27Z

Concept ACK, specifically on changes to the eviction logic, and handling bloom filters.
Numbers also seem fine I guess.

I was curious whether there is (or will be) any other good reason to delay eviction after the version (apart from making this logic cleaner), but I haven't identified any.

mzumsande · 2023-09-28T16:16:00Z

I was curious whether there is (or will be) any other good reason to delay eviction after the version (apart from making this logic cleaner), but I haven't identified any.

Discussed this with @amitiuttarwar and we think that it would be better to move the tx-related eviction into the version processing logic.
Pros:

It's conceptually simpler to only evict a tx-relaying peer when we are sure that it is one
We wouldn't have to evict anyone in the situation where all tx inbound slots are full and a new block-relay inbound connects
We need the logic for this (EvictTxPeerIfFull()) anyway when dealing with bloom-filter peers.

Cons:

The eviction now happens in two different places, after accepting a connection and during version processing.

I will update the PR with this soon!

mzumsande · 2023-10-13T18:38:54Z

Changed the approach to do the eviction of tx-relaying peers within the version message processing, when we can make use of the fRelay flag they sent us - this has the advantage that we don't have to evict new peers before being sure if they want tx relay.

As a result of the later eviction, it became necessary to protect the new peer from being evicted right by AttemptToEvictConnection() right after connecting (so that the new peer only gets evicted if no other peer can be found). This isn't done for the existing unconditional inobound eviction in net (CreateNodeFromAcceptedSocket), because there we would evict a peer before adding the new peer to m_nodes.

amitiuttarwar · 2023-10-18T00:27:16Z

overall, I like this approach significantly more. although we are introducing another touchpoint for inbound eviction logic, it is more straightforward to reason about. but the biggest improvement is not unnecessarily disconnecting a new or existing connection when tx relay isn't being requested.

reviewed this in-depth. the fundamental structure & logic looks good to me. left a lot of comments along the way 😛

some additional thoughts:

commit message for ab5ad61f67f3bcab503ac3eb012c4f35d733b946 says "When we receive an inbound connection and don't have space for another full-relay peer, we now attempt to evict specifically a full-relay inbound." I think this should mention timing of processing the version message for clarity.
the order of events when processing the VERSION message makes sense, because we want to instantiate tx_relay for the before before triggering the new eviction logic (which uses that in the count).
this code strengthens the distinction between inbound-tx-relay connections from inbound-block-relay connections. ideally, this would be captured in separate connection types, but that would require more invasive changes since we don't know the distinction at the time of accepting the connection. I don't think the benefit would be worth the cost, but is something worth keeping in mind if we continue to build stronger distinctions between the two types of inbounds.

amitiuttarwar · 2023-10-17T20:47:34Z

+     *                                a peer to evict (all eligible peers are protected)
+     *                                so that the caller can deal with this.
+     */
+    bool EvictTxPeerIfFull(std::optional<NodeId> protect_peer);


I wonder if it could be clearer to call this function something like TxInboundCapacityAvailable. imo that seems more useful from the POV of the net processing caller, but either is ok with me.

default param here makes the FILTERLOAD/FILTERCLEAR callers slightly cleaner

Suggested change

bool EvictTxPeerIfFull(std::optional<NodeId> protect_peer);

bool EvictTxPeerIfFull(std::optional<NodeId> protect_peer) = std::nullopt;

changed to use default values.

Not sure about the rename suggestion, I think I prefer EvictTxPeerIfFull, because I think for naming describing what the function does is more important than describing the return value. Otherwise other spots might call this function in the future with the intention to just perform a check, and then might be surprised about the side effects.

and then might be surprised about the side effects.

yeah totally. I was trying to make the return values more self-evident, but on further thought, I agree with you that emphasizing eviction is more important - people can always read the doc string to understand the return value. can't think of anything to concisely express HasOrMakeInboundTxCapacity, so leaving as is sounds good :)

mzumsande · 2023-10-19T21:52:49Z

Thanks for the review @amitiuttarwar! i pushed an update, addressing all feedback.

df69b22 doc: improve documentation around connection limit maximums (Amiti Uttarwar) adc171e scripted-diff: Rename connection limit variables (Amiti Uttarwar) e9fd9c0 net: add m_max_inbound to connman (Amiti Uttarwar) c25e0e0 net, refactor: move calculations for connection type limits into connman (Amiti Uttarwar) Pull request description: This is joint work with amitiuttarwar. This has the first few commits of bitcoin#28463. It is not strictly a prerequisite for that, but has changes that in our opinion make sense on their own. It improves the handling of maximum numbers for different connection types (that are set during init and don’t change after) by: * moving all calculations into one place, `CConnMan::Init()`. Before, they were dispersed between `Init`, `CConnman::Init` and other parts of `CConnman`, resulting in some duplicated test code. * removing the possibility of having a negative maximum of inbound connections, which is hard to argue about * renaming of variables and doc improvements ACKs for top commit: amitiuttarwar: co-author review ACK df69b22 naumenkogs: ACK df69b22 achow101: ACK df69b22 Tree-SHA512: 913d56136bc1df739978de50db67302f88bac2a9d34748ae96763288d97093e998fc0f94f9b6eff12867712d7e86225af6128f4170bf2b5b8ab76f024870a22c

marcofleon

Concept ACK

  Based on the discussion in #34542, there seems to be some general agreement that increasing block-relay-only connections is a good first step toward improving partition resistance. I think it makes sense on its own and it would also lay the groundwork for potentially adding reconciliation-based tx relay connections later on.

  I’ll start running fuzzamoto and other relevant fuzz tests on this PR.

marcofleon · 2026-03-05T15:58:35Z

            }
            pfrom.m_bloom_filter_loaded = true;
            pfrom.m_relays_txs = true;
+            if (pfrom.IsInboundConn() && !m_connman.EvictTxPeerIfFull()) {


Is it intentional that we don't protect this new tx-relay peer like we do for the VERSION message in acfecb2? I guess this peer would have some history so it would make sense to consider it for eviction?

I think it makes sense: the logic in the VERSION message follows the existing logic of rather replacing a peer than denying new peers if full (which helps against eclipse attacks).
I don't think that this is important for peers trying to get filters from us - those are likely to be only light clients anyway, and we don't gain much from that connection, so I don't think they need extra protection.

instagibbs · 2026-03-05T16:34:59Z

concept ACK. I would like some of the erlay discussion to move forward too, because I'm not sure how to judge this all holistically.

ajtowns · 2026-03-05T16:59:39Z

Concept ACK.

brunoerg · 2026-03-17T12:50:54Z

Concept ACK

brunoerg · 2026-03-17T13:17:57Z

+        }
+    }
+    if (tx_inbound_peers > m_max_inbound_full_relay) {
+        return AttemptToEvictConnection(/*evict_tx_relay_peer=*/true, protect_peer);


I ran mutation testing for this PR and noticed that the following mutant isn't killed by any test. See:

diff --git a/src/net.cpp b/src/net.cpp index 499312fbbd..88afc05216 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2504,7 +2504,7 @@ bool CConnman::EvictTxPeerIfFull(std::optional<NodeId> protect_peer) } } if (tx_inbound_peers > m_max_inbound_full_relay) { - return AttemptToEvictConnection(/*evict_tx_relay_peer=*/true, protect_peer); + return AttemptToEvictConnection(/*evict_tx_relay_peer=*/false, protect_peer); } :

When evict_tx_relay_peer is true, it means that whether the node is over its tx-relay inbound limit, it must evict an existing tx-relay peer, not a block-relay-only peer. So, a test case to address it would be great. Suggestion (did not fully tested it):

diff --git a/test/functional/p2p_connection_limits.py b/test/functional/p2p_connection_limits.py index 64dab1089e..56c1d81980 100755 --- a/test/functional/p2p_connection_limits.py +++ b/test/functional/p2p_connection_limits.py @@ -33,6 +33,22 @@ class P2PConnectionLimits(BitcoinTestFramework): no_txrelay_version_msg.relay = 0 return no_txrelay_version_msg + def create_no_services_blocks_only_version(self): + """VERSION with relay=0 and nServices=0. + version_msg = msg_version() + version_msg.nVersion = P2P_VERSION + version_msg.strSubVer = P2P_SUBVERSION + version_msg.nServices = 0 + version_msg.relay = 0 + return version_msg + def test_inbound_limits(self): node = self.nodes[0] @@ -81,6 +97,18 @@ class P2PConnectionLimits(BitcoinTestFramework): node.add_p2p_connection(P2PInterface()) self.wait_until(lambda: len(node.getpeerinfo()) == 2) + self.log.info('Test that EvictTxPeerIfFull only evicts tx-relaying peers') + NUM_BLOCK_RELAY_PEERS = 21 + self.restart_node(0, ['-maxconnections=33', '-inboundrelaypercent=0']) + for _ in range(NUM_BLOCK_RELAY_PEERS): + p = self.nodes[0].add_p2p_connection(P2PInterface(), send_version=False, wait_for_verack=False) + p.send_without_ping(self.create_no_services_blocks_only_version()) + self.wait_until(lambda: len(node.getpeerinfo()) == NUM_BLOCK_RELAY_PEERS) + + with node.assert_debug_log(['failed to find a tx-relaying eviction candidate - connection dropped'], timeout=5): + self.nodes[0].add_p2p_connection(P2PInterface(), expect_success=False, wait_for_verack=False) + self.wait_until(lambda: len(node.getpeerinfo()) == NUM_BLOCK_RELAY_PEERS) + if __name__ == '__main__': P2PConnectionLimits(__file__).main()

Thanks, good find. I added a commit with your suggested test, just adding a short comment why nServices=0.

mzumsande · 2026-05-04T14:16:08Z

cfa72f8 to 0e5c40e: just rebased, will address other feedback soon

DrahtBot · 2026-05-04T15:03:24Z

🚧 At least one of the CI tasks failed.
_{Task i686, no IPC: https://github.com/bitcoin/bitcoin/actions/runs/25324068645/job/74240170554}
_{LLM reason (✨ experimental): CI failed because ctest’s sock_tests aborted with fatal checks where socket operations returned an invalid socket (s == -1).}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

n1bor · 2026-05-13T10:38:58Z

I have a node with a lot of inbound connections if you want help testing...

ipv4 ipv6 onion total block
in 395 117 16 528
out 7 2 1 10 2
total 402 119 17 538

obviously each time I restart it I lose them all but normally bounces back after a few days.
But should be able to test the 200 limit and that only 94 are for tx nodes.
Let me know what log parameters you would like run with and will deploy this build for a few weeks.

0xB10C · 2026-05-13T17:42:32Z

I ran a previous version of this branch, but probably best to do that again now nearly two years later.

I've been running two nodes with this PR (mzumsande@cfa72f8) for a while. Running this

bitcoin-cli getpeerinfo | jq 'group_by(.connection_type + "/" + (.relaytxes|tostring)) | map({key: .[0].connection_type + " / relaytxes=" + (.[0].relaytxes|tostring), count: length})'

Node charlie (149 connections total):

[
  {
    "key": "block-relay-only / relaytxes=false",
    "count": 2
  },
  {
    "key": "inbound / relaytxes=false",
    "count": 45
  },
  {
    "key": "inbound / relaytxes=true",
    "count": 94
  },
  {
    "key": "outbound-full-relay / relaytxes=true",
    "count": 8
  }
]

Node ian (146 connections total)

[
  {
    "key": "block-relay-only / relaytxes=false",
    "count": 2
  },
  {
    "key": "inbound / relaytxes=false",
    "count": 42
  },
  {
    "key": "inbound / relaytxes=true",
    "count": 94
  },
  {
    "key": "outbound-full-relay / relaytxes=true",
    "count": 8
  }
]

Compared to a node bob that does not run this patch (123 total connections):

[
  {
    "key": "block-relay-only / relaytxes=false",
    "count": 2
  },
  {
    "key": "inbound / relaytxes=false",
    "count": 26
  },
  {
    "key": "inbound / relaytxes=true",
    "count": 87
  },
  {
    "key": "outbound-full-relay / relaytxes=true",
    "count": 8
  }
]

instagibbs · 2026-05-15T18:39:29Z

will review actively once feedback is addressed, as promised :)

darosior · 2026-05-15T21:24:08Z

Concept ACK

mzumsande · 2026-05-21T07:48:18Z

0e5c40e to 6b2fda0: added test by @brunoerg
6b2fda0 to c3b3c7f: rebased

Thanks for the reviews and sorry for the delay. I think I have addressed all feedback now!

I have a node with a lot of inbound connections if you want help testing...

Thanks! Since @0xB10C has been running this for some time now and (to my knowledge) no issues have come up, there is nothing specific that needs testing - though just runing a node with this branch and reporting anything strange will help!

mzumsande · 2026-05-21T07:59:52Z

I would like some of the erlay discussion to move forward too, because I'm not sure how to judge this all holistically.

My thinking is that Erlay and this PR are two different ways of increasing connectivity, and we should probably only do one at a time, even if they are technically not exclusive.

With Erlay we would also need to increase -maxconnections for a few releases before being able to actually add the additional Erlay connections. If we go for more block-relay-only connections instead we would need to do this PR for a few releases and then bump that number.

Erlay connections have the additional benefit of also helping against tx censorship, whereas this PR only strengthens against partitioning/eclipse attacks, so Erlay does more. The downside is the added complexity of Erlay and the added traffic / reconciliation efforts.

instagibbs · 2026-05-21T12:58:04Z

@mzumsande we talked offline and the next step for erlay is seeing what reconciliation-only connections would look like. We can move on with this effort for now.

instagibbs

initial comments as I dig in a bit

instagibbs · 2026-05-21T13:07:27Z

+     * @param[in] protect_peer        Protect peer with node id
+     * @return                        True if a node was marked for disconnect
+     */
+    bool AttemptToEvictConnection(bool evict_tx_relay_peer = false, std::optional<NodeId> protect_peer = std::nullopt) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex);


e40d62a

asking only: might be worth at least making the first arg non-optional?

I did that. I also renamed it to evict_tx_relay_peer_only so that it becomes clear that if set to false, a tx peer may still be evicted.

instagibbs · 2026-05-21T19:47:10Z

        }

+        // If we have too many tx-relaying inbound peers, attempt to evict an existing one.
+        // Only if this fails, disconnect this peer.


5cdc641

Given this new callsite which may also result in a AttemptToEvictConnection, could one connection cause two disconnects? Perhaps disconnect a blocksonly on the first one, the a tx relay on second? Trying to understand what's possible.

Yes, I think that this would be possible. I'm not sure if it's much of a problem though.
Since we don't know whether the peer wants tx relay in CreateNodeFromAcceptedSocket, I'm not sure how we could avoid it.

instagibbs · 2026-05-21T19:57:48Z

@@ -561,7 +561,11 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
    argsman.AddArg("-forcednsseed", strprintf("Always query for peer addresses via DNS lookup (default: %u)", DEFAULT_FORCEDNSSEED), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
    argsman.AddArg("-listen", strprintf("Accept connections from outside (default: %u if no -proxy, -connect or -maxconnections=0)", DEFAULT_LISTEN), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
    argsman.AddArg("-listenonion", strprintf("Automatically create Tor onion service (default: %d)", DEFAULT_LISTEN_ONION), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);


5cdc641

in commit message: "As a result, the tx-related maximum traffic should not change
drastically." is there a particular reason we can't shoot for parity?

With "parity", do you mean choose parameters such that the maximum tx-related traffic is exactly the same as now?

We could do that, but I think having "nice" numbers such as 200 max connections and FULL_RELAY_INBOUND_PCT=50 is more useful.
I am already a bit worried that changing the long-existing number of 125 maximum connections to a mix of two maxima might confuse users. It would be nice if at least the new values would be something humans can remember.

instagibbs · 2026-05-21T20:03:28Z

        m_max_outbound_block_relay = std::min(MAX_BLOCK_RELAY_ONLY_CONNECTIONS, m_max_automatic_connections - m_max_outbound_full_relay);
        m_max_automatic_outbound = m_max_outbound_full_relay + m_max_outbound_block_relay + m_max_feeler;
        m_max_inbound = std::max(0, m_max_automatic_connections - m_max_automatic_outbound);
+        m_max_inbound_full_relay = std::max(0, static_cast<int>(FULL_RELAY_INBOUND_PCT / 100.0 * m_max_inbound));


5cdc641

IIUC at low "maxconnections" values, the int math flooring could result in having no inbound when the argument would otherwise suggest it could have some. Not saying this is worth "fixing" since people generally don't enable listening and reduce the number drastically, just noting

If the added bool evict_tx_relay_peer_only is set, non-tx-relay peers will be exempt from disconnection. Also allow to specify a peer that exempt from eviction. Both options will be used in later commits. Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>

..and adjust the eviction logic. The new default max connection number is 200, the default maximum of tx-relaying inbounds is limited to 50% of all inbound connections. With 11 outbound connections, that is (200 - 11) * 0.5 = 94.5. As a result, the tx-related maximum traffic should not change drastically. When we receive an inbound connection and don't have space for another full-relay peer, we now attempt to evict specifically a full-relay inbound after receiving the version message of the new peer. Once this commit is widely deployed, the added inbound capacity will allow us to increase the number of outgoing block-relay-only connections. Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>

Co-authored-by: Martin Zumsande <mzumsande@gmail.com>

Permit users to change the amount of inbounds that are permitted to relay transactions. This is particularly relevant to ensure that superusers that are not concerned with resource usage are not artificially restricted from offering many transaction relay slots to the network. Co-authored-by: Martin Zumsande <mzumsande@gmail.com>

… a peer to tx relay Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>

Co-authored-by: Martin Zumsande <mzumsande@gmail.com>

0xB10C · 2026-06-23T08:31:44Z

I've been running dbdddbc (from June 4th) for a few weeks and switching the nodes to something else for now (#35465 (comment)). Some numbers, similar to #28463 (comment):

On charlie (130 connections total):

[
  {
    "key": "block-relay-only / relaytxes=false",
    "count": 2
  },
  {
    "key": "inbound / relaytxes=false",
    "count": 26
  },
  {
    "key": "inbound / relaytxes=true",
    "count": 94
  },
  {
    "key": "outbound-full-relay / relaytxes=true",
    "count": 8
  }
]

On node ian (167 connections total):

[
  {
    "key": "block-relay-only / relaytxes=false",
    "count": 2
  },
  {
    "key": "inbound / relaytxes=false",
    "count": 63
  },
  {
    "key": "inbound / relaytxes=true",
    "count": 94
  },
  {
    "key": "outbound-full-relay / relaytxes=true",
    "count": 8
  }
]

marcofleon · 2026-06-26T14:39:32Z

    {
        ServiceFlags m_local_services = NODE_NONE;
        int m_max_automatic_connections = 0;
+        int m_full_relay_inbound_percent = 0;


I think for fuzz tests like p2p_handshake and process_message that initialize connman with the testing setup defaults, initializing to 0 for both max connections and the inbound percentage could be an issue?

The tests add peers to connman.m_nodes before doing a version handshake, which now calls EvictTxPeerIfFull(). Wouldn't it just disconnect a test inbound peer with m_relay_txs=true right away?

If they do need to be set to 0 here, then maybe we should override them in TestingSetup in setup_common.cpp.

Also curious

dergoegge

Code review ACK c2437c5

instagibbs

reviewed through c2437c5

instagibbs · 2026-07-02T11:31:11Z

+    {
+        LOCK(m_nodes_mutex);
+        for (const CNode* pnode : m_nodes) {
+            if (pnode->IsInboundConn() && pnode->m_relays_txs) {


Match the other functions like GetExtraFullOutboundCount/GetExtraBlockRelayCount?

Suggested change

if (pnode->IsInboundConn() && pnode->m_relays_txs) {

if (pnode->IsInboundConn() && !pnode->fDisconnec &&pnode->m_relays_txs) {

instagibbs · 2026-07-02T11:33:25Z

    {
        ServiceFlags m_local_services = NODE_NONE;
        int m_max_automatic_connections = 0;
+        int m_full_relay_inbound_percent = 0;


Also curious

instagibbs · 2026-07-02T11:38:53Z

-    argsman.AddArg("-maxconnections=<n>", strprintf("Maintain at most <n> automatic connections to peers (default: %u). This limit does not apply to connections manually added via -addnode or the addnode RPC, which have a separate limit of %u. It does not apply to short-lived private broadcast connections either, which have a separate limit of %u.", DEFAULT_MAX_PEER_CONNECTIONS, MAX_ADDNODE_CONNECTIONS, MAX_PRIVATE_BROADCAST_CONNECTIONS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
+    argsman.AddArg("-maxconnections=<n>", strprintf("Maintain at most <n> automatic connections to peers (default: %u). %u slots of these are reserved for outgoing connections. See -inboundrelaypercent for more information about limits applied to transaction relay inbound peers. "
+                                                    "This limit does not apply to connections manually added via -addnode or the addnode RPC, which have a separate limit of %u."
+                                                    "It does not apply to short-lived private broadcast connections either, which have a separate limit of %u.",


Suggested change

"It does not apply to short-lived private broadcast connections either, which have a separate limit of %u.",

"It does not apply to short-lived private broadcast connections either, which have a separate limit of %u. ",

instagibbs · 2026-07-02T12:40:13Z


+        // If we have too many tx-relaying inbound peers, attempt to evict an existing one.
+        // Only if this fails, disconnect this peer.
+        if (pfrom.IsInboundConn() && pfrom.m_relays_txs) {


might be confused but: If we're running in blocksonly mode, should we be dropping inbounds because they say it's ok to announce txs to them (but we won't)

instagibbs · 2026-07-02T13:03:04Z

        pfrom.m_bloom_filter_loaded = false;
        pfrom.m_relays_txs = true;
+        if (pfrom.IsInboundConn() && !m_connman.EvictTxPeerIfFull()) {
+            // We don't have room for another tx-relay peer, disconnect


nit: could these 3 locations where we're filtering get a helper function to increase grep-ability? ala MaybeDisconnectForTxRelayCapacity?

DrahtBot added the P2P label Sep 12, 2023

mzumsande marked this pull request as draft September 12, 2023 20:36

amitiuttarwar mentioned this pull request Sep 12, 2023

Increase # of block-relay-only connections #28462

Open

mzumsande mentioned this pull request Sep 12, 2023

net: improve max-connection limits code #28464

Merged

This was referenced Sep 18, 2023

test/BIP324: functional tests for v2 P2P encryption #24748

Merged

test: Extend stale_tip_peer_management test #23352

Closed

DrahtBot added the Needs rebase label Oct 3, 2023

mzumsande force-pushed the 202308_increase_block_relay branch from f78a0ed to 039ac99 Compare October 3, 2023 19:08

DrahtBot removed the Needs rebase label Oct 3, 2023

DrahtBot mentioned this pull request Oct 11, 2023

p2p: peer connection bug fixes #28248

Closed

mzumsande force-pushed the 202308_increase_block_relay branch from 039ac99 to 82ed8a5 Compare October 13, 2023 18:35

mzumsande force-pushed the 202308_increase_block_relay branch from 82ed8a5 to 8247eaa Compare October 13, 2023 18:46

DrahtBot added CI failed and removed CI failed labels Oct 13, 2023

amitiuttarwar reviewed Oct 18, 2023

View reviewed changes

mzumsande force-pushed the 202308_increase_block_relay branch from 8247eaa to 9555a33 Compare October 19, 2023 21:42

marcofleon reviewed Mar 5, 2026

View reviewed changes

brunoerg reviewed Mar 17, 2026

View reviewed changes

DrahtBot mentioned this pull request May 6, 2026

BIP 434 Support: Peer feature negotiation #35221

Merged

instagibbs reviewed May 21, 2026

View reviewed changes

mzumsande and others added 7 commits June 15, 2026 18:41

test: add functional test for inbound maxconnection limits

aa53900

Co-authored-by: Martin Zumsande <mzumsande@gmail.com>

p2p: trigger possible eviction if we support bloom filters and change…

756f67c

… a peer to tx relay Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>

test: add test that EvictTxPeerIfFull only evicts tx-relaying peers

2b15410

Co-authored-by: Martin Zumsande <mzumsande@gmail.com>

doc: Update docs that refer to -maxconnections

c2437c5

This was referenced Jun 23, 2026

[DO NOT MERGE] Erlay: bandwidth-efficient transaction relay protocol (Full implementation) #35591

Open

net: prevent duplication manual connections (take 2) #35600

Open

marcofleon reviewed Jun 26, 2026

View reviewed changes

dergoegge approved these changes Jul 1, 2026

View reviewed changes

instagibbs reviewed Jul 2, 2026

View reviewed changes

	bool EvictTxPeerIfFull(std::optional<NodeId> protect_peer);
	bool EvictTxPeerIfFull(std::optional<NodeId> protect_peer) = std::nullopt;

	if (pnode->IsInboundConn() && pnode->m_relays_txs) {
	if (pnode->IsInboundConn() && !pnode->fDisconnec &&pnode->m_relays_txs) {

	"It does not apply to short-lived private broadcast connections either, which have a separate limit of %u.",
	"It does not apply to short-lived private broadcast connections either, which have a separate limit of %u. ",

Uh oh!

Conversation

mzumsande commented Sep 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Sep 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

mzumsande commented Sep 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Status Quo

Problem

Approach

Changes to the eviction logic

Numbers

Handling bloom filter peers

Uh oh!

naumenkogs commented Sep 13, 2023

Uh oh!

mzumsande commented Sep 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mzumsande commented Oct 13, 2023

Uh oh!

amitiuttarwar commented Oct 18, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

mzumsande commented Oct 19, 2023

Uh oh!

marcofleon 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!

instagibbs commented Mar 5, 2026

Uh oh!

ajtowns commented Mar 5, 2026

Uh oh!

brunoerg commented Mar 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mzumsande commented May 4, 2026

Uh oh!

DrahtBot commented May 4, 2026

Uh oh!

n1bor commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xB10C commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

instagibbs commented May 15, 2026

Uh oh!

darosior commented May 15, 2026

Uh oh!

mzumsande commented May 21, 2026

Uh oh!

mzumsande commented May 21, 2026

Uh oh!

mzumsande commented Sep 12, 2023 •

edited

Loading

DrahtBot commented Sep 12, 2023 •

edited

Loading

mzumsande commented Sep 12, 2023 •

edited

Loading

mzumsande commented Sep 28, 2023 •

edited

Loading

n1bor commented May 13, 2026 •

edited

Loading

0xB10C commented May 13, 2026 •

edited

Loading

0xB10C commented Jun 23, 2026 •

edited

Loading

marcofleon Jun 26, 2026 •

edited

Loading