Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: Split DNS resolving functionality out of net structures #5005

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

nuttycom
Copy link
Contributor

gmaxwell and others added 15 commits February 19, 2021 20:16
Mruset setInventoryKnown was reduced to a remarkably small 1000
 entries as a side effect of sendbuffer size reductions in 2012.

This removes setInventoryKnown filtering from merkleBlock responses
 because false positives there are especially unattractive and
 also because I'm not sure if there aren't race conditions around
 the relay pool that would cause some transactions there to
 be suppressed. (Also, ProcessGetData was accessing
 setInventoryKnown without taking the required lock.)
Previously this logic could erroneously filter a MSG_BLOCK inventory message.
Previously this logic could erroneously filter a MSG_BLOCK inventory message.
This replaces using inv messages to announce new blocks, when a peer requests
(via the new "sendheaders" message) that blocks be announced with headers
instead of inv's.

Since headers-first was introduced, peers send getheaders messages in response
to an inv, which requires generating a block locator that is large compared to
the size of the header being requested, and requires an extra round-trip before
a reorg can be relayed.  Save time by tracking headers that a peer is likely to
know about, and send a headers chain that would connect to a peer's known
headers, unless the chain would be too big, in which case we revert to sending
an inv instead.

Based off of @sipa's commit to announce all blocks in a reorg via inv,
which has been squashed into this commit.

Rebased-by: Pieter Wuille

Zcash: Includes changes from the following subsequent upstream commits
from bitcoin/bitcoin#7112 (which we already backported):

- 012fc91
- 4082e46
- 9af5f9c
We used to have a trickle node, a node which was chosen in each iteration of
the send loop that was privileged and allowed to send out queued up non-time
critical messages. Since the removal of the fixed sleeps in the network code,
this resulted in fast and attackable treatment of such broadcasts.

This pull request changes the 3 remaining trickle use cases by random delays:
* Local address broadcast (while also removing the the wiping of the seen filter)
* Address relay
* Inv relay (for transactions; blocks are always relayed immediately)

The code is based on older commits by Patrick Strateman.
To make it clear where DNS resolves are happening

(cherry picked from commit e9fc71e)
Note: Some seeds aren't actually returning an IP for their name entries, so
they're being added to addrman with a source of [::].

This commit shouldn't change that behavior, for better or worse.

(cherry picked from commit a98cd1f)
…n be shared and reused

(cherry picked from commit 5e10922)
Rather than allowing CNetAddr/CService/CSubNet to launch DNS queries, require
that addresses are already resolved.

This greatly simplifies async resolve logic, and makes it harder to
accidentally leak DNS queries.

(cherry picked from commit 3675699)
CNetAddr/CService/CSubnet can no longer resolve DNS.

(cherry picked from commit d39f5b4)
@nuttycom nuttycom added the C-upstream-port Category: Changes that are ported from the Bitcoin Core codebase. label Feb 23, 2021
@nuttycom nuttycom self-assigned this Feb 23, 2021
@nuttycom nuttycom requested a review from str4d February 23, 2021 20:17
@nuttycom nuttycom marked this pull request as draft September 15, 2022 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-upstream-port Category: Changes that are ported from the Bitcoin Core codebase.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants