[bkeroack]

Problem

Two gaps left after #876 made --connect peers manual:

1. Misbehavior drops a manual peer. Core's MaybeDiscourageAndDisconnect never disconnects or discourages a manual peer ("Not punishing manually connected peer"). Here punish_disconnect and the per-frame misbehavior_disconnects check exempted only noban, so a --connect or addnode peer that sent, for example, a block with bad proof of work was dropped, and inbound connections from its address were refused for a day. Since net: --connect peers are manual, as in Core #876 that applies to every --connect peer. net: redial --connect addresses that drop, as in Core #908 redials it, but each such message costs the session.
2. addnode opens a second session. addnode onetry and addnode add dialled their target even when it had a live session or a dial in flight, and each dial that connected became its own session. Core's OpenNetworkConnection returns early on AlreadyConnectedToHost(pszDest), and ConnectNode drops a name that resolves to a connected address (AlreadyConnectedToAddressPort).

Fix

• New misbehaving() is Core's Misbehaving: a noban or manual peer is logged and kept, with no score. The 15 sites that are Misbehaving in Core call it: oversized inv, getdata and addrv2; mutated blocks (merkle root, duplicate txs, 64-byte tx without a coinbase); header and block failures that MaybePunishNodeForBlock punishes (bad header, unknown or cached-invalid parent, also via cmpctblock); an invalid compact block; a cmpctblock or blocktxn after a failed reconstruction; and getblocktxn with out-of-range indexes.
• punish_disconnect stays at the other 9 sites. At 8 of them Core sets fDisconnect, which drops a manual peer too: sendaddrv2 after verack, oversized getheaders and getblocks locators, a tx or tx inv the node does not take, filterload without NODE_BLOOM, and the addr reply that ends an addr-fetch connection (two sites). The ninth, the per-peer tx request cap, has no Core counterpart.
• misbehavior_disconnects and the address refusal treat a manual peer like noban. Rate-limit and oversize-frame exits still disconnect a manual peer, as they do a noban one, but no longer refuse its address.
• getblocktxn with out-of-range indexes disconnected directly, noban peers included. It is Misbehaving in Core and now goes through misbehaving().
• addnode (onetry, add, and PeerHub::addnode) skips a target with a live session or a dial in flight, and still returns success. add still records the target, so the redial pass dials it once that session ends.
• A dial now counts as in flight from the moment it is queued: each DialRequest carries its DialInFlight to the dial task, which used to take one only when it began to connect. DialRequest loses Clone and gains a crate-private field, so code outside rbitcoin-net can no longer build one; nothing in the workspace did. addnode and the redial pass share dial_manual_target, which claims the endpoint (check and count under one lock) and then checks for a live session. A dial registers its session before it drops its claim, so two addnode calls in a row, or addnode beside the 2 s redial, queue one dial. A feeler is queued without a mark, as before: it registers no session, and its probe has no connect deadline, so a stuck probe would keep the redial pass off a manual peer at the same address.

Tests

• peer_header_dos_and_self_announce: the noban bad-block beat, now noban_and_manual_bad_block_is_not_punished, adds a manual peer. The bad block leaves it connected with no score, misbehavior_disconnects is false for it at the threshold, and a sendaddrv2 after verack still drops it.
• threshold_exit_refuses_the_address_without_punish_disconnect: a manual peer's threshold exit refuses no address, and punish_disconnect drops it.
• New peers::tests::addnode_skips_an_endpoint_that_is_connected_or_dialling: with a live session, addnode onetry and add by string and PeerHub::addnode by address send no dial. An onion with a dial in flight is not dialled until that dial ends, and then onetry, add and a redial pass right behind each other queue one dial. Once the session ends, onetry dials it as manual.
• New peers::tests::a_queued_feeler_does_not_hold_off_a_manual_dial: with a feeler to an address queued and held, addnode onetry of that address still queues a manual dial.
• redial_same_endpoint_in_addnode_and_connect_dials_once drops the requests it took before the next pass, since a request it holds now counts as a dial in flight.
• node_run_p2p_short: after the --connect peer is redialled, addnode onetry and add of its address succeed, and getpeerinfo keeps one row for it over 500 ms. The existing beat where the seeder's tx ends the manual --connect session on a --blocks-only node still passes, which covers the fDisconnect side end to end.

Each new assertion fails when its guard is reverted:

