Skip to content

fix(p2p): stop nodes from dialing and banning themselves - #134

Merged
On1x merged 1 commit into
masterfrom
fix/p2p-self-connection
Aug 6, 2026
Merged

fix(p2p): stop nodes from dialing and banning themselves#134
On1x merged 1 commit into
masterfrom
fix/p2p-self-connection

Conversation

@chiliec

@chiliec chiliec commented Aug 6, 2026

Copy link
Copy Markdown
Member

Field report

A mainnet node showed 127.0.0.1:2001 and 127.0.0.1:36702 as ACTIVE peers — the two ends of one self-dial — and soft-banned itself. p2p-isolated-peers was the operator's workaround. The node had just been updated for the Boost 1.91 release, but that is coincidental: the fc diff in that release is API renames only (to_ulongto_uint, from_stringmake_address_v4, resolver::queryasync_resolve, io_serviceio_context) — no change to address values or peer selection. The restart is simply what re-ran discovery.

Root cause

Three defects line up:

  1. Peer exchange gossips non-routable endpoints. on_dlt_peer_exchange_request advertises the raw endpoint of every outbound ACTIVE peer, and on_dlt_peer_exchange_reply adopts and dials anything it is handed. A node running a local validator+relay pair (p2p-seed-node = 127.0.0.1:2001) therefore advertises loopback to the whole network; every receiver dials its own listener. Once connected, that loopback peer is outbound+ACTIVE, so the receiver re-advertises it — the loop is self-sustaining and spreads. The info.node_id == _node_id filter does not help: the node_id on that entry belongs to the sender's local relay.

  2. No self-connection guard. on_dlt_hello never compared hello.node_id to _node_id. Both directions of the self-dial stay ACTIVE: they inflate the peer count, feed check_wedge_watchdog's corroboration set with our own head, and a soft_ban_peer() notification aimed at "that peer" is delivered straight back to us — the node bans itself ("Peer 127.0.0.1:2001 soft-banned us").

  3. The duplicate-node_id guard was inert. find_active_peer_by_node_id() scans _peer_states in peer_id order, and state.node_id is assigned just before the call, so the scan can return the calling peer itself and the dup != peer test silently no-ops. This also let duplicate inbound+outbound connections to real peers survive.

Changes

  • is_routable_endpoint() — pure predicate rejecting loopback, RFC1918, link-local, multicast, 0.0.0.0 and port 0. fc's is_public_address() covers everything except 127/8 and 0/8, so those are explicit.
  • Applied on both sides of peer exchange. The adopt side is load-bearing: peers still running the old code keep gossiping these endpoints. Explicitly configured seeds never pass through peer exchange, so deliberate LAN/loopback clusters still peer as configured.
  • Self-connection guard in on_dlt_hello: drop the socket and erase the endpoint from _known_peers so periodic_reconnect_check() does not redial it.
  • find_active_peer_by_node_id(nid, exclude) — makes the existing dedup actually fire.
  • test_routable_endpoint.cpp pins the predicate boundaries, mirroring test_wedge_predicate.cpp.

Verification

  • -fsyntax-only replay of the tree's real compile command for dlt_p2p_node.cpp (worktree includes first): clean, only pre-existing warnings. Same for the new test TU at C++17.
  • Host-order octet arithmetic checked against Boost.Asio directly.
  • Not built or run locally — any fc consumer pulls vendor/equihash (SSE2-only), which does not link on arm64. CI's amd64 build is the first real compile+link, and BUILD_CONSENSUS_TESTS is still OFF there, so the new test does not execute in CI yet (same status as test_wedge_predicate.cpp).
  • No live two-node repro yet. The field check is: deploy, remove p2p-isolated-peers, confirm no 127.0.0.1 entries appear in DLT P2P Stats.

A node was showing 127.0.0.1:2001 and 127.0.0.1:36702 as ACTIVE peers —
the two ends of one self-dial — and then soft-banning itself. Three
independent defects line up to produce that:

1. Peer exchange gossips whatever endpoints its sender has connected,
   with no routability filter on either side. A node running a local
   validator+relay pair advertises 127.0.0.1:2001 to the whole network;
   every receiver adopts it and dials its OWN listener. Once connected,
   that loopback peer is outbound+ACTIVE, so the receiver re-advertises
   it too — the loop is self-sustaining and spreads.

2. on_dlt_hello never compared hello.node_id against _node_id, so a
   connection that lands on our own listener is indistinguishable from a
   real peer. Both directions stay ACTIVE: they inflate the peer count,
   dilute the wedge watchdog's corroboration set with our own head, and
   a soft_ban_peer() notification aimed at "that peer" is delivered
   straight back to us — the node bans itself.

3. The duplicate-node_id guard could not catch it either:
   find_active_peer_by_node_id() scans _peer_states in peer_id order and
   state.node_id is assigned just before the call, so the scan can return
   the calling peer itself and the `dup != peer` test silently no-ops.
   This also let duplicate inbound+outbound connections to real peers
   survive.

Fixes, in the same order: is_routable_endpoint() applied on both the
share and the adopt side of peer exchange (the adopt side is load-bearing
— peers running the old code keep gossiping these); a self-connection
guard that drops the socket and forgets the endpoint; and an `exclude`
parameter that makes the existing dedup actually fire.

Explicitly configured seeds are unaffected — they never pass through
peer exchange, so LAN/loopback clusters still peer as configured.

test_routable_endpoint.cpp pins the predicate's boundaries the way
test_wedge_predicate.cpp pins is_wedged().

@On1x On1x left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the p2p self-connection/self-ban fix. is_routable_endpoint() correctly rejects port 0, loopback (127/8), 0/8 and non-public (RFC1918/link-local/multicast via fc), and is applied on both the advertise (peer-exchange request) and adopt (reply) paths — with explicitly-configured seeds bypassing that path, so LAN validator+relay setups keep working. The on_dlt_hello self-guard (hello.node_id == _node_id -> drop + forget endpoint) is the direct fix for the self-dial, and the new exclude param on find_active_peer_by_node_id fixes the inert dedup guard (state.node_id is set before the scan, so the caller's own peer had to be excluded). Networking only, no consensus/state change; CI green; adds a consensus_sim routable-endpoint test. LGTM.

@On1x
On1x merged commit 9515d74 into master Aug 6, 2026
2 checks passed
On1x added a commit that referenced this pull request Aug 6, 2026
…idator handbook + seed config #135)

Keep the HF14/PM branch current with master so PR #124 CI rebuilds against the
latest toolchain + p2p fix and confirms PM still builds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants