[PastaPastaPasta]

Issue being fixed or feature implemented

The backport of bitcoin#22729 (#7300) changed what happens when a node is started with an explicit -bind=<addr> but no -bind=...=onion. Before, Dash Core always added the dedicated onion listener 127.0.0.1:9996 (19996 testnet, 19896 regtest) and pointed the automatically created Tor onion service (-listenonion, on by default) at it. Now it binds no onion listener and points the onion service at the first -bind address.

That listener is not in onion_binds, so every peer Tor forwards to it is classified by its source address: Tor's loopback address, or the node's own IP when -bind is a routable address. It is not classified as an onion peer. As a result, a node that runs an onion service next to a clearnet address (-externalip, or a discovered/bound routable address) advertises its clearnet address to peers that reached it through its .onion address. That links the two identities, which the privacy-network check in GetLocal() is meant to prevent. Those peers also lose the rest of the onion-specific handling, such as getpeerinfo network and eviction protection for onion peers.

The release note added for #7300 has a related problem. It says -listenonion=0 drops the implicit onion bind, but it never did: the bind is added before -listenonion is looked at. An operator whose 127.0.0.1:9996 is taken who follows the note still gets a node that refuses to start.

Both changes are only in v24.0.0-rc.1 and rc.2.

Why this is a real problem

Code path at c14104b:

• src/init.cpp#L2678-L2695: with vBinds non-empty and no =onion bind, onion_service_target = connOptions.vBinds.front() (L2682). It is not pushed into onion_binds, and StartTorControl(onion_service_target) still runs (L2694).
• src/net.h#L1291 copies only onion_binds into m_onion_binds. src/net.cpp#L1977 sets inbound_onion only for listeners in m_onion_binds.
• src/net.cpp#L584-L592: without m_inbound_onion, ConnectedThroughNetwork() and IsConnectedThroughPrivacyNet() fall back to the peer's source address. The privacy filter in GetLocal() (L186-L193) then skips the node's own onion address for that peer and lets the clearnet one through.

I reproduced this on the unfixed build with a small functional-test-style script. A minimal fake Tor control port accepts PROTOCOLINFO/AUTHENTICATE/ADD_ONION and records the target dashd asks Tor to forward to. A P2P peer then connects to exactly that target, as Tor does for every inbound onion connection, and sends getaddr. The node runs with -bind=127.0.0.1:<port> -listenonion=1 -externalip=1.2.3.4.

Unfixed (c14104b):

Tor control commands received: ['PROTOCOLINFO 1', 'AUTHENTICATE', 'GETINFO net/listeners/socks', 'ADD_ONION NEW:ED25519-V3 Port=19899,127.0.0.1:17626']
onion-service target handed to Tor: 127.0.0.1:17626
getnetworkinfo localaddresses: [{'address': '1.2.3.4', 'port': 17626, 'score': 4}, {'address': 'pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion', 'port': 19899, 'score': 4}]
Connecting a peer to 127.0.0.1:17626, as Tor would for an inbound onion connection
getpeerinfo: addr=127.0.0.1:63442 addrbind=127.0.0.1:17626 network=not_publicly_routable
addresses self-advertised to this peer: ['1.2.3.4:17626']

The onion service forwards to the clearnet listener, the forwarded peer is not_publicly_routable instead of onion, and it is sent the node's clearnet address 1.2.3.4.

In the fixed run the list is empty for two reasons. The privacy filter withholds 1.2.3.4 from an onion peer, and the test P2PInterface never sends sendaddrv2, so the node cannot send it the v3 onion address either. The fixed run also prints a wait_until() failed line after the 10 s wait for addresses; that line is left out of the excerpt above.

With this PR:

