net: treat manual peers as Core does for misbehavior and addnode

TESTING.md

@@ -390,7 +390,7 @@ A cell that names a crate-local test describes the witness that exists today. It
 | `a_heavier_header_chain_keeps_its_context` | IBD (crate) | One hub: a challenger keeps median time while the tip is genesis (invalid at the median, valid one second later); that hub then confirms a stale ancestor, a heavier chain from below it replaces the candidate and drops its script skip, and a rewind uses the confirmed base work and height. Retarget is a second hub (period length is not regtest difficulty): a heavier fork across a gap retarget is adopted, rewind restores difficulty before the next retarget, and a challenger keeps its retarget snapshot. Those three checkpoint layouts cannot be one candidate. |
 | `analog_selector_keeps_each_boundary_rate` | Mempool (crate) | One selector table: the exact band edge, absolute log distance, the requested quantile, and the two-hundredth nearest window each keep their own rate. |
 | `fee_history_gaps_cache_and_reorg_share_one_history` | IBD (crate) | One history: heights without a hurdle are not observations, rates stay cached until a new height, only the newest hashes are kept, and a reorg drops those above. The RPC backfill stays `fee_history_backfills_from_the_chain_when_relay_starts`. |
-| `peer_header_dos_and_self_announce` | P2P (**default**, crate) | One hub and one peer: verack order (wtxidrelay and sendaddrv2 stick, ping is logged, redundant verack is ignored, sendaddrv2 and oversized addrv2 after verack disconnect), unknown-parent block and compact, a full header batch continues from its last header, minchainwork stays silent until the floor, bad proof-of-work disconnects and time-too-new does not, empty-locator serves only a hash that has a body, an ancient weaker header disconnects unless noban. A noban peer stays up on a bad block and gathers no score. A version-3 header at CLTV activation logs `bad-version`, and a far stamp logs `time-too-new`. |
+| `peer_header_dos_and_self_announce` | P2P (**default**, crate) | One hub and one peer: verack order (wtxidrelay and sendaddrv2 stick, ping is logged, redundant verack is ignored, sendaddrv2 and oversized addrv2 after verack disconnect), unknown-parent block and compact, a full header batch continues from its last header, minchainwork stays silent until the floor, bad proof-of-work disconnects and time-too-new does not, empty-locator serves only a hash that has a body, an ancient weaker header disconnects unless noban. A noban or manual peer stays up on a bad block and gathers no score, and a manual peer that sends `sendaddrv2` after verack is still dropped. A version-3 header at CLTV activation logs `bad-version`, and a far stamp logs `time-too-new`. |
 | `self_announce_clearnet_then_overlay` | P2P (**default**, crate) | One address book, no chain: externalip is daily, loopback and `--no-discover` suppress it, then onion and i2p replace that clearnet address on the same book. |
 | `p2p_compact_hb_getblocktxn_and_orphan` | P2P (**default**) | One mature pad: HB coinbase `cmpctblock`, 2-tx compact → `getblocktxn` then same-peer compact retry while pending + `blocktxn` connect, unique short-id fill that fails header merkle → `getdata` (not `getblocktxn`, header not `BLOCK_FAILED`) then honest full `block` connects, orphan child GetData then parent accept (INV AlreadyHave), then live `getblocks` → `inv`, inbound `feefilter`, `filterload` disconnect. The pad seals filters through height 1, does not advertise `NODE_COMPACT_FILTERS` while the tip is ahead, stays silent for a stop at the tip, and answers `getcfilters` / `getcfheaders` / `getcfcheckpt` for the sealed height. An oversize locator is rejected. `mempool`, `filteradd`, and `filterclear` disconnect. Live mutated `block` disconnects in `on_block`. Inbound `getdata` of 20 witness blocks serves `MAX_SERVE_BLOCKS` (16); 17th is not queued (`getdata_skips_reconstruct_when_serve_inflight_at_cap`). Same-peer pending compact skip is `same_peer_pending_cmpct_does_not_getblocktxn_again`. Tokio-worker park and park-not-reject logs are `peer_blocksonly_and_orphan_tx` |
 | `p2p_feeler_completes_and_closes` | P2P (**default**) | Outbound feeler: VERSION then close (`feeler connection completed`). No live follow; dummy has no completed inbound. Same test: `run_feeler_timed` silence is `Timeout`. Inbound/outbound/plain silence stay `handshake_timeout_after_silence` |
