highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-11 14:38:29 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#20018: p2p: ProcessAddrFetch(-seednode) is unnecessary if -connect is specified

Browse Source 
2555a3950f p2p: ProcessAddrFetch(-seednode) is unnecessary if -connect is specified (Dhruv Mehta)

Pull request description:

  If the user runs: `bitcoind -connect=X -seednode=Y`, I _think_ it is safe to ignore `-seednode`. A more populated `addrman` (via `getaddr` calls to peers in `-seednode`) is not useful in this configuration: `addrman` entries are used to initiate new outbound connections when slots are open, or to open feeler connections and keep `addrman` from getting stale. This is all done in a part of `ThreadOpenConnections` (below [this line](https://github.com/bitcoin/bitcoin/blob/master/src/net.cpp#L1803)) which is never executed when `-connect` is supplied. With `-connect`, `ThreadOpenConnections` will run [this loop](https://github.com/bitcoin/bitcoin/blob/master/src/net.cpp#L1785) and exit thread execution when interrupted.

  Reviewers may also find it relevant that when `-connect` is used, we [soft disable](https://github.com/bitcoin/bitcoin/blob/master/src/init.cpp#L800) `-dnsseed` in init.cpp perhaps for the same reason i.e. seeding is not useful with `-connect`.

  Running `ProcessAddrFetch` does not seem to have downside except developer confusion AFAICT. I was confused by this and felt it might affect other new bitcoiners too. If there is strong preference to not remove the line, I'd also be happy to just leave a comment there mentioning `ADDR_FETCH`/`-seednode` is irrelevant when used with `-connect`.

  If this change is accepted, the node will still make `getaddr` calls to peers in `-connect` and expand `addrman`. However, disabling those `getaddr` calls would leak information about the node's configuration.

ACKs for top commit:
  mzumsande:
    Code Review ACK 2555a3950f
  achow101:
    ACK 2555a3950f
  vasild:
    ACK 2555a3950f

Tree-SHA512: 9187a0cff58db8edeca7e15379b1c121e7ebe8c38fb82f69e3dae8846ee94c92a329d79025e0f023c7579b2d86e7dbf756e4e30e90a72236bfcd2c00714180b3
This commit is contained in:
Andrew Chow
2023-02-17 14:15:20 -05:00
parent 35fbc97208 2555a3950f
commit f722a9bd13
3 changed files with 44 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/net.cpp
View File

@@ -1471,6 +1471,8 @@ void CConnman::ThreadDNSAddressSeed()
}
LogPrintf("Loading addresses from DNS seed %s\n", seed);
// If -proxy is in use, we make an ADDR_FETCH connection to the DNS resolved peer address
// for the base dns seed domain in chainparams
if (HaveNameProxy()) {
AddAddrFetch(seed);
} else {
@@ -1492,8 +1494,9 @@ void CConnman::ThreadDNSAddressSeed()
}
addrman.Add(vAdd, resolveSource);
} else {
// We now avoid directly using results from DNS Seeds which do not support service bit filtering,
// instead using them as a addrfetch to get nodes with our desired service bits.
// If the seed does not support a subdomain with our desired service bits,
// we make an ADDR_FETCH connection to the DNS resolved peer address for the
// base dns seed domain in chainparams
AddAddrFetch(seed);
}
}
@@ -1602,7 +1605,6 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
{
for (int64_t nLoop = 0;; nLoop++)
{
ProcessAddrFetch();
for (const std::string& strAddr : connect)
{
CAddress addr(CService(), NODE_NONE);