Tor control commands received: ['PROTOCOLINFO 1', 'AUTHENTICATE', 'GETINFO net/listeners/socks', 'ADD_ONION NEW:ED25519-V3 Port=19899,127.0.0.1:19896']
onion-service target handed to Tor: 127.0.0.1:19896
getnetworkinfo localaddresses: [{'address': '1.2.3.4', 'port': 18132, 'score': 4}, {'address': 'pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion', 'port': 19899, 'score': 4}]
Connecting a peer to 127.0.0.1:19896, as Tor would for an inbound onion connection
getpeerinfo: addr=127.0.0.1:50581 addrbind=127.0.0.1:19896 network=onion
addresses self-advertised to this peer: []

Reproduction script (run with WORKTREE=<src root> python3 repro_tor_target.py --configfile=<src root>/test/config.ini)

#!/usr/bin/env python3
"""Reproduction: with an explicit -bind (no =onion) and -listenonion=1, which
listener does dashd hand to Tor as the onion-service target, and how is a peer
that Tor forwards to that target classified?

A minimal fake Tor control port answers PROTOCOLINFO/AUTHENTICATE/ADD_ONION
and records the target from the ADD_ONION command. The test then connects a P2P
peer to exactly that target (what Tor does for every inbound onion connection),
prints getpeerinfo's network field, sends getaddr and prints the addresses the
node self-advertises to that peer.

Run from anywhere:
  python3 repro_tor_target.py --configfile=<worktree>/test/config.ini
"""
import os
import socket
import sys
import threading

WORKTREE = os.environ["WORKTREE"]
sys.path.insert(0, os.path.join(WORKTREE, "test", "functional"))

from test_framework.messages import msg_getaddr  # noqa: E402
from test_framework.p2p import P2PInterface  # noqa: E402
from test_framework.test_framework import BitcoinTestFramework  # noqa: E402
from test_framework.util import p2p_port  # noqa: E402

ONION_ID = "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd"
EXTERNAL_IP = "1.2.3.4"


class FakeTorControl:
    def __init__(self):
        self.srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        self.srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
        self.srv.bind(("127.0.0.1", 0))
        self.srv.listen(1)
        self.port = self.srv.getsockname()[1]
        self.commands = []
        self.target = None
        threading.Thread(target=self._serve, daemon=True).start()

    def _serve(self):
        conn, _ = self.srv.accept()
        f = conn.makefile("rwb", buffering=0)
        while True:
            line = f.readline()
            if not line:
                return
            cmd = line.decode().strip()
            self.commands.append(cmd)
            if cmd.startswith("PROTOCOLINFO"):
                f.write(b'250-PROTOCOLINFO 1\r\n250-AUTH METHODS=NULL\r\n250-VERSION Tor="0.4.8.12"\r\n250 OK\r\n')
            elif cmd.startswith("ADD_ONION"):
                # ADD_ONION NEW:ED25519-V3 Port=<virtport>,<target>
                self.target = cmd.split("Port=")[1].split(",", 1)[1]
                f.write(f"250-ServiceID={ONION_ID}\r\n250-PrivateKey=ED25519-V3:AAAA\r\n250 OK\r\n".encode())
            else:
                f.write(b"250 OK\r\n")


class AddrCollector(P2PInterface):
    def __init__(self):
        super().__init__()
        self.addrs = []

    def on_addr(self, message):
        self.addrs += [f"{a.ip}:{a.port}" for a in message.addrs]

    def on_addrv2(self, message):
        self.addrs += [f"{a.ip}:{a.port}" for a in message.addrs]


class ReproTorTarget(BitcoinTestFramework):
    def set_test_params(self):
        self.setup_clean_chain = True
        self.bind_to_localhost_only = False
        self.num_nodes = 1

    def setup_network(self):
        self.tor = FakeTorControl()
        self.extra_args = [[
            f"-bind=127.0.0.1:{p2p_port(0)}",
            "-listenonion=1",
            f"-torcontrol=127.0.0.1:{self.tor.port}",
            f"-externalip={EXTERNAL_IP}",
            "-debug=tor",
        ]]
        self.setup_nodes()

    def run_test(self):
        node = self.nodes[0]
        self.log.info(f"dashd args: {self.extra_args[0]}")
        self.wait_until(lambda: self.tor.target is not None)
        self.log.info(f"Tor control commands received: {self.tor.commands}")
        self.log.info(f"onion-service target handed to Tor: {self.tor.target}")
        self.log.info(f"getnetworkinfo localaddresses: {node.getnetworkinfo()['localaddresses']}")

        self.generate(node, 1, sync_fun=self.no_op)  # leave IBD so self-advertisement happens

        host, port = self.tor.target.rsplit(":", 1)
        self.log.info(f"Connecting a peer to {host}:{port}, as Tor would for an inbound onion connection")
        peer = node.add_p2p_connection(AddrCollector(), dstaddr=host, dstport=int(port))
        info = [p for p in node.getpeerinfo() if p["addrbind"] == f"{host}:{port}"][0]
        self.log.info(f"getpeerinfo: addr={info['addr']} addrbind={info['addrbind']} network={info['network']}")

        peer.send_and_ping(msg_getaddr())
        try:
            self.wait_until(lambda: len(peer.addrs) > 0, timeout=10)
        except AssertionError:
            pass
        self.log.info(f"addresses self-advertised to this peer: {peer.addrs}")


if __name__ == "__main__":
    ReproTorTarget().main()

Release-note claim, unfixed build, with 127.0.0.1:19896 held by another process:

$ src/dashd -regtest -datadir=$D -listenonion=0 -port=18555 -rpcport=18556 -printtoconsole=0
Error: Unable to bind to 127.0.0.1:19896 on this computer. Dash Core is probably already running.
Error: Failed to listen on any port. Use -listen=0 if you want this.
exit=1

Why it matters

Running an onion service alongside a clearnet address is a supported setup. Operators who also pass an explicit -bind, which is common on servers and masternodes, would silently lose the separation between the two once v24.0.0 ships: any peer that connects to the .onion address can learn the node's clearnet IP from self-advertisement. Exposure needs a working Tor control connection and a routable local address (-externalip, or discovery or a routable -bind), so it does not affect every node. Nodes with -bind that do not use Tor are unaffected either way. The release-note error is only an operational trap, but it would ship in the v24.0.0 notes.

What was done?

• src/init.cpp: drop the branch that used vBinds.front() as the onion-service target, so the target is always the first onion_binds entry. Without a -bind=...=onion, the default onion target is now added to onion_binds and used as the Tor target unless -bind is given and -listenonion is disabled. In that one case no onion bind is added and Tor control is not started. -listenonion is read after parameter interaction, so -listen=0 (which soft-sets -listenonion=0) is handled as before. The -bind help text now mentions the exception.
• doc/release-notes-7300.md: describe the actual rule. The implicit bind is added unless -bind is given together with -listenonion=0. -bind=...=onion moves it, -listenonion=0 alone or -whitebind alone keeps it, and -listen=0 disables all binds.
• test/functional/feature_bind_extra.py: node 2 (explicit -bind, no extra port) now says -listenonion=0 explicitly. Before, it relied on the framework's config default. A new node 3 with explicit -bind and -listenonion=1 must listen on both its -bind port and 127.0.0.1:19896, and a peer connecting to 19896 must show up with network onion. Node 3's -torcontrol points at an unused port so the test never creates an onion service through a Tor instance running on the test host.
• test/functional/feature_proxy.py: one step starts node 1 with -listenonion=1 on top of the framework's bind=127.0.0.1. With this PR that node would also bind the fixed regtest onion target 127.0.0.1:19896, which is the port feature_bind_extra.py node 3 holds, and since backport: Merge bitcoin/bitcoin#30397, 22729, 29633 #7300 a failed bind stops startup. Under test_runner.py -jN the two tests could then collide. The step now passes -bind=127.0.0.1:<tor_port(1)>=onion, the same per-node port the framework uses when it adds the binds itself. I kept this local to the one test rather than teaching TestNode.start() a new rule, because it is the only test that enables -listenonion together with an explicit bind.

Why this is the correct minimal fix