@@ -407,7 +407,7 @@ A cell that names a crate-local test describes the witness that exists today. It
 | `end_of_ibd_follow` | P2P (**default**) | Miner plus `--sh-index` syncer. One mature regtest: coinbases pay script A, one spend pays script B. IBD reaches that tip, leaves IBD, and Electrum history matches. The next block tip-follows. Restart does not rewrite the scripthash pack mark; one more block arrives by write-behind. Cancelling IBD once the height is below the miner, then `run_p2p`, finishes the same tip and history. With the syncer caught up, dropping the miner leaves tip follow (not IBD). A partial datadir whose miner is down does not open Electrum and stays short of that tip; the same miner address coming back lets that datadir finish |
 | `end_of_ibd_sh_interrupt` | P2P (**default**) | Same miner and scripts. The syncer first catches up with `--sh-index` off, then the datadir is frozen after pass 1 (`DONE.keys`, no `DONE.post`). Resume builds the index and Electrum's first answer is the full A/B history; restart does not rewrite the pack mark. A block mined after the freeze is in that history. A copy that already has the block, with `include_hwm` at the new tip and that create appended on the unsealed head, still serves the same history and does not open Electrum early |
 | `end_of_ibd_work_fork` | P2P (**default**) | Two miners on a regtest with retargeting and min-difficulty. One chain retargets harder and stops shorter. The other forks after that retarget, resets to the pow limit, and grows taller with less work. The syncer IBD-adopts the heavier tip, keeps it across a reopen, and follows one more heavy block while the tall chain is still ahead |
-| `node_run_p2p_short` | Node (**default**) | Product `run_p2p` `--blocks-only` `--connect` to a live seeder (`--max-tip-age` so the 3-block pad is not stale IBD); process `getpeerinfo` / `getconnectioncount` / `getnetworkinfo` / `getnettotals` / `ping` while connected (v2 `manual`, as Core reports `-connect`; handshake `startingheight` equals the seeder tip; `timeoffset` present; `synced_headers`/`synced_blocks` stay `-1` until the peer announces a header hash (empty getheaders at tip does not copy VERSION height); `servicesnames` present; `getnetworkinfo.timeoffset` present); after catch-up `localrelay` / mempool `relay_enabled` stay false and `sendrawtransaction` is not `relay disabled`; Electrum `broadcast` and Esplora `POST /tx` admit decode/consensus errors (not hub-missing / not `relay disabled`); `addconnection inbound` refuses; `disconnectnode` unknown `nodeid` / empty params error then a real addr drops that row from the next `getpeerinfo`, the seeder sees it go, and a second `disconnectnode` of that `nodeid` (or of an address never connected) is `-29`; the `--connect` peer is redialled as `manual` under a new id and the seeder sees the inbound; seeder inbound `tx` then ends that session. Exit via `stop`. `max_run_secs=0` is `node_listen_and_exit`. Mock-clock `timeoffset` is the median (odd N, even N upper-middle, inbound-only 0, peer clock behind). A connecting dummy reports `-1`. Header-only and connected peers report `synced_blocks` differently. A query without a chain errors. `pingwait`, `NETWORK_LIMITED`, and `noban` are on the peer row |
+| `node_run_p2p_short` | Node (**default**) | Product `run_p2p` `--blocks-only` `--connect` to a live seeder (`--max-tip-age` so the 3-block pad is not stale IBD); process `getpeerinfo` / `getconnectioncount` / `getnetworkinfo` / `getnettotals` / `ping` while connected (v2 `manual`, as Core reports `-connect`; handshake `startingheight` equals the seeder tip; `timeoffset` present; `synced_headers`/`synced_blocks` stay `-1` until the peer announces a header hash (empty getheaders at tip does not copy VERSION height); `servicesnames` present; `getnetworkinfo.timeoffset` present); after catch-up `localrelay` / mempool `relay_enabled` stay false and `sendrawtransaction` is not `relay disabled`; Electrum `broadcast` and Esplora `POST /tx` admit decode/consensus errors (not hub-missing / not `relay disabled`); `addconnection inbound` refuses; `disconnectnode` unknown `nodeid` / empty params error then a real addr drops that row from the next `getpeerinfo`, the seeder sees it go, and a second `disconnectnode` of that `nodeid` (or of an address never connected) is `-29`; the `--connect` peer is redialled as `manual` under a new id and the seeder sees the inbound; `addnode onetry` and `add` of that address succeed and open no second session; seeder inbound `tx` then ends that session. Exit via `stop`. `max_run_secs=0` is `node_listen_and_exit`. Mock-clock `timeoffset` is the median (odd N, even N upper-middle, inbound-only 0, peer clock behind). A connecting dummy reports `-1`. Header-only and connected peers report `synced_blocks` differently. A query without a chain errors. `pingwait`, `NETWORK_LIMITED`, and `noban` are on the peer row |
 
 Removed (covered by the rows above): `confirm_cross_block_prevout_without_tx_head`,
 `double_archive_keeps_tx_height_for_coinbase_maturity`, `mega_batch_duplicate_header_is_idempotent`,

changelog.d/manual-peer-parity.md

@@ -0,0 +1,15 @@
+Fixed
+
+- **Manual peers are not punished for misbehavior, as in Core.** A
+  `--connect` or `addnode` peer that sends an invalid or mutated block, an
+  oversized `inv`, `getdata` or `addrv2`, or a bad compact block now stays
+  connected and its address is not refused. Core never disconnects or
+  discourages a manual peer for misbehavior. Protocol violations that Core
+  answers with a plain disconnect, such as `sendaddrv2` after `verack` or a
+  `tx` to a `--blocks-only` node, still drop a manual peer.
+- **`addnode` opens no second session to a connected address.** `addnode
+  onetry` or `add` of an address with a live session, or one still being
+  dialled, dialled it again, and each dial that connected became its own
+  session. As in Core, the RPC now succeeds without dialling. A dial now
+  counts from the moment it is queued, so an `addnode` or `--connect`
+  redial right behind another dial to the same address adds nothing.

crates/rbitcoin-net/src/peer.rs

@@ -140,11 +140,11 @@ async fn accept_received_from_peer(
     })
 }
 
-/// One-day in-memory refusal. Noban peers are not recorded.
+/// One-day in-memory refusal. Noban and manual peers are not recorded.
 /// A loopback address is every local connection, so Core disconnects that
 /// peer and does not discourage the address.
 fn note_threshold_refusal(session: Option<&crate::peers::LivePeer>) {
-    let Some(s) = session.filter(|s| !s.session_noban()) else {
+    let Some(s) = session.filter(|s| !misbehavior_exempt(s)) else {
         return;
     };
     if local_addr_skips_discourage(s.addr.ip()) {
@@ -169,6 +169,27 @@ fn threshold_disconnect(session: Option<&crate::peers::LivePeer>) -> NetError {
     NetError::Protocol("peer misbehavior threshold")
 }
 
+/// Core `MaybeDiscourageAndDisconnect`: misbehavior never disconnects or
+/// discourages a noban peer or a manual one (`--connect`, `addnode`).
+fn misbehavior_exempt(s: &crate::peers::LivePeer) -> bool {
+    s.session_noban() || s.conn_type == crate::peers::PeerConnType::Manual
+}
+
+/// Core `Misbehaving`. Unlike [`punish_disconnect`], it also keeps a manual
+/// peer connected.
+fn misbehaving(ban_score: &mut u32, session: Option<&crate::peers::LivePeer>) {
+    if let Some(s) =
+        session.filter(|s| s.conn_type == crate::peers::PeerConnType::Manual && !s.session_noban())
+    {
+        rbitcoin_log::info!("Warning: not punishing manually connected peer {}!", s.id);
+        return;
+    }
+    punish_disconnect(ban_score, session);
+}
+
+/// Disconnects and refuses the address; a noban peer is kept. Used where Core
+/// sets `fDisconnect`, which drops a manual peer too. Core `Misbehaving`
+/// sites call [`misbehaving`].
 fn punish_disconnect(ban_score: &mut u32, session: Option<&crate::peers::LivePeer>) {
     if let Some(s) = session.filter(|s| s.session_noban()) {
         rbitcoin_log::info!("Warning: not punishing noban peer {}!", s.id);
@@ -181,9 +202,10 @@ fn punish_disconnect(ban_score: &mut u32, session: Option<&crate::peers::LivePee
     }
 }
 
-/// Core `MaybeDiscourageAndDisconnect`: misbehavior never drops a noban peer.
+/// Core `MaybeDiscourageAndDisconnect`: misbehavior never drops a noban or
+/// manual peer.
 fn misbehavior_disconnects(ban_score: u32, session: Option<&crate::peers::LivePeer>) -> bool {
-    ban_score >= BAN_SCORE_THRESHOLD && !session.is_some_and(|s| s.session_noban())
+    ban_score >= BAN_SCORE_THRESHOLD && !session.is_some_and(misbehavior_exempt)
 }
 /// Cap on incomplete compact blocks awaiting `blocktxn` (DoS).
 const MAX_PENDING_CMPCT: usize = 1;
@@ -2889,7 +2911,7 @@ fn on_addrv2(
     rbitcoin_log::info!("{}", received_addrv2_log(nbytes, id));
     if n > MAX_ADDR_TO_SEND {
         rbitcoin_log::info!("{}", addrv2_message_size_log(n));
-        punish_disconnect(&mut follow.ban_score, session);
+        misbehaving(&mut follow.ban_score, session);
         return Ok(());
     }
     if session.is_some_and(|s| s.conn_type == crate::peers::PeerConnType::AddrFetch && n > 1) {
@@ -3026,7 +3048,7 @@ async fn serve_getdata(
     inv: &[Inventory],
 ) -> Result<(), NetError> {
     if inv.len() > MAX_INV_SIZE {
-        punish_disconnect(&mut follow.ban_score, session);
+        misbehaving(&mut follow.ban_score, session);
         return Ok(());
     }
     let inflight = session.map(|s| &s.serve_inflight);
@@ -3202,11 +3224,8 @@ fn on_getblocktxn(
         }
         if bad {
             rbitcoin_log::info!("p2p: getblocktxn with out-of-bounds tx indices");
-            // Out-of-range indexes: disconnect.
-            follow.ban_score = follow.ban_score.saturating_add(BAN_SCORE_THRESHOLD);
-            if let Some(s) = session {
-                s.request_disconnect();
-            }
+            // Core `Misbehaving`: noban and manual peers stay connected.
+            misbehaving(&mut follow.ban_score, session);
         } else {
             // Core: past `MAX_GETBLOCKTXN_DEPTH` (10) send the full block.
             const MAX_GETBLOCKTXN_DEPTH: u32 = 10;
@@ -3244,7 +3263,7 @@ fn on_inv(
     items: &[Inventory],
 ) -> Result<(), NetError> {
     if items.len() > MAX_INV_SIZE {
-        punish_disconnect(&mut follow.ban_score, session);
+        misbehaving(&mut follow.ban_score, session);
         return Ok(());
     }
     let mut want = Vec::new();
@@ -3434,7 +3453,7 @@ fn on_headers(
         {
             // Headers on a cached-invalid chain: disconnect
             // (`p2p_unrequested_blocks` step 8 follow-up header).
-            punish_disconnect(&mut follow.ban_score, session);
+            misbehaving(&mut follow.ban_score, session);
             return Ok(());
         }
         let connecting = header_announcement_connects(hub, &follow.pending_headers, prev);
@@ -3520,19 +3539,19 @@ async fn on_block(
     if !block.check_merkle_root() {
         rbitcoin_log::info!("Block mutated: bad-txnmrklroot, hashMerkleRoot mismatch");
         take_requested_block(hub, &mut follow.requested_blocks, &hash);
-        punish_disconnect(&mut follow.ban_score, session);
+        misbehaving(&mut follow.ban_score, session);
         return Ok(());
     }
     if crate::compact::merkle_body_mutated(&block.txdata) {
         rbitcoin_log::info!("Block mutated: bad-txns-duplicate");
         take_requested_block(hub, &mut follow.requested_blocks, &hash);
-        punish_disconnect(&mut follow.ban_score, session);
+        misbehaving(&mut follow.ban_score, session);
         return Ok(());
     }
     if rbitcoin_consensus::block_mutated_without_coinbase(block) {
         rbitcoin_log::info!("Block mutated: 64-byte transaction without a coinbase");
         take_requested_block(hub, &mut follow.requested_blocks, &hash);
-        punish_disconnect(&mut follow.ban_score, session);
+        misbehaving(&mut follow.ban_score, session);
         return Ok(());
     }
     if let Some(s) = session {
@@ -3561,7 +3580,7 @@ async fn on_block(
     }
     if let Err(e) = hub.ensure_header(&block.header) {
         if !crate::chain::accept_err_is_temporary_time(&e) {
-            punish_disconnect(&mut follow.ban_score, session);
+            misbehaving(&mut follow.ban_score, session);
         }
         return Ok(());
     }
@@ -3592,12 +3611,12 @@ fn on_block_unrequested_skip(
         && !follow.pending_headers.contains_key(&prev)
     {
         rbitcoin_log::info!("{}", accept_prev_not_found_log(hash));
-        punish_disconnect(&mut follow.ban_score, session);
+        misbehaving(&mut follow.ban_score, session);
         return Ok(true);
     }
     if let Err(e) = hub.ensure_header(&block.header) {
         if !crate::chain::accept_err_is_temporary_time(&e) {
-            punish_disconnect(&mut follow.ban_score, session);
+            misbehaving(&mut follow.ban_score, session);
         }
         return Ok(true);
     }
@@ -3747,7 +3766,7 @@ fn on_cmpctblock_reject_early(
 ) -> Result<bool, NetError> {
     if session.is_some_and(|s| s.has_failed_cmpct(&hash)) {
         rbitcoin_log::info!("p2p: previous compact block reconstruction attempt failed");
-        punish_disconnect(&mut follow.ban_score, session);
+        misbehaving(&mut follow.ban_score, session);
         return Ok(true);
     }
     if let Some(s) = session {
@@ -3757,13 +3776,13 @@ fn on_cmpctblock_reject_early(
     }
     if !crate::compact::prefilled_indexes_ok(hsi) {
         rbitcoin_log::info!("p2p: invalid index in cmpctblock message");
-        punish_disconnect(&mut follow.ban_score, session);
+        misbehaving(&mut follow.ban_score, session);
         return Ok(true);
     }
     // Child of a cached-invalid block: disconnect. Same-hash cached invalid
     // via compact stays connected.
     if hub.is_block_invalid(&hsi.header.prev_blockhash) {
-        punish_disconnect(&mut follow.ban_score, session);
+        misbehaving(&mut follow.ban_score, session);
         return Ok(true);
     }
     if hub.is_block_invalid(&hash) {
@@ -3956,7 +3975,7 @@ async fn on_blocktxn(
     }
     if session.is_some_and(|s| s.has_failed_cmpct(&hash)) {
         rbitcoin_log::info!("p2p: previous compact block reconstruction attempt failed");
-        punish_disconnect(&mut follow.ban_score, session);
+        misbehaving(&mut follow.ban_score, session);
         return Ok(());
     }
     let Some(pc) = follow.pending_cmpct.remove(&hash) else {