The PR branch HEAD was 28df701eb1 at the time of this review club meeting.
On block-relay-only connections (introduced in PR #15759),
transaction and address relay are disabled. Currently, Bitcoin Core nodes
per default make two outbound block-relay-only connections in addition to 8 regular
Block-relay-only connections have been subject of multiple review club sessions,
among them #15759 on the PR introducing them.
Currently, block-relay-only connections are established
When making an outbound block-relay-only
connection, a node sets the boolean flag fRelay in the version message to false.
fRelay (introduced in the context of BIP37)
does not imply that transactions cannot be sent for the entire duration of the
connection - in its original use case with BIP37, relay of transactions can
be activated later on.
fRelay=false is also used in -blocksonly mode, a low-bandwidth
option in which a node does not want to receive transactions from any peer,
but does participate in address relay.
Therefore, nodes currently don’t have a notion which of their incoming peers
see the connection as block-relay-only and don’t have any logic attached to it.
accompanied by the new BIP proposal BIP338,
introduces the new p2p message disabletx for block-relay-only connections, which
makes it explicit that no messages related to transaction relay should ever be
exchanged over the duration of the connection.
<stickies-v> seems very sensible, but also that I think it maybe would've been nice to have fRelay be more expressive as an integer so we could capture more states, including the one expressed by disabletx
<stickies-v> I think the benefit is mostly altruistic, in that your peer can optimize their local state to not have to prepare for sending you transactions in the future (e.g. keeping sketches with erlay)
<sipa> glozow: Fair, with whitelisted peers that forcibly give you transactions you could still have a mempool (though that's kind of weird)... the point of blockonly mode is just reducing bandwidth because you don't care about having a mempool.
<sipa> I think the rationale is mostly knowing that a connection will never need these datastructures, which could help with e.g. deciding how many connections can be supported given local resource constraints.
<lightlike> I wonder if having a separate protocol message for each boolean of information you want to exchange during feature negotiation is ideal design in the long run - it could lead to a zoo of messages over time that might be hard to keep track with
<sipa> lightlike: If we had perfect foresight about what we'd ever want the protocol to be, I'm sure something cleaner/more efficient could be chosen, but the "new protocol version number to add support for X; new message to negotiate the feature" has proven pretty flexible so far.
<lightlike> When a node makes an outgoing block-relay-only connection, it will send a disabletx message after this PR. Will there be other changes in behavior for the sender beyond this (if yes, which ones)?
<jnewbery> lightlike: I share that concern. I think it's tractable now, but it gets more complex as the number of features increases and there are features that can only be negotiated if other features have already been enabled (eg erlay requires wtxidrelay).
<stickies-v> lightlike I don't think it's implemented in this PR yet, but my understanding is that in follow up PRs the sender of DISABLETX would also have to stop sending other messages like ADDR in order not to be disconnected?
<michaelfolkson> So a possible downside to this DISABLETX approach is it formalizes the relationship as a block relay only and to get out of it you need to redo the handshake. Current behavior is a peer can later request to receive transactions again using filterload without redoing the handshake?
<lightlike> Earlier discussions in the PR revolved around the interaction of disabletx and address relay. Why is it, after PR #21528, no longer necessary to change any address-relay related code in this PR?
<lightlike> It basically changed the way address relay is setup: if you make an outgoing block-relay-connection, you know that you don't want addr related messages, and just don't send any such messages.
<glozow> for every read/write to an atomic_bool, the lock will be taken right before and released right before that operation is over, so you wouldn't be taking multiple locks at a time in different orders?
<sipa> larryruane: Pet peeve of mine: the compiler is irrelevant. The question you should ask if is if it's permitted by the *language* (and so far, I think the answer is no, but it could be expanded in future language versions of course).
<sipa> In practice, x86 has a fairly strong consistency model, so it's not unreasonable that the resulting compiled code actually works there (but it certainly might not). On other hardware (non-Mac ARM, in particular) non-atomic operations are pretty likely to kill you.