News268: Add review club n28165 (p2p v2 encrypted transport)

Author: LarryRuane

_posts/en/newsletters/2023-09-13-newsletter.md

@@ -57,12 +57,90 @@ Club][] meeting, highlighting some of the important questions and
 answers.  Click on a question below to see a summary of the answer from
 the meeting.*
 
-FIXME:LarryRuane
+[Transport abstraction][review club 28165] is a recently-merged PR by Pieter Wuille (sipa)
+that introduces a _transport_ abstraction (interface class). Concrete
+derivations of this class convert a (per-peer) connection's (already
+serialized) send and receive messages to and from wire format. This
+can be thought of as implementing a deeper level of serialization and
+deserialization. These classes do not do the actual sending and receiving.
+
+The PR derives two concrete classes from the `Transport` class,
+`V1Transport` (what we have today) and `V2Transport` (encrypted on the
+wire). This PR is part of the
+[BIP324][topic v2 p2p transport]
+_Version 2 P2P Encrypted Transport Protocol_ [project][v2 p2p tracking pr].
 
 {% include functions/details-list.md
-  q0="FIXME"
-  a0="FIXME"
-  a0link="https://bitcoincore.reviews/28122#l-126FIXME"
+  q0="What is the distinction between [*net*][net] and
+      [*net_processing*][net_processing]?"
+  a0="*Net* is at the bottom of the networking stack and handles
+       low-level communication between peers, while *net_processing*
+       builds on top of the *net* layer and handles the processing
+       and validation of messages from *net* layer."
+  a0link="https://bitcoincore.reviews/28165#l-22"
+
+  q1="More concretely, name examples of classes or functions that
+      we'd associate with *net_processing*, and, in contrast,
+      with *net*?"
+  a1="*net_processing*: `PeerManager`, `ProcessMessage`.
+      *net*: `CNode`, `ReceiveMsgBytes`, `CConnMan`."
+  a1link="https://bitcoincore.reviews/28165#l-25"
+
+  q2="Does BIP324 require changes to the *net* layer, the
+      *net_processing* layer, or both? Does it affect policy
+      or consensus?"
+  a2="These changes are only at the *net* layer; they don't affect consensus."
+  a2link="https://bitcoincore.reviews/28165#l-37"
+
+  q3="What are examples of implementation bugs that could result in
+      this PR being an (accidental) consensus change?"
+  a3="A bug that restricts the maximum message size less to than
+      4MB, resulting in a blockweight soft fork; a bug in block
+      deserialization that causes the node to reject a consensus-valid
+      block, also a soft fork."
+  a3link="https://bitcoincore.reviews/28165#l-45"
+
+  q4="`CNetMsgMaker` and `Transport` both “serialize” messages.
+      What is the difference in what they do?"
+  a4="`CNetMsgMaker` performs the serialization of data structures
+      into bytes; `Transport` receives these bytes, adds
+      (serializes) the header, and actually sends it."
+  a4link="https://bitcoincore.reviews/28165#l-60"
+
+  q5="In the process of turning an application object like a
+      `CTransactionRef` (transaction) into bytes / network packets, what
+      happens? Which data structures does it turn into in the process?"
+  a5="`msgMaker.Make()` serializes the `CTransactionRef` message by
+      calling `SerializeTransaction()`, then `PushMessage()` puts the
+      serialized message into the `vSendMsg` queue, then `SocketSendData()`
+      adds a header/checksum (after the changes from this PR) and asks the
+      transport for the next packet to send, and finally calls `m_sock->Send()`."
+  a5link="https://bitcoincore.reviews/28165#l-83"
+
+  q6="How many bytes are sent over the wire for the `sendtxrcncl`message
+      (taking that message as a simple example)?"
+  a6="36 bytes: 24 for the header (magic 4 bytes, command 12 bytes,
+      message size 4 bytes, checksum 4 bytes), then 12 bytes for the
+      payload (version 4 bytes, salt 8 bytes)."
+  a6link="https://bitcoincore.reviews/28165#l-86"
+
+  q7="After `PushMessage()` returns, have we sent the bytes corresponding
+      to this message to the peer already (yes/no/maybe)? Why?"
+  a7="All are possible. **Yes**: we (*net_processing*) don't have to do
+      anything else to cause the message to be sent.
+      **No**: it's extremely unlikely to have
+      been received by the recipient by the time that function returns.
+      **Maybe**: if all the queues are empty it will have made it to the
+      kernel socket layer, but if some of the queues aren't, then it
+      will still be waiting on those to drain further before getting
+      to the OS."
+  a7link="https://bitcoincore.reviews/28165#l-112"
+
+  q8="Which threads access `CNode::vSendMsg`?"
+  a8="`ThreadMessageHandler` if the message is sent synchronously
+      (\"optimistically\"); `ThreadSocketHandler` if it gets queued
+      and picked up and sent later."
+  a8link="https://bitcoincore.reviews/28165#l-120"
 %}
 
 ## Releases and release candidates
@@ -93,9 +171,13 @@ Proposals (BIPs)][bips repo], [Lightning BOLTs][bolts repo], and
 {% include references.md %}
 {% include linkers/issues.md v=2 issues="26567" %}
 [LND v0.17.0-beta.rc2]: https://github.com/lightningnetwork/lnd/releases/tag/v0.17.0-beta.rc2
+[net]: https://github.com/bitcoin/bitcoin/blob/master/src/net.h
+[net_processing]: https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.h
 [news195 taro]: /en/newsletters/2022/04/13/#transferable-token-scheme
 [osuntokun bips]: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-September/021938.html
 [osuntokun blip post]: https://lists.linuxfoundation.org/pipermail/lightning-dev/2023-September/004089.html
 [osuntokun blip]: https://github.com/lightning/blips/pull/29
+[review club 28165]: https://bitcoincore.reviews/28165
 [sanders post]: https://lists.linuxfoundation.org/pipermail/lightning-dev/2023-September/004088.html
 [sanders ptlc]: https://gist.github.com/instagibbs/1d02d0251640c250ceea1c66665ec163
+[v2 p2p tracking pr]: https://github.com/bitcoin/bitcoin/issues/27634