Repository navigation
Land verified protocol stack - #1697
Conversation
Peer identity keyed by SocketAddr cannot name Tor or I2P peers and lets a proxy loopback address stand in for the logical peer. A typed endpoint now carries identity through the peer table, addrman, and RPC surfaces, while the transport address stays socket-local. Addrman records migrate to version 9 through the existing backup path with byte-identical IP records. Overlay dials fail closed until the SOCKS and SAM transports land; direct IP dialing is unchanged.
Add bounded SOCKS, Tor control, and SAM transport paths. Keep logical peer identity separate from transport addresses. Fence retired overlay generations so stale sessions cannot enter a new epoch. Preserve full-width peer identifiers in network RPCs. Include strict-lint extractions and typed peer callers so the transport integration remains buildable. See also: #1601
Add the pinned overlay recorder with bounded process lifetimes and sanitized evidence. Keep readiness and protocol observations separate so missing prerequisites cannot produce a false pass.
Host loopback probes could enter simulated bridge sessions and race root recovery assertions. Track session-owned sockets in tests and keep bridge shutdown idempotent so external probes cannot alter protocol evidence.
Use a private Tor network and a compliant i2pd SAM fixture without public bootstrap or operator state. Keep recorder streams transparent and require observed readmission before a successful verdict. Preserve failed restart evidence. Initial transport works, but same-key I2P readmission is not qualified with the pinned fixture.
Require DNS seeds to be disabled when proxy, onion, or I2P routing is configured, and reject hostname fixed peers that would resolve locally. Typed IP, Tor, and I2P endpoints remain supported with explicit validation tests.
Negotiate feature announcements before verack with peers that support protocol version 70017. Bound identifiers and feature data and preserve the legacy handshake for older peers. See also: #1634
Retain bounded stale-tip announcements and relay them only to peers that negotiate BIP332. Carry demoted-tip facts from chain admission and invalidation through the node adapters. Keep the relay disabled unless the operator enables it. See also: #1637
Keep the bounded selected opening window pending until the peer send succeeds, the tip retires, or the connection is forgotten. Body availability and queue refusal must not consume an offer. All six new regressions fail against the original retainer and pass with the fix. Preserve the ten-tip opening cap, origin suppression, and bounded global retention without extra retry-window copies.
Negotiate package relay and serve bounded ancestor and package requests. Charge package bodies to one ingress ledger and admit peer packages atomically through the mempool. Bind claims to their source connection and keep package relay disabled by default. See also: #1635
Resident orphans need an owned ancestor-package request before a package reply can enter admission. Publish negotiated capabilities with the ready session and retain one bounded request lifecycle through admission, expiry, and disconnect.
…oversized claims, bound package walk
…acer notfound, unidentified parent lookup, connection-id probe rotation Filter soft-blocked body candidates before truncation with an all-soft fallback so a stalled peer cannot crowd healthy servers out of a multi-peer request scan. Record notfound answers from tracked prefix probe racers without releasing the pending owner. Resolve anonymous inbound sessions for parent requests through unidentified-aware lookup. Rotate header probes by connection id rather than endpoint.
…d, preserve demoted tips, guard duplicate fetches Listener relays stale-tip announcements with the announcer's identity but without wire-response semantics, so the batch never consumes the pending getheaders request nor feeds a live download-twice cursor with a foreign side-branch page. The headers drain applies the same minimum-work/fault hold as ordinary headers: valid low-work pages defer instead of admitting past the floor, broken pages admit directly to report the error and carry peer fault. A rejected batch that committed a higher-work prefix preserves the displaced tip via tip-movement inference instead of losing it. Both direct-fetch branches skip bodies the compact path already owns. Regressions: partial-prefix demoted preservation (chain), low-work defer vs broken admit, partial-prefix preservation, duplicate-fetch skip (sync), identified-peer non-wire routing (listener).
Duplicate-fetch and partial-prefix tests now send InboundHeaders through inbound_headers_tx and drain_inbound_headers, asserting via the peer receiver (no GetData when owned, GetData for unowned control) and via the stale-tip retainer (displaced tip retained). The low-work test keeps pinning the minimum-work hold at the presync anchor and body-carried gate.
Delete dead package result fields, narrow internal visibility, and remove the stale line-count expectation while preserving atomic package verdicts.
Use contextual test failures, explicit state defaults, and accurate lint expectations while preserving all protocol assertions.
Drop an expectation that no longer matches block_verbose_typed.
Extract journal configuration validation and describe the implemented default-off BIP332 offer and activation gates with local proof.
Describe the default-off package-relay offer, role bits, handshake and IBD gates, and message enforcement with existing proof tests.
Make unidentified peer registration test-only, share chain-tip publication and IBD checks, and read the applied tip once per recovery snapshot.
Expose the existing peer registration test seam to RPC tests and assert the negotiated protocol version instead of a stale literal.
Rotate addrv2 gossip with current overlay filters, route terminal package solicitations to missing-parent requests, and cite P2P-01's executable proofs.
Map Core dnsseed into node configuration, require its privacy opt-out in the overlay fixture, and assert the current addrman schema version.
There was a problem hiding this comment.
Review Summary
This is a large protocol stack PR (BIP331, BIP332, BIP434, stale tip relay, tx ingress, addrv2) with 184 changed files and ~37k additions. The CI gate passed (native-gate-20261027T191330Z-1920978).
Areas reviewed in depth:
crates/mempool/src/admission.rs— admission retry logic, orphan/fence semantics,batch_requestscrates/mempool/src/submit_package.rs— newsubmit_peer_package,check_ancestor_shape,PackageAttemptsplitcrates/mempool/src/standardness.rs—MAX_PACKAGE_WEIGHTconstant extractioncrates/mempool/src/package.rs— weight bound reference updatecrates/p2p/src/sync/stale_tips.rs— BIP332StaleTipRetainer(fully new)crates/p2p/src/staletip.rs— BIP332staletipcodec and test vectorscrates/p2p/src/packages.rs— BIP331ancpkginfo,getpkgtxns,pkgtxns,combined_package_hashcrates/p2p/src/handshake.rs— BIP434 feature negotiationcrates/node/src/tx_ingress.rs— peer package ingress pipeline
Note: An earlier inline comment on check_ancestor_shape (lines 60–73 of crates/mempool/src/submit_package.rs) explores the logic and concludes no action is needed — that comment should be disregarded; the function is correct.
No merge-blocking defects were found. The admission retry fencing, package shape validation, stale tip relay invariants, and BIP331/332/434 protocol implementations are consistent with their stated contracts. The ordered function correctly delegates check_structure to submit_package_with_times. The time: 0 placeholder in batch_requests for preview paths is intentional — submit_package_attempt overrides it immediately after, and preview_transactions does not commit anything.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
Key peer metadata by exact PeerSource so anonymous onion sessions can serve bodies, and prefilter automatic dials by route availability. The remote branch already fixes package ancestor ordering.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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.
Follow-up merge of origin/main e40477e (verified tools/docs stack). Resolves two new conflicts, preserving the prior merge decisions: - listener/aggregate_budget_tests.rs: main's Option-returning handle_compact_outcome assertion (.is_none()). - p2p sync/requests.rs tests: union of branch scheduler-coverage helpers (test_addr/test_chain/scheduler_with_owner_pending) with main's UnavailableBodies ledger tests; constructor unified to UnavailableBodies::new(). No push.
Merge left test code on the old identity model while lib moved to endpoint-identity (PeerEndpoint/PeerRemote/PeerSource). Update test-only helpers and call sites at root, no shims, no lib changes: - sync/requests: close racer test fn, drop duplicated imports from bad union merge; disambiguate for_test SocketAddr. - addrman tests: Candidate/Anchor endpoint -> addr; fix ip/parse_endpoint helpers to PeerEndpoint patterns per main. - peer_table/peer_info/overlay/pinned/service: pass PeerEndpoint/PeerRemote directly (remove PeerEndpoint::Ip double-wrap); add PeerInfo endpoint/network; wrap addr_bind with TransportRemote::new; source_ip(SocketAddr) only for sockets, else source(Some(endpoint)); fix disconnect_matching Vec<PeerRemote> asserts. - sync tests: is_connected/register/publish_info take Into<PeerRemote>, pass PeerRemote through; fix session endpoint comparisons; source(Some(ep)) for PeerEndpoint. - listener tests + inv/tx_relay: same Into<PeerRemote> fixes; TransportRemote::new for sockets; correct call arities; drop stale publish_info_and_notify_ready; fix PeerToken addr field. - tx_policy: remove Ip() double-wrap; source(Some()) for endpoints; explicit SocketAddr::from for for_test tuples. - inv test: import SocketAddr; peer_table asserts expect PeerRemote::Endpoint. cargo check -p bitcoin-rs-p2p --all-targets: green (0 errors). cargo check --workspace --all-targets --locked: still red in unrelated crates (rpc network.rs syntax; mempool submit_package MempoolObserver/trait mismatch) - pre-existing at HEAD, untouched by this p2p test-only diff; reported separately.
- rpc getpeerinfo: replace broken v31 inline struct plus stray peer_info_wire tail with direct PeerInfoWire path (u64 node_id, endpoint/network via peer_info_wire); preserves stack row shape and main field coverage. - rpc disconnectnode: drop required-address plus positional-index duplicate; use optional address with single u64 nodeid and disconnect_ready variants; anonymous overlay selectable by nodeid. - rpc overlay tests: onion->onion_proxy, proxy_randomize_credentials-> randomize_credentials, NetworkLabel::Onion->PeerNetwork::Onion. - mempool submit_package: remove obsolete wants_accepted_transactions; pass observer as (observer, BodyInterest::Metadata). - node chain_effects: add missing PeerToken endpoint field. - node open: drop duplicate overlay field, keep identity_dir join. cargo check -p bitcoin-rs-p2p --all-targets: green. cargo check --workspace --all-targets --locked: green.
|
@codex review |
…nfig contracts Keep branch protocol surface (package-relay/stale-tip relay offers, overlay/endpoint identity, pkg relay wire messages, feature/staletip tests) on main's merged contracts (BlockPayload single-decode wire, index-bundle config, journal retention docs, Block(BlockPayload)). Regenerated nothing: Cargo.lock auto-merged without conflict.
…2p stubs to payload APIs - node/config: restore validate_overlay for/fn closing braces and Ok(()) dropped by the overlay/index-bundle union. - p2p/requests test: pass &mut RefusalFilter to next_peer_request. - p2p/headers RoutingStubChain: commit_window takes &[&BlockPayload], switch_to_branch staged_body returns Option<BlockPayload>. cargo check --workspace --all-targets --locked: exit 0.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87cd6485a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
When package relay negotiates successfully, publish the bits via PeerTable::note_package_relay for the current source before notifying ready consumers, so peer-table snapshot and TxIngressConsumer see the ANC capability. Add negotiated_package_bits_publish_before_ready regression test.
Extend workers.listeners with overlay.start handles so startup rollback and join_core_workers wait for overlay teardown instead of dropping (detaching) them. Add overlay_workers_are_retained_for_join_and_rollback lifecycle test.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62729a50eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Construct inbound metadata without requiring a destination endpoint, preserving AnonymousTor and setting network to Onion instead of unwrapping endpoint(). Add anonymous_tor_inbound_preserves_identity_and_onion_network test.
shutdown() cancelled worker leases before calling anchor_peers(), which excludes cancelled leases, so no restart anchor was ever persisted on a live shutdown. Capture the anchor set first, then cancel workers and connections. Harden the shutdown-anchor test: install a Workers epoch so the cancellation path runs (workers present, lease observably cancelled). Under the old ordering the test fails with no anchors persisted.
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
standardness.rs: keep main fee block (modified_fee_res/fee_for_max). submit_package.rs: union Txid import; keep branch PeerToken/Arc/overlay lifecycle, main results/package_error returns. util.rs: keep main getrpcinfo active-calls tests; branch whitespace only. docs/rpc-reference.md: keep both disconnectnode (branch) and submitpackage (main) Deviation rows; counts 34/56/5/83 total 178.
The merge left two submit_package_attempt methods (LocalOrigin thin wrapper plus AdmissionOrigin full implementation); Rust rejects the duplicate definition. Keep the AdmissionOrigin implementation as the single owner, route submit_package_with_times through check_structure plus submit_package_atomic, and convert the two RPC call sites with AdmissionOrigin::from(LocalOrigin::Rpc).
Draft subsystem stack split from PR #1687.
Base: f024fc3
Tip: ffeb1b8
Full repository gate: native-gate-20261027T191330Z-1920978 (scripts/ci-pr.sh all, exit 0).
Scope: protocol (BIP331, BIP332, BIP434, stale tip, tx ingress, addrv2).
This is a verified stack tip. Do not rebase or reparent it
without owner approval.
No better-than-Core claim. No mainnet qualification.
See also: #1687


Confidence Score: 5/5
The reviewed changes appear safe to merge; both previous findings are addressed.Summary
This stack adds package relay, stale-tip relay, transaction ingress changes, and overlay networking.
The latest changes address both previous findings:
No new actionable issues were found in the changes since the previous review.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Automatic peer candidate] --> B{Network and route allowed?} B -->|No| C[Skip before queuing a dial] B -->|Yes| D[Dial and complete handshake] D --> E[Ready session with exact connection identity] F[Anonymous inbound peer] --> E E --> G[Foreground or historical block request] G --> H[Check identity before sending]Reviews (3) · Last reviewed commit: "Fix anonymous peer serving and route pre..." · Reviewed by Greptile