The root cause is that the onion service can be pointed at a listener that is not tagged as onion. The only listener that can be both the Tor target and correctly tagged is one in onion_binds, so the target must always come from onion_binds. The alternative of adding vBinds.front() to onion_binds would tag every clearnet peer on that port as onion, which leaks the onion address to clearnet peers and distorts eviction, so it is not an option.

Two other designs would avoid the multi-instance cost described under Breaking Changes. Both are reasonable, and the choice is up to maintainers:

• When -bind is given without =onion and -listenonion is not set explicitly, soft-set -listenonion=0 during parameter interaction. This keeps rc.1's "respect -bind" behavior and avoids the extra bind. The cost is that nodes upgrading from v23 with -bind and an automatic onion service would silently lose that onion service.
• Keep the default onion bind, but treat a failure to bind it as non-fatal and skip Tor control in that case. This brings back v23's tolerance for this one bind and gives up part of backport: Merge bitcoin/bitcoin#30397, 22729, 29633 #7300's "fail on any bind error" rule.

This PR does neither. It keeps v23's behavior for the onion service, plus the stricter bind handling from #7300.

bitcoin#22729's goal was to let users avoid the extra 127.0.0.1:9996 bind. This PR keeps that, but only when the user also says they don't want an onion service (-listenonion=0). Without an onion service there is nothing to tag. Nothing changes for nodes without -bind, with -bind=...=onion, or with -listen=0. The stricter "fail startup if any bind fails" part of bitcoin#22729 is unchanged too.

Relation to upstream

Bitcoin Core has the same bug. The open upstream fix, bitcoin#36170 ("net: require a dedicated bind for automatic Tor"), has an Approach ACK. It also removes the vBinds.front() fallback, but handles the -bind + -listenonion case differently: instead of adding the default onion bind, it refuses to start until the operator adds -bind=<addr>=onion or -listenonion=0. That turns every existing -bind setup with the default -listenonion=1 into a startup error, including many masternodes, so this PR keeps v23's default onion bind for that case.

The src/init.cpp block uses the same layout as bitcoin#36170: the default bind is added up front under one condition, and the Tor target is always onion_binds[0] inside the -listenonion branch. Backporting bitcoin#36170 later then only changes that condition and adds its errors. bitcoin#36170 also rejects a wildcard =onion bind. That is left to the backport, since it is a separate new startup error.

This should also go to the v24.0.x release branch, since the behavior was introduced in v24.0.0-rc.1.

How Has This Been Tested?

macOS arm64, --enable-debug build, functional tests run outside any sandbox.

• Reproduction script above: unfixed build shows the onion target 127.0.0.1:<bind port>, peer network=not_publicly_routable, and the peer is sent 1.2.3.4. Fixed build shows target 127.0.0.1:19896, peer network=onion, and no clearnet address is sent.
• feature_bind_extra.py with the new cases. The test is Linux-only (it reads /proc/net/tcp), so I ran it locally through a wrapper that swaps get_bind_addrs() for an lsof-based equivalent and lifts the platform skip. The test itself is unchanged. Nodes 0–2 pass on both builds.
  • Unfixed (only the src/init.cpp hunks reverted): fails at node 3 with AssertionError: not({('7f000001', 17013)} == {('7f000001', 17013), ('7f000001', 19896)}).
  • Fixed: passes, including the network == "onion" check.
• feature_proxy.py with --nocleanup, checking the Bound to lines in node 1's debug.log:
  • Fixed build without the feature_proxy.py change: node 1 binds 127.0.0.1:19896. This is the collision described above.
  • Fixed build with the change: node 1 binds only its p2p port and its tor_port(), and nothing on 19896. The test passes.
  • Unfixed build with the change: same binds, and the test passes.
