The PR branch HEAD was 9db82f1 at the time of this review club meeting.
Notes
Each of our node’s peers is either inbound (our peer initiated the connection)
or outbound (we initiated the connection). Once the connection is
established, the relationship is largely symmetric (the protocol works the same
in both directions), although we treat outbound peers with some preference.
When we initiate an outbound connection, we may decide that we won’t
relay (forward) transactions or addresses to this peer.
This peer is designated a block-relay-only peer.
This kind of peer connection was covered in earlier review club meetings
(PRs 15759,
19858,
20726).
The peer
doesn’t know that we’re treating it as block-relay-only.
When connecting with a peer, we allocate a
Peer
object in the net_processing application layer (and a
Cnode
in the connection layer).
If we plan to announce and send transactions and addresses to
this peer (for example, if it’s not a block-relay-only peer),
we create an additional
TxRelay
struct object (owned by the Peer object) to track
the state needed for relaying transactions to this peer.
The TxRelay object includes:
the list of transactions we plan to announce to this peer
the rolling bloom filter of
transactions we expect the peer to know about (because either we sent the
transaction to the peer, or it has sent it to us)
if we’ve enabled BIP37 (see below), the bloom filter sent by the peer
If we advertise support for
BIP37
bloom filters to our peer
during version handshake, the peer can send us a bloom filter
after which we won’t send transactions that aren’t present in
the filter (to reduce bandwidth).
A node which initiates a connection may indicate that it doesn’t want to
receive transactions. It does this using the
fRelay field
in the
version
message.
If we’ve advertised support for bloom filters to this peer,
it may, at any time later, send us a filterload message,
which has the side-effect of enabling transaction relay from
us to this peer, even if it had sent relay = false in the version
message.
Why are some peers block-relay-only? Briefly, what’s the purpose of having
some peers being block-relay-only?
This PR reduces resource usage. Which resource is reduced and by how much?
Why is the
TxRelay
object separate from the
Peer
object?
Why is NODE_BLOOMnot the default behavior
(why does a node choose to advertise it, or not, using a service bit)?
Why do most nodes apparently not enable this service?
Why would a peer send us fRelay = false in the version message?
Why might a peer send us fRelay = false if it wasn’t planning to send
us a filterload?
This PR is divided into four commits. What are some principles we
should follow when deciding on how to distribute the changes
among the commits?
Bonus Questions
This PR adds
GetTxRelay()
to safely (using a mutex) return a pointer to a peer’s TxRelay
structure. If the a call to GetTxRelay() (for example,
this one)
returns a non-null pointer, then the pointer is used without holding
the mutex. What prevents the TxRelay
structure from being deallocated just after the pointer to it is returned
by GetTxRelay(), making the pointer invalid?
Some class variables are annotated mutable. What does this mean?
Hint: This is fairly common with mutex variables.
The pointer variable tx_relay is
initialized
as const, but it’s used to mutate the TxRelay structure it’s pointing to.
How is that possible?
<larryruane> Mine personally is to checkout the branch locally, start up vscode (or just "code" on linux), which has a nice git integration, and look at the commits separately... then I also sometimes run the debugger on both one of the python tests and the node (bitcoind)
<BlueMoon> It seems to me that if our peer initiates the connection it is an incoming peer, if we initiate the connection the peer is an outgoing peer.
<larryruane> when we initiate the connection (an outgoing connection), there are multiple types, two main ones that are most important for today's PR, can anyone say what they are and what's the difference between them?
<OliverOff> one kind transmits both block data and mempool data (unconfirmed txs), the other kind transmits only block data and it's called block-relay-only connections
<svav> For network security. By not relaying transactions or addresses, these connections are harder to detect by a third party, thus helping obfuscate the network topology.
<OliverOff> to make it more difficult to identify the networks topology, which could lead to privacy loss (identifying the IP that originated a transaction) and facilitate GBP attacks
<theStack> an interesting fact is that there is also a "blocksonly" mode for reducing traffic by turning off transaction relay completely (option -blocksonly); this can be confusing due to the similar naming to "block-relay-only"... at least it was for me :)
<larryruane> theStack: yes excellent, i was just going to mention that next! setting up a block-relay-only peer affects only how we interact with that peer, whereas `blocksonly=` (configuration option) affects our behavior with all our peers
<larryruane> you can still initiate a transaction and relay it out to the network, but it's bad for privacy because everyone will know that you initiated it
<theStack> (for those interested, this is a thread in the bitcointalk board where gmaxwell gives a bit background and bandwidth savings results for the back then newly introduced -blocksonly mode: https://bitcointalk.org/index.php?topic=1377345.0)
<svav> A Bloom filter is a space-efficient probabilistic data structure, conceived by Burton Howard Bloom in 1970, that is used to test whether an element is a member of a set.
<larryruane> so it's a way of compactly storing a set of values, but you can only test if a value is present (you can't iterate a bloom filter like a map or a vector), and you can get false positives (the filter can say an item is there when it actually isn't)
<larryruane> OliverOff: good! sipa: yes, that's question 6, why do most nodes not support this bloom filtering service (that is they don't set the `NODE_BLOOM` service bit in the version message they send to their peers)?
<larryruane> bloom filtering (offering that service) used to be enabled by default (you always had the option of disabling it), but it was changed to default-disabled here: https://github.com/bitcoin/bitcoin/pull/16152
<sipa> We use bloom filters for a lot of things in the p2p code and validation. BIP37 is the only place where it's specifically part of the protocol, in order to perform server-side filtering.
<sipa> But "disabling Bloom filters" is maybe a bit confusing, as it's just about disabling server-side filtering, not all other places where Bloom filters are used.
<larryruane> sipa: thanks, isn't there a newer data structure that's similar to but an improvement on bloom filters? I can't recall its name (if there is one)
<sipa> BIP158 (client-side filtering) uses yet another datastructure (golomb filters), which are smaller than Bloom filters and cuckoo filters, but don't support efficient updating after creation.
<larryruane> Thanks, I've learned a lot. Okay so finally we're ready to consider question 4, this PR reduces resource usage, which resource and roughly how much?
<theStack> it seems that the "pure" bloom filter CBloomFilter is only used for BIP37, whereas the rolling bloom filter CRollingBloomFilter is used at more places (if i interpreted my "git grep" results correctly :))
<sipa> OliverOff: Yes and no - they're very different designs. BIP157/BIP158 can be used for some things BIP37 was, but not all, and the other way around.
<OliverOff> and what was the DoS problem with BIP37 that warranted it to be disabled by default? was it sending an avalanche of `filterload` and `filterclear` messages?
<sipa> You can't explicitly delete anything from it, but they only guarantee that inserted elements remain in it for a number of insertions that follow.
<sipa> OliverOff: You can send a filter that matches nothing, and then ask for all blocks in the chain. Now the attacker gets only tiny messages (empty filtered blocks), while the victim is reading through the entire blockchain and processing every tx in it.
<larryruane> To maybe help answer question 4, let's look at question 5, what is the `TxRelay` object (struct), and why is it separate from the `Peer` object (also a struct)?
<svav> if we have not offered NODE_BLOOM to the peer and it has set fRelay to 0, then we know that it will never request transaction announcements, and that we can save resources by not initializing the TxRelay data structure for that incoming node
<larryruane> svav: yes, `TxRelay` contains only state that's needed for relaying transactions (outward) to a particular peer ... so if we never need to send transactions to this peer, then there's no reason to allocate one of these for this peer
<svav> yashraj the reason for this PR is that there are some circumstances that we know a TXRelay will never be needed for a particular incoming node, and therefore we save memory resources in our node by not creating it - I think!
<larryruane> yashraj: yes, the `TxRelay` is per-peer (whether inbound or outbound), because it contains stuff like: which transactions does this peer already know about?