• Dropping the manual exemption from misbehaving() fails the journey on "Core never disconnects a manual peer for misbehavior".
• Dropping manual from misbehavior_exempt fails it on "a manual peer at the threshold stays connected", and fails the threshold test on "a manual peer is not recorded".
• Exempting manual peers in punish_disconnect as well fails both on "a protocol violation still drops a manual peer" and "a protocol violation drops a manual peer".
• Dropping the live-session check from dial_manual_target fails the unit test on "a connected endpoint is not dialled again", and fails node_run_p2p_short on "addnode opened a second session".
• Letting claim_dial ignore a dial in flight fails the unit test on "nor is one still dialling".
• Not counting the claim, so a queued dial is invisible until the dial task starts it, fails the unit test on "a queued dial is not queued again".
• Counting a queued feeler fails the feeler test on "a queued feeler does not block addnode".

Ran locally: cargo fmt --all -- --check, ./scripts/ast-grep.sh, cargo clippy --workspace --all-targets -- -D warnings, the full rbitcoin-net lib tests, the rbitcoin-rpc addnode tests, the rbitcoin-node run:: tests, node_run_p2p_short three times, and p2p_feeler_completes_and_closes.

Not changed

• At the fDisconnect sites a noban peer is still kept and a plain peer's address is still refused. Core drops the noban peer and refuses no address there. Both predate this change.
• The disconnect for a peer that announces an ancient, weaker header chain (announced_tip_is_hopeless) still drops a manual peer. The rule is rbitcoin's own, and this change leaves it alone. Since net: redial --connect addresses that drop, as in Core #908, a --connect peer dropped there is redialled every 2 s.

🤖 Generated with Claude Code

[rearden-grok]

Summary

The misbehavior split matches the Core claim: misbehaving logs and keeps a manual peer with no score and no address refusal, while the fDisconnect sites still go through punish_disconnect and drop that peer. dial_manual_target claims under one lock before the live-session check, and a queued dial keeps that claim until prepare_outbound_session has registered the session, so two addnodes or addnode beside the redial pass cannot both queue. The tests pin those guards and would fail if they were reverted. One regression sits outside that path: every dial / dial_domain now owns a DialInFlight from enqueue time, including feelers, which never enter the connect timeout that exists so a stuck dial cannot pin the mark.

Issue counts by severity

• bugs: 1
• suggestions: 0
• nits: 0

[rearden-grok]

[bug] dial and dial_domain now take DialInFlight before enqueue and move it into this task. The feeler branch returns before prepare_outbound_session, so that guard stays alive across connect_net and the whole run_feeler call. connect_net has no deadline here. OUTBOUND_CONNECT_TIMEOUT exists so a proxy that accepts and never answers cannot hold a DialInFlight mark forever and block the redial pass; feelers used to skip that mark entirely. addconnection feeler is the caller (PeerHub::dial). A stuck SOCKS handshake to an address that is also --connect or addnode makes claim_dial keep returning None, so the manual peer is not redialled until the feeler task ends. A domain feeler drops the guard on the early Err, but an IP feeler does not.

Suggestion: Drop in_flight at the start of the feeler branch (feelers do not register a session and were not part of the in-flight set), or apply the same connect_timeout_for(..., OUTBOUND_CONNECT_TIMEOUT) around connect_net before the probe and do not hold the mark past that connect.

[bkeroack]

Fixed in cd30a67. A feeler is queued without a DialInFlight again (PeerHub::queue_dial; DialRequest::in_flight is now an Option), so it never enters the in-flight set, as before this PR. The new a_queued_feeler_does_not_hold_off_a_manual_dial holds a queued feeler and checks that addnode onetry of the same address still queues a manual dial; it fails if the feeler is counted.

The feeler's own connect_net still has no deadline, as before this PR. I left that alone here.

[rearden-grok]

Summary

The previous bug is fixed. Every queued feeler goes through queue_dial (dial, dial_net, dial_domain, and addconnection), which leaves in_flight as None and does not touch dials_in_flight. send_dial is the only DialRequest constructor, and dial_manual_target always passes Some(claim) with PeerConnType::Manual. In the dial task, a feeler still returns from run_outbound_session_with_abort before prepare_outbound_session, so the unwrap_or_else(|| peers.dial_in_flight(...)) fallback never runs for one and cannot start a count. A manual, addnode, or redial dial is still claimed at enqueue; claim_dial then the live-session check is unchanged, so a second addnode or a redial beside an in-flight manual dial still queues nothing. a_queued_feeler_does_not_hold_off_a_manual_dial holds the queued feeler across addnode and would fail if that request carried a mark. It does not run the dial task; that is not a hole, because nothing after dequeue attaches a mark to a feeler. No new defect in the fix. Ready to approve.

Issue counts by severity

• bugs: 0
• suggestions: 0
• nits: 0