• Release-note cases on the fixed build, with 127.0.0.1:19896 held by another process: -listenonion=0 alone still fails with Unable to bind to 127.0.0.1:19896, as the corrected note says. -bind=127.0.0.1:18555 -listenonion=0 starts.
• test/functional/test_runner.py feature_proxy.py p2p_eviction.py rpc_net.py (after the test changes) and earlier feature_config_args.py: all passed. feature_config_args.py failed once in a -j4 run with Unable to start HTTP server (an RPC port collision with another run on the same host) and passed on rerun. feature_bind_extra.py, feature_bind_port_*.py and rpc_bind.py are skipped on macOS by the runner. CI covers them on Linux.
• test/lint/lint-whitespace.py and flake8 with the codes enabled in test/lint/lint-python.py pass. The -bind help line keeps the single-line AddArg style of its neighbours rather than the clang-format suggestion.

Breaking Changes

Compared with v24.0.0-rc.1/rc.2 only: a node started with an explicit -bind and -listenonion enabled (the default) binds 127.0.0.1:9996 (19996 testnet) again, as v23 did. With the stricter bind handling from #7300, it fails to start if that port is unavailable.

This breaks one setup that works on rc.1/rc.2: several dashd instances on one host and one chain, each with its own explicit -bind, and -listenonion left at its default. For example, several masternodes on separate IPs on one server. On rc.1/rc.2 each instance binds only its own -bind address. With this PR every instance also tries to bind 127.0.0.1:9996; the first one gets it and every later one fails to start. v23 started all of them because a failed bind was only a warning there, and avoiding this extra bind was the reason for bitcoin#22729.

Operators with that setup need one of these per instance:

• -listenonion=0, if the instance does not need an automatic onion service; or
• a distinct -bind=127.0.0.1:<port>=onion for each instance.

Maintainers should weigh this cost against the self-advertisement problem above before merging. The alternatives listed under "Why this is the correct minimal fix" avoid this cost but make other trade-offs.

Checklist:

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

🤖 Generated with Claude Code

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (1)

doc/developer-notes.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 46ee9b33-c881-48f3-97bd-fc9994ab94bb

📥 Commits

Reviewing files that changed from the base of the PR and between 3789ec0 and 5f40d56.

📒 Files selected for processing (5)

• doc/release-notes-7300.md
• doc/tor.md
• src/init.cpp
• test/functional/feature_bind_extra.py
• test/functional/feature_proxy.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

Walkthrough

When -listenonion is enabled with explicit binds, startup now requires a non-wildcard onion bind and uses the first onion bind as the Tor service target. Startup reports an error when no onion bind exists or an onion bind is wildcard. When neither regular nor onion binds are configured, Dash Core adds the default onion target. Documentation and functional tests reflect these rules.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Merge Risk: ⚪ Minimal · up to 5f40d

With onion listening enabled, explicit binds must include a dedicated non-wildcard onion bind; this startup requirement is documented and covered by functional-test expectations. No actionable merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 5f40d

The change removes a privacy-sensitive fallback and fails closed when a dedicated onion listener is missing. Existing configurations using explicit binds may require an update before restarting, including nodes without Tor configured. No introduced security finding was identified, but live Tor recovery and deployment behavior were not exercised.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The affected security boundary is each configured node's inbound P2P identity and address advertisement. At the base, a remote onion peer could reach an ordinary listener through Tor and be treated as a non-onion peer, potentially linking that node's onion and clearnet identities. The head removes this automatic forwarding path rather than expanding remote authority.

Security Findings and Attack Paths

• inferred — The inspected startup-to-classification path supports rejecting an introduced privacy-disclosure concern: the automatic Tor target is now an onion-bind member, and the existing address-selection filter excludes clearnet local addresses for onion-classified peers. This conclusion is source-based and does not assert that every external Tor deployment has been validated.

Trust Boundaries and Controls

• observed — A direct connection to an explicitly routable =onion listener is still classified as onion, and Tor-forwarded connections can still match source-address whitelist rules. Both behaviors existed at the base. Dedicated listener selection fixes target/classifier alignment; it should not be interpreted as Tor authentication or complete whitelist isolation.

Resilience and Maintainability Implications

