Repository navigation
feat(p2p): add opt-in Tor and IVNP I2P connectivity - #1684
Conversation
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 19 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/p2p/src/discouragement.rs">
<violation number="1" location="crates/p2p/src/discouragement.rs:37">
P3: Silently ignoring `AnonymousTor` here is intentional, since inbound Tor ids are fresh per accept. The `discouraged` field in the `note_misbehavior` log is now wrong for these peers, though. Add a doc comment on `insert` that states anonymous identities are not recorded, and make the log field reflect whether an entry was actually stored.</violation>
</file>
<file name="crates/p2p/src/overlay/sam.rs">
<violation number="1" location="crates/p2p/src/overlay/sam.rs:210">
P3: The retry changes lack coverage. Add a test that forces `identity::create` to fail once, then confirms the retry reuses the same key. Also add one that confirms backoff resets only after 60 seconds of healthy monitoring.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
| self.expire(now); | ||
| let ip = ip.to_canonical(); | ||
| let ip = ip.into().to_canonical(); | ||
| if matches!(ip, PeerAddress::AnonymousTor(_)) { |
There was a problem hiding this comment.
P3: Silently ignoring AnonymousTor here is intentional, since inbound Tor ids are fresh per accept. The discouraged field in the note_misbehavior log is now wrong for these peers, though. Add a doc comment on insert that states anonymous identities are not recorded, and make the log field reflect whether an entry was actually stored.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At crates/p2p/src/discouragement.rs, line 37:
<comment>Silently ignoring `AnonymousTor` here is intentional, since inbound Tor ids are fresh per accept. The `discouraged` field in the `note_misbehavior` log is now wrong for these peers, though. Add a doc comment on `insert` that states anonymous identities are not recorded, and make the log field reflect whether an entry was actually stored.</comment>
<file context>
@@ -34,6 +34,9 @@ impl DiscouragedPeers {
pub(crate) fn insert(&mut self, ip: impl Into<PeerAddress>, now: Instant) {
self.expire(now);
let ip = ip.into().to_canonical();
+ if matches!(ip, PeerAddress::AnonymousTor(_)) {
+ return;
+ }
</file context>
| if control.line()? != "PONG bitcoin-rs" { | ||
| return Err(invalid("SAM control heartbeat failed")); | ||
| } | ||
| if healthy_since.elapsed() >= Duration::from_secs(60) { |
There was a problem hiding this comment.
P3: The retry changes lack coverage. Add a test that forces identity::create to fail once, then confirms the retry reuses the same key. Also add one that confirms backoff resets only after 60 seconds of healthy monitoring.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At crates/p2p/src/overlay/sam.rs, line 210:
<comment>The retry changes lack coverage. Add a test that forces `identity::create` to fail once, then confirms the retry reuses the same key. Also add one that confirms backoff resets only after 60 seconds of healthy monitoring.</comment>
<file context>
@@ -205,6 +207,9 @@ impl OverlayTransport {
if control.line()? != "PONG bitcoin-rs" {
return Err(invalid("SAM control heartbeat failed"));
}
+ if healthy_since.elapsed() >= Duration::from_secs(60) {
+ backoff = Duration::from_secs(1);
+ }
</file context>
gosunuts
left a comment
gosunuts
left a comment
There was a problem hiding this comment.
Tor / IVNP overlay follow-up — adopt targeted hardening from #1687, preserve #1684's ownership model
I compared this PR at e003ccc with the current #1687 overlay implementation (head 69626f0). Please strengthen #1684 using the useful protocol and validation work from #1687 before treating its Tor/IVNP support as qualified. Do not mechanically merge the two implementations: they have diverged, and #1684's single OverlayConfig policy, typed logical peer identity, asynchronous Tor/SAM owners, and live-session-based I2P address publication are clearer boundaries worth retaining.
Requested changes / follow-ups:
-
Tor ControlPort authentication and protocol hardening.
crates/p2p/src/overlay/tor.rscurrently sendsAUTHENTICATEwith a raw cookie (or NULL) and supports only a TCP control endpoint. #1687'scrates/p2p/src/tor_control.rsimplementsPROTOCOLINFOdiscovery, challenge/responseSAFECOOKIEvalidation, password authentication, Unix sockets, bounded control replies and negative tests (including a mismatched server hash). Reuse those protocol invariants and tests, not necessarily the larger service architecture. Keep #1684's independent owner and retry-until-shutdown behavior. A modern secured Tor installation should not require weakening its ControlPort authentication to use the node. -
Verify the requested I2P destination before
STREAM CONNECT.crates/p2p/src/overlay/sam.rs::sam_lookuphashes the public destination returned byNAMING LOOKUPand uses it as the peer endpoint; for an explicit<hash>.b32.i2prequest it does not verify that the returned destination's SHA-256 equals the requested hash. #1687'sSamSession::connectchecks this and has a regression case (naming_destination_hash_mismatch_is_transport_and_forbids_stream_connect). Add the same identity-binding invariant and test to #1684: a mismatch must refuse the stream beforeSTREAM CONNECT, be classified as a local bridge/transport failure, and must not penalize the requested peer in AddrMan. Retain bounded SAM response parsing and validate the complete destination encoding. -
Do not silently fork the persisted AddrMan v9 format. Both branches claim schema
VERSION = 9, but #1684 serializesSource::Overlay(PeerAddress)for overlay gossip while #1687 currently has noOverlaysource variant and uses a different endpoint serialization boundary. In particular, reading a #1684-saved overlay-source record with #1687's decoder can reject the file despite the same version number. Define one canonical v9 wire/storage contract or a versioned migration with explicit provenance preservation. Add round trips for IP/IPv6/Tor/I2P, overlay-source gossip, old v8 migration with backup, restart anchors, and cross-branch files in both directions. On an unsupported record, fail closed and preserve source bytes; never rewrite away peer history or keys. -
Complete real external interoperability and failure-recovery evidence. #1684 adds a useful ignored process E2E and pinned IVNP/Tor harness, but the PR correctly says the external campaign and final-head process convergence were not run. #1687's private overlay harness and failure taxonomy provide reference scenarios (not interchangeable evidence for IVNP). Exercise pinned IVNP, not just i2pd: bidirectional Bitcoin header/block exchange, onion/I2P identity persistence, same-key restart and redial, SAM control loss/reconnect, Tor daemon restart, proxy refusal, and clean cancellation while setup/accept is blocked. Explicitly distinguish an IVNP/router-level same-key failure from a bitcoin-rs regression using a SAM-only control if needed. Record binary hashes and outcomes, and keep unresolved cases marked unqualified.
Preserve what #1684 already gets right: OverlayConfig::allows + local_dns_allowed govern DNS lookup as well as the eventual dial (no local DNS or clearnet fallback in onion-/I2P-only mode); overlay workers are separately owned; Tor retries do not stop permanently after an arbitrary count; outbound-only I2P sessions do not appear as listening addresses. These are improvements over the current #1687 integration—please do not regress them when borrowing features.
Scope: the identity check and storage-format compatibility are correctness work to resolve before merging competing overlay changes. The additional Tor auth and external provider proof are concrete qualification gaps. Keep changes focused and continue to use the existing P2P session/admission/sync owners instead of introducing another peer or transport authority.
This is a COMMENT review requesting follow-up improvements, not an approval or an automatic merge blocker on unrelated parts of the repo.
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
![gormes-agent[bot]](https://avatars.githubusercontent.com/in/5341388?s=60&v=4){kind=small}
| let mut naming = | ||
| Setup::new_until(address, &stop, deadline.min(Instant::now() + SETUP_TIMEOUT))?; | ||
| hello(&mut naming)?; | ||
| let (public, resolved) = lookup(&mut naming, &endpoint.address().to_string())?; |
There was a problem hiding this comment.
P2 — I2P naming failures no longer reach AddressBook::attempted.
Since 34b0ee81, sam_connect performs NAMING LOOKUP before STREAM CONNECT. status() (L66-71) attributes only STREAM STATUS CANT_REACH_PEER/TIMEOUT as remote_failure, so a dead I2P peer whose LeaseSet cannot be found fails here with NAMING REPLY RESULT=KEY_NOT_FOUND, which is classified as a local ConnectionRefused.
In listener.rs:852-861 that means book.attempted() is never called: last_attempt/failures stay unchanged, so neither the 10-minute chance() penalty nor terrible() ever applies, and the dead address keeps being selected at full probability. This reopens for I2P the behavior fixed in 93dc7025. (Core also resolves before connecting through SAM, but records Attempt before dialing.)
Suggested fix: in the connect path, map NAMING REPLY KEY_NOT_FOUND (and possibly TIMEOUT) to remote_failure, keeping hash mismatch / invalid encoding local as now. Add a regression asserting remote_connect_failed for a KEY_NOT_FOUND naming reply. This matches cubic thread 4387360956, still open at this head.
Source trace; not reproduced against a live router.
Review at
|
Record the p2p rustix dependency in all three standalone workspaces and the existing RPC metrics dependency in LDK. Keep resolved package versions unchanged.
Classify a valid SAM naming KEY_NOT_FOUND reply as a remote dial failure so the existing AddressBook attempt owner applies retry policy. Keep malformed replies, router errors, identity mismatch and undocumented naming timeouts local. Add a socket regression covering attribution, no stream creation and the retained session epoch. Sync impact: transport discovery/retry can affect SYNC-02, SYNC-04 and SYNC-06; authoritative header/body validation, frontier scheduling and chainstate commits are unchanged. Overlay (25 passed, 1 external ignored), frontier recovery (12), request tick (1), expiry handover (1), and node config (27) tests passed on Windows/Rust 1.99 with test opt-level=0. P2P all-target Clippy -D warnings, formatting and diff checks passed. Real Tor/IVNP convergence remains unqualified.
Merging main adds public-listener constraints alongside overlay validation, taking the ordered gate just over the line-count lint. Document the reason to retain one fail-early validation gate; runtime behavior is unchanged. Node all-target Clippy with fjall,zmq,silent-payments and config_layered (27 tests) pass on Rust 1.99.
Exercise both block-transfer directions before and after application identity restart in the ignored Tor/IVNP process test, checking admitted headers, complete block bodies, and the receiving tip for every transfer. Add local Tor/SAM fixtures proving failed identity publication neither advertises a service nor discards its generated key. Retrying must persist the same key. Keep backend sockets open to prove shutdown interrupts a blocked Tor heartbeat and an unanswered SAM accept. Sync impact: test-only changes; authoritative scheduling, validation and commit paths are unchanged. The process assertions cover SYNC-02, SYNC-04 and SYNC-06 but remain unexecuted external evidence, not qualification. Validation: offline Windows overlay suite 27 passed, 1 external test ignored; strict P2P library/test and overlay E2E Clippy; formatting and diff checks. No external Tor/IVNP campaign, installation, or network policy change. The 60-second healthy-backoff reset and real backend recovery remain gaps. Refs #1601
gosunuts
left a comment
gosunuts
left a comment
There was a problem hiding this comment.
The overall direction is good: Tor/I2P terminate at the transport boundary and reuse the existing admission, PeerTable/PeerLease, relay, and BlockSync owners. I would keep that design rather than build a parallel overlay P2P stack.
Before merging, I'd like us to reduce a few sources of ongoing complexity:
-
Separate routable peer addresses from ephemeral connection identities.
PeerAddress::AnonymousTor(u64)is a connection handle, not a dialable/learnable address. Putting it in the sharedPeerAddressenum forces special cases in AddrMan, netgroups, persistence, and admission (includingunreachable!/serialization rejection). Could we represent anonymous inbound connections at the session/connection layer instead, so the address-book API only accepts actual peer destinations? KeepPeerEndpointfor IP/onion/I2P. -
Avoid a second outbound retry scheduler.
P2pService::spawn_outbound_drainnow maintainsmanual_retry: HashMap<PeerEndpoint, (Instant, Duration)>and a separate retry loop for overlay/proxied pinned peers. Please check whether the existing outbound/pinned-peer scheduling owner can handle these retries with route-specific dial outcomes instead. We should avoid two independently evolving retry state machines for the same peer. -
Make SAM epoch/teardown ownership explicit. The SAM
Session::aliveflag,PeerLease::transport_alive, and network-wideretire(..., I2p)interact to invalidate connections. The epoch-scoped invalidation is a good idea, but please confirm one owner is responsible for retiring that epoch and its leases, with no independent replacement/cleanup authority in the transport and peer scheduler. A concise invariant plus a recovery test would help. -
Resolve AddrMan v9 compatibility with #1687. Both branches currently use v9 while having different overlay layout/netgroup interpretations. Please establish one canonical on-disk format or explicit provenance handling before merge; the version number alone is not sufficient to identify the layout. Do not add speculative bidirectional migration machinery unless we have concrete supported input fixtures.
I'd treat (1) and (2) as the main complexity reductions, and (4) as a compatibility issue. No need for a wholesale rewrite or code-golfing the Tor/SAM protocol implementation: the goal is one authoritative owner per state transition, one connection scheduling path, and no overlay-specific special cases above the transport boundary unless the protocol requires them.
Read and write one explicitly identified v1 format. Preserve incompatible development snapshots before fresh discovery, remove historical conversions and upgrade fixtures, and retain current validation and atomic publication. Refs #1688
Keep PeerAddress and PeerEndpoint destination-only. Use the lease connection id for anonymous Tor runtime identities and preserve attribution through sync, mempool and RPC without putting connection handles in AddrMan. Sync impact: runtime peer keys and attribution touch delivery and recovery boundaries for SYNC-02/04/06; header/body validation and chainstate commit owners are unchanged. Anonymous admission (2), peer table (33), parent requests (6), transaction policy (47), frontier recovery (12), tick delivery (1), endpoint (2), and RPC network (53) tests passed. P2P/node/RPC/mempool all-target checks and strict library/test Clippy passed. Real process and external overlay convergence remain unrun.
Make the fixed-peer worker resolution-only and let the existing outbound drain own canonical retry state for fixed peers and persistent addnode peers on every route. Completion drives bounded backoff; ready sessions reset it, removed pins drop it, and one-shot requests retain no retry state. Keep route-specific AddrMan failure attribution at the transport/listener boundary. Sync impact: outbound peer recovery and work driving affect SYNC-04/06, without editing validation, frontier or commit owners. Three pinned policy/TCP regressions, 42 existing service tests, 12 frontier recovery tests and the tick delivery proof passed. Strict P2P Clippy and node/RPC all-target checks passed; external overlays and process restart campaigns remain unrun.
Let the SAM lifecycle worker unpublish the session and wake only leases carrying its exact epoch token. PeerTable remains the owner of connection identity and removal; stale cleanup cannot cancel a successor epoch. Add a recovery regression covering old leases and a replacement connection. Add RPC coverage that anonymous Tor handles can be listed/disconnected but cannot enter discovery or addnode. Set the production-writer test timeout before cloning its socket, so Windows exercises the intended write deadline. Preserve the concurrent author changes for anonymous remote identities, unified pinned retries and canonical pre-release AddrMan v1. Sync impact: lease retirement affects SYNC-04/06 recovery and delivery, with SYNC-02 frontier continuity covered by existing frontier recovery tests. No validation, authoritative frontier or durable commit path changes. Validation: offline locked P2P lib 929 passed, 1 external Tor test ignored; RPC network 54 passed; all-target strict Clippy for primitives, mempool, p2p, node and rpc; fmt and diff checks passed. Real Tor/IVNP interoperability, binary/workspace/native campaigns remain unrun.
Use PeerLease::is_cancelled for inbound and outbound payload readers instead of exposing a separate local cancellation flag. Retain SAM lifecycle ownership and exact-epoch lease wakeups. A trickling old stream cannot strand the active dial after epoch revocation; recovery tests also protect replacement and unrelated epochs. Sync impact: peer loss and transport delivery can affect SYNC-02/04/06; authoritative frontier, validation and chainstate commit implementations are unchanged. After integrating the concurrent epoch-scoped teardown change, payload cancellation, exact-epoch wakeups, mocked SAM recovery (26 passed, 1 external test ignored), anonymous RPC handling, frontier recovery (12), and tick delivery (1) passed. Strict P2P Clippy, node/RPC all-target checks and formatting passed. An AddrMan anchor-consumption test failed once in a parallel suite; isolated and full 69-test reruns passed, with cause unconfirmed. External router/process convergence remains unrun.
|
Response to the four-point review, at current head 8c8eac2 (including my 4d7e5b8 follow-up):
Local validation repeated at 8c8eac2 on Windows, Rust 1.99, offline/locked: P2P library 929 passed, 1 ignored; RPC network 54 passed; strict all-target Clippy for primitives/mempool/p2p/node/rpc; fmt and whitespace checks passed. The ignored test needs an explicitly provisioned Tor SAFECOOKIE endpoint. A Windows-only fixture issue found during validation was corrected by setting the write timeout before cloning the writer socket; the timeout regression now exercises its intended deadline. Sync impact: peer recovery/delivery can affect SYNC-04/06, and SYNC-02 frontier continuity is covered by the existing frontier recovery and tick-delivery proofs included in the passing P2P suite. This follow-up does not edit validation, authoritative frontier, or durable commit ownership. CI at 8c8eac26 passed: fmt, deny, Clippy, test-crates and ci-gate. CodeQL, script-tools, fuzz build/lockfile and template-provider lockfile checks also passed. The earlier 4d7e5b8 run was superseded by the author's follow-up push: its test job was cancelled and its aggregate gate consequently failed; it is not being reported as a passing run. No real Tor/IVNP campaign, dependency installation, workflow dispatch, or network configuration change was performed. Real interop acceptance and the merge-group binary/workspace/native lanes remain separate evidence gaps; this is not a claim that the original end-to-end acceptance is complete. Human review remains open. |
Approve (comment only, not a formal review) at
|
End-to-End Live Interoperability Test Report: Tor & I2P (IVNP)Following up on the verification note regarding overlay interop testing (ref), an end-to-end live measurement campaign was conducted using Both external overlay test suites passed cleanly with 100% success. Test Environment & BackendsEvidence recorded in
Verification Summary1. Tor Onion Service (
|
Main brings #1684 opt-in Tor/I2P overlay, #1685 public-listener PeerRemote design, plus #1676 descriptor scan and #1677 compact-scan bound. Branch brings state-payload plumbing (BlockPayload wire, BodyInterest admission, index bundle validation). Resolutions: - crates/node/src/config.rs: keep both validations; branch validate_index_bundle plus main p2p.overlay.validate. - crates/p2p/src/listener.rs send_block: main PeerRemote parameter on branch BlockPayload body; single-conversion entry. - crates/p2p/src/listener.rs handle_compact_outcome: keep branch Option follow-up return (single enqueue_owned_body_fetch publisher per branch 5dee503) with main PeerRemote parameter. Main header-carried body_fetch_owned mark not restored: branch sync/headers.rs and inbound.rs intentionally supersede it with one atomic enqueue; restoring would reintroduce the racy double-mark 5dee503 deleted. - aggregate_budget_tests.rs: main PeerEndpoint address with branch is_none assertion matching the Option return. - crates/rpc/tests/core_compat.rs: main ipv4/ipv6/onion/i2p networks assertion (branch had dropped the stale len-3 check).
Merges origin/main at 46de452 (Tor/I2P #1684 over #1685 public-listener split) into stack/11-tools-docs (shared observer/retention ports, ZMQ body interest, corpus and validation-tooling docs). Listener conflicts (main design wins; no ListenerRole or rpc-public-bind restored): crates/p2p/src/listener.rs keeps the branch sync-owner handle_compact_outcome returning Option<(Hash256, Message)> via enqueue_owned_body_fetch, because the merged tree unanimously uses that API (enqueue/defer/resolve_owned_body_fetch, 3-arg send_headers; record_owned_body_fetch and body_fetch_owned exist nowhere else in the merge). Adopts main's #1684 additive change on the same function: peer_addr is impl Into<PeerRemote> with let peer_addr = peer_addr.into(). crates/p2p/src/listener/aggregate_budget_tests.rs keeps the branch .is_none() assertion for the Option return while adopting main's PeerEndpoint::from address construction. All other paths auto-merged (mempool admission/gateway/mutation/orphan, node config/options, rpc core_compat, sync, docs); Cargo.lock merged without conflict so no metadata regeneration was needed.
Merges origin/main (46de452: #1684 opt-in Tor/I2P connectivity, #1685 RPC public/management listener split) into the mining-pool stack. Intent preserved from main: PeerRemote endpoint identity across the listener (no ListenerRole or rpc-public-bind restoration), overlay config validation, and the ipv4/ipv6/onion/i2p getnetworkinfo compat pins. Intent preserved from the branch: Stratum mining-pool config and validation (validate_index_bundle, --pool gating), the exact block payload owner (Message::Block carries BlockPayload; InboundBlock stores the wire payload via InboundBlock::new), and the unified compact-block fetch-ownership contract (handle_compact_outcome returns the follow-up for the single enqueue_owned_body_fetch site). AddrMan (auto-merged, no conflicts): branch canonical v1 shape holds -- FORMAT="bitcoin-rs-addrman", SCHEMA_VERSION=1, Stored.format preserved, old unmarked pre-release files never decoded, no migration decoders; main's #1684 functional changes apply onto that shape. Mempool (auto-merged, no conflicts): branch BodyInterest, admission, gateway, mutation, and orphan intent kept alongside main's newer behavior; no side silently dropped. Discarded listener intent: main's split (Block, serialized) listener plumbing and bool compact-outcome contract give way to the branch's exact-payload/ownership shapes the merged wire and inbound types already carry; branch's SocketAddr addr types give way to main's PeerRemote/PeerEndpoint identity. Resolutions: - crates/node/src/config.rs: keep both validators, validate_index_bundle plus p2p.overlay.validate. - crates/p2p/src/listener.rs send_block: main's peer_addr: impl Into<PeerRemote> with the branch's payload: BlockPayload parameter. - crates/p2p/src/listener.rs handle_compact_outcome: branch's Option<(Hash256, Message)> return (matches the merged enqueue_owned_body_fetch caller and BlockPayload body) with main's let peer_addr = peer_addr.into() prologue. - aggregate_budget_tests.rs: main's PeerEndpoint address with the branch's .is_none() assertion for the Option contract. - crates/rpc/tests/core_compat.rs: main's expanded networks assertion (branch had removed the length check without replacement).
Merge origin/main (Tor/I2P overlay #1684, public listener #1685) into the index-snapshot lane. Branch intent preserved: BlockPayload unification, index bundle snapshot config/tests, and layered config coverage. Resolutions: - crates/node/src/config.rs: union; keep validate_index_bundle (branch) and p2p.overlay.validate (main), one call each. - crates/p2p/src/listener.rs send_block: main PeerRemote param (impl Into<PeerRemote> + into()) with branch BlockPayload body; all nine merged call sites pass payload, so main-only (block, serialized) would break every caller and the already-merged forward_block payload form. - crates/p2p/src/listener.rs handle_compact_outcome: main PeerRemote param with branch Option<(Hash256, Message)> flow; the merged caller in process_compact_wire_message destructures Some((hash, follow_up)), so main-only bool would break it. No ListenerRole restored. - crates/p2p/src/listener/aggregate_budget_tests.rs: main PeerEndpoint addr with branch .is_none() assertion. - crates/rpc/tests/core_compat.rs: main ipv4/ipv6/onion/i2p networks pin (replaces stale len==3 the branch had dropped); branch BlockPayload test changes kept via automerge.
Main brings #1684 AddrMan overlay support and #1685 public-listener addressing (PeerRemote/PeerEndpoint, overlay validate, expanded getnetworkinfo compat). Branch preserves AssumeUTXO index-bundle config plus the BlockPayload/linearizable-compact-outcome listener lineage from its parent stacks. Resolutions: - crates/node/src/config.rs: union; keep validate_index_bundle() (branch) and add p2p.overlay.validate() (main). - crates/p2p/src/listener.rs send_block: main's impl Into<PeerRemote> peer with branch's BlockPayload body; every merged caller (message loop, compact outcome, unit tests) passes a payload against wire Message::Block(BlockPayload), so the (Block, serialized) signature has no live caller in this tree. - crates/p2p/src/listener.rs handle_compact_outcome: keep the Option-follow-up/enqueue_owned_body_fetch design all merged callers use, with main's PeerRemote addressing at the boundary. - aggregate_budget_tests.rs: main's PeerEndpoint addr with the Option-style .is_none() assertion matching the kept signature. - crates/rpc/tests/core_compat.rs: take main's expanded networks assertion (branch had dropped the stale len==3 check); all other auto-merged cases from both sides kept. - No ListenerRole or rpc-public-bind restored; Cargo.lock regenerated via cargo metadata.
Merge origin/main (46de452, #1684 Tor/I2P overlay + #1676/#1677 scans) into the Lightning lane. Listener (crates/p2p/src/listener.rs, aggregate_budget_tests.rs): main's #1685 addressing wins — PeerRemote/PeerEndpoint throughout, no ListenerRole or rpc-public-bind restored. The compact-outcome path keeps the branch's enqueue-owned-body-fetch design (Option return, single linearizable enqueue site) because the merged tree carries the branch's 3-arg send_headers/InboundHeaders and enqueue-only BlockSync API; main's bool/record variant has no matching API left post-merge. Tests updated to PeerEndpoint with the is_none assertion. All other auto-merged regions keep both intents: branch Lightning/ZMQ/admission work plus main's overlay, scan-bound, and RPC compat changes.
Merges origin/main at 46de452 (Tor/IVNP overlay connectivity, #1684, atop the #1685 public-listener split) into the performance-evidence lane. Listener (crates/p2p/src/listener.rs, aggregate_budget_tests.rs): main's #1685 addressing design wins - peer_addr stays impl Into<PeerRemote>, test addrs stay PeerEndpoint, no ListenerRole, no rpc-public-bind. The compact-outcome ownership path keeps the branch's enqueue design (Option return, single linearizable enqueue_owned_body_fetch call site, 3-arg send_headers without body_fetch_owned) because the merged sync/inbound tree carries only that API: record_owned_body_fetch and body_fetch_owned exist nowhere in the merged tree, so main's bool/record caller could not resolve. Branch block_not_found/NotFound release path and block_not_found_tests kept; their sync.block_not_found dependency merged cleanly. All other auto-merged hunks (mempool admission/gateway/mutation/orphan, node config/options, sync, tx_relay, rpc compat, docs, Cargo.lock) staged as merged. Discarded: main's bool handle_compact_outcome body and record_owned_body_fetch caller, incompatible with the merged sync ownership API.
Main intent (46de452, #1684 Tor/I2P): migrate listener peer addresses from SocketAddr to PeerEndpoint/impl Into<PeerRemote>, adding the .into() conversion and PeerEndpoint test fixtures. Branch intent (stack compact-ownership rework): handle_compact_outcome returns the follow-up as Option<(Hash256, Message)> for the single enqueue_owned_body_fetch site; send_headers/InboundHeaders drop the body_fetch_owned flag in favor of scheduler-owned publication. Resolution (crates/p2p/src/listener.rs, aggregate_budget_tests.rs): adapt main's endpoint migration onto the branch mechanism the rest of the merged tree already shares (sync.rs, inbound.rs, sync/headers.rs auto-merged to the branch shape): handle_compact_outcome keeps the branch Option return and body while taking main's peer_addr: impl Into<PeerRemote> plus the let peer_addr = peer_addr.into(); conversion; the budget test keeps the branch .is_none() assertion while taking main's PeerEndpoint fixture. Discarded intent: main's bool/fetch_issued/record_owned_body_fetch path within these two files (its callees -- record_owned_body_fetch, InboundHeaders.body_fetch_owned -- were removed tree-wide by the branch side and live outside this merge's files); branch's SocketAddr parameter and fixture types (superseded by main's PeerEndpoint migration). No ListenerRole or rpc-public-bind content exists on any side of these files.
Merges origin/main (46de452: #1684 Tor/I2P overlay connectivity plus #1685 public-listener work) into the 03-rpc lane (registration-time observer interest, ZMQ body interest on rawtx, retained RPC allocations). Listener conflicts (crates/p2p/src/listener.rs, aggregate_budget_tests.rs): resolved by composing main's PeerEndpoint/PeerRemote address generalization (impl Into<PeerRemote> params, into() conversion, PeerEndpoint test addr) with the branch Option/enqueue_owned_body_fetch ownership protocol that the merged tree implements (3-arg send_headers, 3-field InboundHeaders, sync enqueue API). Neither bare side compiles: main's bool signature against the branch body (Some/None returns, if-let-Some caller) is a type error, and the branch signature without main's into() line misuses the generalized generic param. No ListenerRole or rpc-public-bind content exists anywhere in the tree, so nothing of that design was restored. Discarded alternative (documented): restoring main's bool/fetch_issued/record_owned_body_fetch callee end-to-end would require rewriting auto-merged files outside the conflicted set (sync.rs method, send_headers arity, InboundHeaders constructors, staged sync tests that call the enqueue API) and would drop the lane's body-interest plumbing; left for Main to revisit if desired.
Main intent: public listener (#1685 lineage) and AddrMan v1 plus Tor/I2P overlay transport (#1684) land in origin/main. Branch intent: BIP331 package relay, BIP332 stale-tip relay, BIP434 negotiation, mempool admission/gateway/mutation/orphan, and relay policy stay. Resolutions: - p2p listener.rs + aggregate_budget_tests.rs: main design wins; RelayOffers + relay_offers/dial_routes plumbing kept for branch config. - AddrMan: single v1 shape (FORMAT=bitcoin-rs-addrman, SCHEMA_VERSION=1, Stored.format); old unmarked pre-release files preserved, never decoded; no migration decoders or historical fixtures. Main #1684 functional changes (validation, scoping, ASMap, anchors, atomic publish, addrv2 gossip enablement) adapted to that shape. Branch addrv2 rotation kept. - Endpoint identity: branch PeerEndpoint enum kept (pervasive common use); main PeerAddress/PeerNetwork/PeerRemote kept; enum gains main-compatible API (address/new/socket_addr/to_canonical/is_ipv4+6, From<(T,u16)>, FromStr, Serialize/Deserialize, NetworkLabel mappings). Single crate::OverlayConfig (main overlay::OverlayConfig); branch DialRoutes/RouteTable/prefilter kept and wired via overlay.dial_routes. - PeerSession/PeerInfo/PeerToken keep branch field names plus main addr (PeerRemote); peer_table keyed by PeerRemote with anonymous handling preserved. - Discouragement/BannedReader, eviction keys, sync staller keys: main PeerAddress/PeerRemote routing wins; branch tests ported. - Node config/options: main settings kept; one overlay field; main onlynet (PeerNetwork) with Core-conf keys restored (proxy, onion, torcontrol, i2psam, i2pacceptincoming, onlynet); torpassword dropped (no main equivalent). - Discarded/superseded: ListenerRole never restored; rpc-public-bind untouched; branch overlay startup/TorControlOwner/establish fns and their tests (superseded by overlay.start); branch tor_control module (superseded by overlay/tor); legacy migration decoders and fixtures; duplicate struct fields/exports/imports from auto-merge. Verification: zero unmerged paths; git diff --check clean; strict conflict-marker scan clean; cargo check -p bitcoin-rs-p2p --lib clean. Test targets have residual type-mismatch errors from the identity unification (documented for follow-up); parent gates own test proof.
Adds opt-in Tor SOCKS5/ControlPort and IVNP SAM connectivity through the existing admission, PeerTable/PeerLease, relay and BlockSync owners. Logical IP/Tor-v3/I2P destinations remain separate from local proxy/router sockets.
Synchronization impact
Peer identity and transport delivery touch the input/recovery boundary of authoritative BlockSync scheduling and receive paths. Header/body validation, authoritative frontier implementation and chainstate commit mechanics are unchanged. Relevant clauses are SYNC-02 (frontier continuity), SYNC-04 (peer-loss/replacement recovery) and SYNC-06 (delivery/work drivers).
The synchronization contract's smallest executed proofs are frontier_recovery (12 tests) and tick_fetches_new_tip_headers_from_at_tip_peers (1). Expiry-strike, addrman_restart_e2e and full process convergence retain their existing deep lanes and were not rerun in these follow-ups.
Validation and remaining evidence
Windows focused checks passed:
One AddrMan anchor-consumption test failed once in a parallel suite; its isolated rerun and the full 69-test rerun passed. The cause is unconfirmed; this is not evidence of stable passing behavior across repeated Windows persistence campaigns.
Real Tor/IVNP interoperability, process convergence and privacy qualification remain unrun. #1687 still needs to adopt the same canonical fixture/layout and identity/netgroup rules before merge; application-key compatibility across the branches is not qualified. Password/Unix ControlPort support and broader provider-backoff timing qualification remain outside these refactors. No bidirectional historical AddrMan migration is intended under #1688.
Refs #1601, #1688. Keep #1601 open for external qualification.
No actionable defect was established; the PR appears safe to merge within its stated scope.Summary
Adds opt-in Tor and IVNP I2P routes through the existing peer connection owners.
No new actionable issue was established. The supplied previous threads are unnumbered, so they require no numbered ledger entries. Their fixes are present: keys are retained across publication retries, explicit remote failures reach address-book accounting, and all three independent lockfiles include
rustix.This review used code inspection. No tests or external interoperability campaigns were run.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR Config[Routing configuration] --> Resolve[Resolve allowed destinations] Resolve --> Drain[Outbound drain and retries] Drain --> Route{Destination network} Route --> TCP[Direct TCP or SOCKS] Route --> Tor[Tor SOCKS] Route --> SAM[IVNP SAM session] TCP --> Lease[PeerTable and PeerLease] Tor --> Lease SAM --> Lease Inbound[Tor and I2P inbound streams] --> Admission[Shared inbound admission] Admission --> Lease Lease --> Delivery[Existing sync and relay paths] Epoch[SAM epoch retirement] --> Cancel[Cancel only matching leases] Cancel --> LeaseReviews (17) · Last reviewed commit: "Merge branch 'main' into feat/1601-overl..." · Reviewed by Greptile