--- Log opened Wed Sep 06 00:00:17 2023
01:37 -!- abubakarsadiq [uid602234@id-602234.hampstead.irccloud.com] has joined #bitcoin-core-pr-reviews
02:31 -!- dongcarl [~dongcarl@cpe-66-65-169-19.nyc.res.rr.com] has quit [Ping timeout: 246 seconds]
02:31 -!- dongcarl [~dongcarl@cpe-66-65-169-19.nyc.res.rr.com] has joined #bitcoin-core-pr-reviews
02:36 -!- dongcarl [~dongcarl@cpe-66-65-169-19.nyc.res.rr.com] has quit [Ping timeout: 258 seconds]
02:39 -!- __gotcha [~Thunderbi@85.234.220.64] has joined #bitcoin-core-pr-reviews
02:41 -!- __gotcha [~Thunderbi@85.234.220.64] has quit [Client Quit]
02:42 -!- __gotcha [~Thunderbi@85.234.220.64] has joined #bitcoin-core-pr-reviews
03:55 -!- __gotcha [~Thunderbi@85.234.220.64] has quit [Ping timeout: 255 seconds]
04:25 -!- __gotcha [~Thunderbi@85.234.220.64] has joined #bitcoin-core-pr-reviews
04:36 -!- windsok [~windsok@rarepepe.cash] has quit [Quit: No Ping reply in 180 seconds.]
04:38 -!- windsok [~windsok@rarepepe.cash] has joined #bitcoin-core-pr-reviews
04:39 -!- b10c [~quassel@user/b10c] has quit [Quit: https://quassel-irc.org - Chat comfortably. Anywhere.]
04:43 -!- b10c [~quassel@static.33.106.217.95.clients.your-server.de] has joined #bitcoin-core-pr-reviews
04:43 -!- b10c [~quassel@user/b10c] has changed host
05:16 -!- jonatack3 [~jonatack@user/jonatack] has quit [Read error: Connection reset by peer]
05:17 -!- jonatack3 [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
06:44 -!- vmammal [~vmammal@162.250.26.106] has joined #bitcoin-core-pr-reviews
07:45 -!- vmammal [~vmammal@162.250.26.106] has quit [Quit: Connection closed]
07:58 -!- brunoerg [~brunoerg@50.204.42.240] has joined #bitcoin-core-pr-reviews
08:40 -!- brunoerg [~brunoerg@50.204.42.240] has quit [Remote host closed the connection]
08:46 -!- pablomartin4btc [~pablomart@193.160.247.131] has joined #bitcoin-core-pr-reviews
08:47 -!- brunoerg [~brunoerg@50.204.42.240] has joined #bitcoin-core-pr-reviews
09:03 -!- brunoerg [~brunoerg@50.204.42.240] has quit [Remote host closed the connection]
09:04 -!- brunoerg [~brunoerg@50.204.42.240] has joined #bitcoin-core-pr-reviews
09:06 -!- Talkless [~Talkless@mail.dargis.net] has joined #bitcoin-core-pr-reviews
09:09 -!- brunoerg [~brunoerg@50.204.42.240] has quit [Ping timeout: 248 seconds]
09:21 -!- brunoerg [~brunoerg@50.204.42.240] has joined #bitcoin-core-pr-reviews
09:26 -!- brunoerg [~brunoerg@50.204.42.240] has quit [Ping timeout: 255 seconds]
09:36 -!- dberkelmans [~dberkelma@2001:1c03:530d:8100:e52b:63ba:aaa:150d] has joined #bitcoin-core-pr-reviews
09:39 -!- brunoerg [~brunoerg@50.204.42.240] has joined #bitcoin-core-pr-reviews
09:42 -!- brunoerg [~brunoerg@50.204.42.240] has quit [Read error: Connection reset by peer]
09:42 -!- brunoerg_ [~brunoerg@50.204.42.240] has joined #bitcoin-core-pr-reviews
09:49 -!- vmammal [~vmammal@162.250.26.106] has joined #bitcoin-core-pr-reviews
09:57 -!- effexzi [uid474242@id-474242.ilkley.irccloud.com] has joined #bitcoin-core-pr-reviews
10:00 < glozow> hi!
10:00 < glozow> #startmeeting
10:00 < abubakarsadiq> hello
10:00 < dberkelmans> Hi
10:00 -!- mayrf [~mayrf@80-108-88-245.cable.dynamic.surfer.at] has joined #bitcoin-core-pr-reviews
10:01 < effexzi> Hi every1
10:01 < glozow> hello, welcome to PR review club!
10:01 < lightlike> Hi
10:01 -!- BrandonOdiwuor [~BrandonOd@105.162.30.125] has joined #bitcoin-core-pr-reviews
10:01 < mayrf> Hi
10:01 < sipa> hi
10:01 < glozow> This week's PR is #28165, notes are in the usual place: https://bitcoincore.reviews/28165
10:01 < BrandonOdiwuor> Hello
10:01 < glozow> Did anybody get a chance to review the PR or look at the notes? how about a y/n
10:02 < mayrf> n
10:02 < abubakarsadiq> read the notes recently, but  did not review the PR
10:02 < dberkelmans> No read half
10:03 < BrandonOdiwuor> y
10:03 < glozow> If you didn't have a chance to review the PR, today's questions are more general and you should be able to figure them out while we're going through the questions. So don't worry :) Tomorrow's questions will be PR-specific
10:03 < lightlike> y
10:03 < sipa> y
10:04 < michaelfolkson> hi
10:04 < michaelfolkson> y
10:04 < glozow> great, let's get started with the questions. And feel free to ask your own questions at any time!
10:04 < glozow> What is the distinction between “net” and “net processing”? What data structures and tasks might we associate with each of them?
10:05 < BrandonOdiwuor> Net sits 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
10:06 < glozow> BrandonOdiwuor: great answer!
10:07 < glozow> To make this more concrete, can anybody name an example of a class or function that we'd associate with net processing and not net?
10:08 < glozow> And can somebody name a class or function that we'd associate with net, and not net processing?
10:08 < abubakarsadiq> peermanager is associated with net_processing
10:08 < glozow> abubakarsadiq: yup I agree!
10:09 -!- BrandonOdiwuor29 [~BrandonOd@105.162.30.125] has joined #bitcoin-core-pr-reviews
10:10 < glozow> Anyone wanna name something in net?
10:11 < abubakarsadiq> classes like CNode
10:11 < BrandonOdiwuor29> ReceiveMsgBytes and MarkReceivedMsgForProcessing by net
10:11 < glozow> Great answers!
10:11 < michaelfolkson> CNetCleanup
10:11 < instagibbs> CConnman\
10:12 -!- BrandonOdiwuor [~BrandonOd@105.162.30.125] has quit [Ping timeout: 255 seconds]
10:13 < BrandonOdiwuor29> ProcessMessages and PollMessages in net_processing
10:13 -!- dberkelmans [~dberkelma@2001:1c03:530d:8100:e52b:63ba:aaa:150d] has quit [Quit: Client closed]
10:13 -!- dberkelmans [~dberkelma@2001:1c03:530d:8100:e52b:63ba:aaa:150d] has joined #bitcoin-core-pr-reviews
10:13 < glozow> I think it's also worth pointing out here that the line is somewhat blurry. `CConnman::AttemptToEvictConnection` (which is in connman/net) uses "application" logic like whether a peer provided us a valid transaction recently to decide whether we should consider evicting a peer.
10:15 < glozow> Next question: Does BIP324 require changes to the “net” layer, the “net processing” layer, or both? Does it affect policy or consensus?
10:17 < BrandonOdiwuor29> I think it mostly requires changes in the net layer which deals with communication between peers
10:18 < michaelfolkson> net_processing not touched in either PR
10:18 < michaelfolkson> (.cpp)
10:18 < glozow> BrandonOdiwuor29: I agree!
10:19 < abubakarsadiq> I dont think this is a consensus change
10:19 < instagibbs> hopefully not :)
10:19 < glozow> abubakarsadiq: correct. at least we hope so
10:20 < glozow> ooh, fun question: what kind of implementation bug could result in the PR being an (accidental) consensus change?
10:20 < abubakarsadiq> consensus change will require all nodes to upgrade to the new version
10:20 < glozow> ah, maybe not this PR. I mean "bug in the implementation of BIP324"
10:22 < michaelfolkson> It has got to be creative surely, this doesn't impact validation at all
10:22 < instagibbs> bug which restricts max message size less than 4MB, resulting in a blockweight softfork
10:22 < instagibbs> (assuming your only connections are v2)
10:22 < sipa> instagibbs: bing bing bing
10:22 < instagibbs> or if the bug infected v1
10:22 < glozow> instagibbs: bingo. You could imagine a bug in deserialization of a block where you'd reject something consensus-valid
10:22 < instagibbs> I was checking that specific logic today :)
10:23 < glozow> Is {CNetMessage, CMessageHeader, CSerializedNetMsg, BytesToSend} used in sending, receiving, or both?
10:23 < BrandonOdiwuor29> CNetMessage is used in Receiving while CSerializedNetMsg is mostly used in sending. BytesToSend is also used in sending
10:23 < BrandonOdiwuor29> CMessageHeader is used in both sending and receiving
10:24 < sipa> CMessageHeader is really only used directly for V1, though some constants are reused in V2 too
10:24 < glozow> BrandonOdiwuor29: nice prep :D
10:25 < glozow> CNetMsgMaker and Transport both “serialize” messages. What is the difference in what they do?
10:26 < michaelfolkson> For that bug it would need to be a ~4MB transaction to trigger it?
10:26 < sipa> no
10:26 < instagibbs> michaelfolkson just a block that's above the size of the buggy limit
10:26 < sipa> if 4M block messages can't get through, that'd be a problem
10:27 < sipa> though compact blocks can partially mitigate it
10:27 < instagibbs> OOB *rdinals
10:27 < _aj_> would be fun for IBD
10:28 -!- brunoerg_ [~brunoerg@50.204.42.240] has quit [Remote host closed the connection]
10:29 -!- BrinkingChancell [~BrinkingC@cpe-24-168-85-43.si.res.rr.com] has joined #bitcoin-core-pr-reviews
10:30 < glozow> Hint: `CNetMsgMaker` creates a `CSerializedNetMsg` (declared here https://github.com/bitcoin/bitcoin/blob/cf421820f50abcbd4f2709f200d3a78fb69fc698/src/net.h#L107)
10:30 < glozow> What does `Transport` do?
10:32 < _aj_> being a bit pedantic: Transport encodes something that's already serialized? (it takes a Span<uint8_t> which as already been serialized)
10:32 < lightlike> CNetMsgMaker: Performs the serialization of data structures into bytes, Transport  adds the header and actually sends it
10:33 < sipa> i've tried to get the "serialize" terminology out of most places related to the transport-network part
10:33 < _aj_> i guess serializednetmsg has a message and payload which isn't completely serialized
10:33 < glozow> lightlike: thank you!
10:33 < glozow> Yeah I added this question when I saw https://github.com/bitcoin/bitcoin/pull/28165/files#r1301596054
10:33 < sipa> i think at some point in the codebase the serialization of data structure to bytes, and the addition of message headers, were done at the same time, so it was all call "serialization"
10:33 < sipa> *called
10:34 < sipa> but using "serialization" to refer to the "turn message type/payloads into network packets" is pretty confused
10:35 < _aj_> "class CNode: { public: /** Transport serializer/deserializer"
10:35 < sipa> sigh!
10:35 < _aj_> :D
10:36 < instagibbs> taking this as approach NACK
10:36 < glozow> The next question is about this process (and mostly an exercise to the reader): in the process of turning an application object like a `CTransactionRef` into bytes / network packets, what happens? What data structures does it turn into in the process?
10:37 < glozow> The exercise is to grep/ctags your way from https://github.com/bitcoin/bitcoin/blob/ab42b2ebdbf61225e636e4c00068fd29b2790d41/src/net_processing.cpp#L2334-L2335 to https://github.com/bitcoin/bitcoin/blob/ab42b2ebdbf61225e636e4c00068fd29b2790d41/src/net.cpp#L949
10:39 < glozow> (feel free to post your answer at any time but I'll move on to the next question)
10:39 -!- BrandonOdiwuor [~BrandonOd@105.162.30.125] has joined #bitcoin-core-pr-reviews
10:40 < glozow> Next question is also an exercise. The RPC getpeerinfo returns a map of bytessent_per_msg and bytesrecv_per_msg. Add a print(self.nodes[0].getpeerinfo()[0]['bytessent_per_msg']) to one of the subtests in test/functional/p2p_sendtxrcncl.py after peers send sendtxrcncl to each other. What is the number of bytes sent for "sendtxrcncl" ?
10:40 < glozow> e.g. you could add the line here: https://github.com/bitcoin/bitcoin/blob/ab42b2ebdbf61225e636e4c00068fd29b2790d41/test/functional/p2p_sendtxrcncl.py#L75
10:41 -!- brunoerg [~brunoerg@50.204.42.240] has joined #bitcoin-core-pr-reviews
10:41 < instagibbs> 36?
10:41 < lightlike> msgMaker.Make() serializes the CTransactionRef message, calling SerializeTransaction(), then PushMessage puts the serialized msg into the vSendMsg queue, then SocketSendData adds a header/checksum (after the changes from this PR) and asks transport for the next package to send, and finally calls m_sock->Send
10:41 < glozow> instagibbs: I got the same thing!
10:42 < glozow> But BIP330 specifies that sendtxrcncl just has a 4B version and 8B salt! Where does the other 24B come from?
10:42 < sipa> 36 bytes sounds correct
10:42 < glozow> lightlike: 👑 you dropped this
10:42 -!- BrandonOdiwuor29 [~BrandonOd@105.162.30.125] has quit [Ping timeout: 255 seconds]
10:43 < sipa> hint: what is CMessageHeader::HEADER_SIZE?
10:43 < glozow> (hint: `CMessageHeader` is defined here: https://github.com/bitcoin/bitcoin/blob/d2ccca253f4294b8f480e1d192913f4985a1af08/src/protocol.h#L26)
10:44 < glozow> sipa: nice hint
10:44 < BrandonOdiwuor> CMessageHeader::HEADER_SIZE is 24 bytes
10:44 < sipa> BrandonOdiwuor: correct
10:45 < glozow> correct! And what makes up those 24 bytes?
10:45 < BrandonOdiwuor> MESSAGE_START_SIZE(4) + COMMAND_SIZE(12) + MESSAGE_SIZE_SIZE(4) + CHECKSUM_SIZE(4)
10:46 < glozow> BrandonOdiwuor: yep!
10:46 < sipa> so what are the 36 bytes of a sendtxrcncl on the wire?
10:48 -!- mayrf_ [~mayrf@80-108-88-245.cable.dynamic.surfer.at] has joined #bitcoin-core-pr-reviews
10:49 < glozow> I guess `print(msg)` in `P2PConnection::_on_data`?
10:50 < sipa> way too practical
10:50 < instagibbs> making stuff up instead of checking: network magic + "sendtxrcncl"(padded to 12 bytes?) + 4 byte size of payload + 4 byte checksum + 4 byte version + 8 byte salt
10:51 < sipa> ding ding ding
10:51 -!- mayrf [~mayrf@80-108-88-245.cable.dynamic.surfer.at] has quit [Ping timeout: 255 seconds]
10:51 < glozow> I was assuming you wanted the actual bytes 😂
10:51 < sipa> oh.
10:51 < sipa> Sure!
10:52 < instagibbs> header followed by the payload of the command itself, in order
10:52 < glozow> anyway next question
10:52 < glozow> After `PushMessage` returns, have we sent the bytes corresponding to this message to the peer already (yes/no/maybe)? Why?
10:53 < sipa> nice question
10:54 < instagibbs> maybe, if the queue was empty
10:55 < _aj_> yes: we(net_processing) don't have to do anything else to get it to go; 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 arent, then it will still be waiting on those to drain further before getting to the OS
10:55 < sipa> _aj_: nice, all 3
10:55 < _aj_> (i also have "can you repeat the question" playing in my head)
10:56 < sipa> one nit: even if all the queues were empty, but the message size exceeds the OS's send buffer size, only the part that fits will make it to the socket layer
10:56 -!- pablomartin4btc [~pablomart@193.160.247.131] has quit [Remote host closed the connection]
10:56 < glozow> nice! I was going for "maybe" for the scenarios described
10:56 -!- pablomartin4btc [~pablomart@193.160.247.131] has joined #bitcoin-core-pr-reviews
10:57 < glozow> last question: Which threads access `CNode::vSendMsg`?
11:00 < lightlike> ThreadMessageHandler if it gets sent "optimistically", ThreadSocketHandler if it gets queued and picked up later
11:00 < glozow> lightlike: yes!
11:01 < glozow> This will come in handy tomorrow when we go through questions about the PR 🧠
11:01 < sipa> and i think that's all; i don't think RPC or GUI or so even access those
11:01 -!- pablomartin4btc [~pablomart@193.160.247.131] has quit [Ping timeout: 245 seconds]
11:01 < glozow> Thanks everyone for coming today, we managed to get through all the questions (yay!)
11:02 < _aj_> sipa: hmm, bump net.core.wmem_max and net.core.wmem_default up to 5M or something then?
11:02 < glozow> Remember we're back tomorrow at the same time (17UTC), and we'll dig a bit deeper into the PR
11:02 < glozow> #endmeeting
11:03 < instagibbs> thanks glozow!
11:03 < lightlike> thanks glozow!
11:03 < michaelfolkson> Thanks!
11:03 < sipa> glozow: thanks for picking up my PR here!
11:03 < dberkelmans> Thanks
11:04 < effexzi> Thanks every1 
11:04 -!- dberkelmans [~dberkelma@2001:1c03:530d:8100:e52b:63ba:aaa:150d] has quit [Quit: Client closed]
11:05 -!- andrew_mo_ [andrew_mo_@gateway/vpn/protonvpn/andrewmo/x-47904524] has joined #bitcoin-core-pr-reviews
11:05 -!- andrew_mo_ [andrew_mo_@gateway/vpn/protonvpn/andrewmo/x-47904524] has quit [Client Quit]
11:06 -!- mayrf_ [~mayrf@80-108-88-245.cable.dynamic.surfer.at] has quit [Quit: Leaving]
11:13 -!- BrandonOdiwuor [~BrandonOd@105.162.30.125] has quit [Ping timeout: 258 seconds]
11:15 -!- brunoerg [~brunoerg@50.204.42.240] has quit []
11:31 -!- emzy [~quassel@user/emzy] has quit [Server closed connection]
11:31 -!- emzy [~quassel@user/emzy] has joined #bitcoin-core-pr-reviews
11:43 -!- vmammal [~vmammal@162.250.26.106] has quit [Quit: Connection closed]
12:07 -!- Talkless [~Talkless@mail.dargis.net] has quit [Quit: Konversation terminated!]
12:23 -!- pablomartin [~pablomart@193.160.247.140] has joined #bitcoin-core-pr-reviews
12:24 -!- pablomartin is now known as pablomartin4btc
13:16 -!- abubakarsadiq [uid602234@id-602234.hampstead.irccloud.com] has quit [Quit: Connection closed for inactivity]
13:16 -!- pablomartin4btc [~pablomart@193.160.247.140] has quit [Remote host closed the connection]
13:18 -!- pablomartin4btc [~pablomart@193.160.247.140] has joined #bitcoin-core-pr-reviews
13:26 -!- abubakarsadiq [uid602234@id-602234.hampstead.irccloud.com] has joined #bitcoin-core-pr-reviews
13:35 -!- pablomartin4btc [~pablomart@193.160.247.140] has quit [Ping timeout: 258 seconds]
13:44 -!- brunoerg [~brunoerg@50.204.42.240] has joined #bitcoin-core-pr-reviews
13:55 -!- pablomartin [~pablomart@193.160.247.129] has joined #bitcoin-core-pr-reviews
14:01 -!- brunoerg [~brunoerg@50.204.42.240] has quit [Remote host closed the connection]
14:05 -!- grettke [~grettke@065-026-198-174.biz.spectrum.com] has joined #bitcoin-core-pr-reviews
14:07 -!- jonatack [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
14:08 -!- jonatack3 [~jonatack@user/jonatack] has quit [Read error: Connection reset by peer]
14:13 -!- emzy [~quassel@user/emzy] has quit [Quit: http://quassel-irc.org - Chat comfortably. Anywhere.]
14:25 -!- brunoerg [~brunoerg@50.204.42.240] has joined #bitcoin-core-pr-reviews
14:30 -!- brunoerg [~brunoerg@50.204.42.240] has quit [Remote host closed the connection]
14:31 -!- pablomartin4btc [~pablomart@193.160.247.139] has joined #bitcoin-core-pr-reviews
14:32 -!- pablomartin [~pablomart@193.160.247.129] has quit [Ping timeout: 255 seconds]
15:17 -!- effexzi [uid474242@id-474242.ilkley.irccloud.com] has quit [Quit: Connection closed for inactivity]
15:28 -!- chiplover [~chiplover@static-198-54-135-121.cust.tzulo.com] has joined #bitcoin-core-pr-reviews
15:29 -!- chiplover [~chiplover@static-198-54-135-121.cust.tzulo.com] has quit [Client Quit]
15:31 -!- guest9 [~guest9@static-198-54-135-121.cust.tzulo.com] has joined #bitcoin-core-pr-reviews
15:31 -!- guest9 [~guest9@static-198-54-135-121.cust.tzulo.com] has quit [Client Quit]
15:31 -!- guest9 [~guest9@static-198-54-135-121.cust.tzulo.com] has joined #bitcoin-core-pr-reviews
15:31 -!- guest9 [~guest9@static-198-54-135-121.cust.tzulo.com] has quit [Client Quit]
15:32 -!- hahs [~hahs@static-198-54-135-121.cust.tzulo.com] has joined #bitcoin-core-pr-reviews
15:32 -!- hahs [~hahs@static-198-54-135-121.cust.tzulo.com] has quit [Client Quit]
15:38 -!- __gotcha [~Thunderbi@85.234.220.64] has quit [Read error: Connection reset by peer]
15:38 -!- __gotcha1 [~Thunderbi@85.234.220.64] has joined #bitcoin-core-pr-reviews
15:40 -!- pablomartin4btc [~pablomart@193.160.247.139] has quit [Ping timeout: 255 seconds]
15:41 -!- __gotcha1 is now known as __gotcha
16:28 -!- abubakarsadiq [uid602234@id-602234.hampstead.irccloud.com] has quit [Ping timeout: 240 seconds]
16:28 -!- abubakarsadiq [uid602234@id-602234.hampstead.irccloud.com] has joined #bitcoin-core-pr-reviews
17:26 -!- abubakarsadiq [uid602234@id-602234.hampstead.irccloud.com] has quit [Quit: Connection closed for inactivity]
18:51 -!- pablomartin [~pablomart@193.160.247.138] has joined #bitcoin-core-pr-reviews
18:57 -!- brunoerg [~brunoerg@2601:c2:1201:8410:e00a:b282:92c7:b674] has joined #bitcoin-core-pr-reviews
19:21 -!- brunoerg [~brunoerg@2601:c2:1201:8410:e00a:b282:92c7:b674] has quit []
19:47 -!- emzy [~quassel@user/emzy] has joined #bitcoin-core-pr-reviews
21:14 -!- duderonomy [~duderonom@c-107-3-165-79.hsd1.ca.comcast.net] has quit [Read error: Connection reset by peer]
21:15 -!- duderonomy [~duderonom@c-107-3-165-79.hsd1.ca.comcast.net] has joined #bitcoin-core-pr-reviews
21:48 -!- jon_atack [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
21:49 -!- jonatack [~jonatack@user/jonatack] has quit [Read error: Connection reset by peer]
22:04 -!- grettke [~grettke@065-026-198-174.biz.spectrum.com] has quit [Quit: grettke]
--- Log closed Thu Sep 07 00:00:17 2023