The PR branch HEAD was 9db82f1 at the time of this review club meeting.
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
doesn’t know that we’re treating it as block-relay-only.
When connecting with a peer, we allocate a
object in the net_processing application layer (and a
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
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
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
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
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
object separate from the
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?
This PR adds
to safely (using a mutex) return a pointer to a peer’s TxRelay
structure. If the a call to GetTxRelay() (for example,
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
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)
<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?
<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
<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)
<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)?
<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: 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.
<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!