• observed — Tor control starts before listener initialization at both base and head, so service publication and successful binding remain non-atomic. Bind initialization fails on a configured-listener failure; the daemon then follows interruption and shutdown. Tor disconnect or controller destruction removes local service advertisement, reconnect retains the immutable target, and network shutdown clears listening sockets. These paths do not reinstate the removed ordinary-bind fallback.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (2 skipped: 2 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the dedicated onion listener fix and the related release-note correction.
Description check	✅ Passed	The description explains the onion-listener issue, the fix, the compatibility impact, and the reported tests.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[thepastaclaw]

✅ Final review complete — no blockers (commit 5f40d56) · triage: low

[thepastaclaw]

Final validation — Phase 1 + Phase 2

Static verification at f8b56ba confirms that the automatic Tor service targets an onion-classified listener, the explicit-bind opt-out remains valid, and the documentation and regression tests match the implemented behavior. Both reviewers' clean assessments are supported by the source; no actionable in-scope defects or prerequisite claims were identified. No builds or tests were run in this lane: the supplied CI snapshot shows successful container-build and formatting checks, queued manifest jobs, and no completed functional-test results.

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff is a small, contained listener-selection fix in src/init.cpp with focused functional-test updates and a release-note correction, not a large or intricate change to a critical surface.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 99% left, weekly 69% left; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

[knst]

self-ACK 5f40d56

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the complete diff at 5f40d56 and found no blocking correctness defects: automatic Tor now requires a dedicated, non-wildcard onion bind when explicit binds are configured. Four non-blocking issues remain concerning the stale PR description, omitted early validation, undocumented partial-backport exclusions, and wildcard-bind release notes. This was static verification only; the supplied CI snapshot shows lint passing, with source builds queued or running and completed build/test validation unavailable.

🟡 4 suggestion(s)

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: backport-reviewer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: backport-reviewer); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff makes a small, contained change to Tor listener initialization and startup validation in src/init.cpp, with focused tests and documentation rather than intricate changes to a critical surface.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 98% left, weekly 99% left; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — backport-reviewer (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — backport-reviewer (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/init.cpp`:
- [SUGGESTION] src/init.cpp:2679-2689: PR description no longer matches the head commit's behavior
  The PR description still says explicit -bind configurations retain the implicit onion listener and fail only if its port is unavailable. At this head, the default listener is added only when both bind vectors are empty; an explicit normal bind without an onion bind instead causes an unconditional startup error when -listenonion is enabled. Wildcard onion binds are also rejected now, although the description says that change is deferred. The documented node-3 regression scenario and its network=='onion' assertion were removed by the final commit. Update the implementation rationale, upstream relationship, breaking changes, and test evidence to describe the final policy, distinguishing earlier implementation results from validation of this head. These differences change the operator migration requirements, not just the wording.
- [SUGGESTION] src/init.cpp:2683-2685: Declared-partial omission: early Tor bind validation
  Upstream bitcoin#36170 rejects explicit bind configurations without an =onion suffix in AppInitParameterInteraction, while retaining the defensive check here. Dash includes only this late check: LoadChainstate and wallet loading occur before it. Consequently, a configuration already known to be invalid can spend substantial time loading chainstate and wallets before reporting the dedicated-bind error. Carry the upstream early check, or explain its intentional exclusion from this partial backport. The current check preserves eventual rejection and the privacy fix; this is not a blocker or a missing API prerequisite.

In `test/functional/feature_bind_extra.py`:
- [SUGGESTION] test/functional/feature_bind_extra.py:90-105: Declared-partial omission: two upstream Tor test adaptations
  The upstream diff also adds a dedicated onion bind in feature_torcontrol.py::restart_with_mock() and in the Tor-only scenario in p2p_private_broadcast.py. Their containing tests were introduced by bitcoin#34158 (569383356ecd5baa65a87563d776809a63f8f7a3) and bitcoin#29415 (e74d54e04896a86cad4e4b1bd9641afcc3a026c2), respectively, and neither file exists in this Dash base or head. This does not break an existing Dash test or establish a production dependency. However, CONTRIBUTING.md requires Bitcoin backports to explain omitted hunks and tests, and the final commit has only a subject while the PR description describes the superseded implementation. Document these two exclusions and their missing containing tests in the PR description or a backport note, and identify the upstream source revision used. Adding the unrelated prerequisite features is not required to resolve this documentation issue.

In `doc/release-notes-7300.md`:
- [SUGGESTION] doc/release-notes-7300.md:8-16: Release-note adaptation omits wildcard onion binds
  The release note explains rejection when no onion bind is supplied, but omits rejection of an existing wildcard onion bind such as -bind=0.0.0.0:<port>=onion. That configuration satisfies the note's stated requirement for an =onion bind yet now fails startup when -listenonion is enabled. Upstream explicitly mentions shared and wildcard binds, and this head's help text and doc/tor.md require a non-wildcard address. Add the wildcard compatibility change here and state that the dedicated onion bind must use a specific, non-wildcard address.

[thepastaclaw]

🟡 Suggestion: PR description no longer matches the head commit's behavior

The PR description still says explicit -bind configurations retain the implicit onion listener and fail only if its port is unavailable. At this head, the default listener is added only when both bind vectors are empty; an explicit normal bind without an onion bind instead causes an unconditional startup error when -listenonion is enabled. Wildcard onion binds are also rejected now, although the description says that change is deferred. The documented node-3 regression scenario and its network=='onion' assertion were removed by the final commit. Update the implementation rationale, upstream relationship, breaking changes, and test evidence to describe the final policy, distinguishing earlier implementation results from validation of this head. These differences change the operator migration requirements, not just the wording.

_{source: glm-5.3-flash (phase1-reviewer: general)}

[thepastaclaw]

🟡 Suggestion: Declared-partial omission: early Tor bind validation

Upstream bitcoin#36170 rejects explicit bind configurations without an =onion suffix in AppInitParameterInteraction, while retaining the defensive check here. Dash includes only this late check: LoadChainstate and wallet loading occur before it. Consequently, a configuration already known to be invalid can spend substantial time loading chainstate and wallets before reporting the dedicated-bind error. Carry the upstream early check, or explain its intentional exclusion from this partial backport. The current check preserves eventual rejection and the privacy fix; this is not a blocker or a missing API prerequisite.

_{source: gpt-6.1-sol (phase2-reviewer: general, backport-reviewer, dash-core-commit-history)}

[thepastaclaw]

🟡 Suggestion: Declared-partial omission: two upstream Tor test adaptations

The upstream diff also adds a dedicated onion bind in feature_torcontrol.py::restart_with_mock() and in the Tor-only scenario in p2p_private_broadcast.py. Their containing tests were introduced by bitcoin#34158 (5693833) and bitcoin#29415 (e74d54e), respectively, and neither file exists in this Dash base or head. This does not break an existing Dash test or establish a production dependency. However, CONTRIBUTING.md requires Bitcoin backports to explain omitted hunks and tests, and the final commit has only a subject while the PR description describes the superseded implementation. Document these two exclusions and their missing containing tests in the PR description or a backport note, and identify the upstream source revision used. Adding the unrelated prerequisite features is not required to resolve this documentation issue.

_{source: glm-5.3-flash (phase1-reviewer: general); gpt-6.1-sol (phase2-reviewer: general, backport-reviewer, dash-core-commit-history)}

[thepastaclaw]

🟡 Suggestion: Release-note adaptation omits wildcard onion binds

The release note explains rejection when no onion bind is supplied, but omits rejection of an existing wildcard onion bind such as -bind=0.0.0.0:=onion. That configuration satisfies the note's stated requirement for an =onion bind yet now fails startup when -listenonion is enabled. Upstream explicitly mentions shared and wildcard binds, and this head's help text and doc/tor.md require a non-wildcard address. Add the wildcard compatibility change here and state that the dedicated onion bind must use a specific, non-wildcard address.

_{source: gpt-6.1-sol (phase2-reviewer: general, backport-reviewer, dash-core-commit-history)}