--- Log opened Wed Dec 08 00:00:55 2021
00:11 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
00:15 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 252 seconds]
00:27 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Ping timeout: 240 seconds]
00:41 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has joined #bitcoin-core-pr-reviews
00:42 -!- _andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has joined #bitcoin-core-pr-reviews
00:43 -!- andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has quit [Remote host closed the connection]
00:47 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Ping timeout: 256 seconds]
00:59 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has joined #bitcoin-core-pr-reviews
01:00 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
01:03 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Ping timeout: 240 seconds]
01:05 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 265 seconds]
01:10 -!- b10c [uid500648@id-500648.ilkley.irccloud.com] has joined #bitcoin-core-pr-reviews
01:15 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has joined #bitcoin-core-pr-reviews
01:39 -!- _andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has quit [Remote host closed the connection]
01:40 -!- _andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has joined #bitcoin-core-pr-reviews
01:50 -!- brunoerg [~brunoerg@187.183.47.88] has joined #bitcoin-core-pr-reviews
01:55 -!- brunoerg [~brunoerg@187.183.47.88] has quit [Ping timeout: 265 seconds]
02:14 -!- brunoerg [~brunoerg@187.183.47.88] has joined #bitcoin-core-pr-reviews
02:18 -!- grettke [~grettke@cpe-65-29-228-30.wi.res.rr.com] has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
02:20 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Ping timeout: 256 seconds]
02:26 -!- brunoerg [~brunoerg@187.183.47.88] has quit [Ping timeout: 256 seconds]
02:34 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has joined #bitcoin-core-pr-reviews
02:39 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Ping timeout: 256 seconds]
02:53 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has joined #bitcoin-core-pr-reviews
02:55 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
02:57 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Ping timeout: 265 seconds]
02:59 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 252 seconds]
03:06 -!- andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has joined #bitcoin-core-pr-reviews
03:09 -!- _andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has quit [Ping timeout: 276 seconds]
03:09 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has joined #bitcoin-core-pr-reviews
03:44 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
03:49 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 240 seconds]
04:01 -!- Common [~Common@096-033-221-075.res.spectrum.com] has quit [Ping timeout: 252 seconds]
04:02 -!- Common [~Common@096-033-221-075.res.spectrum.com] has joined #bitcoin-core-pr-reviews
04:12 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Ping timeout: 265 seconds]
04:18 -!- b10c [uid500648@id-500648.ilkley.irccloud.com] has quit [Quit: Connection closed for inactivity]
04:26 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has joined #bitcoin-core-pr-reviews
04:30 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Ping timeout: 252 seconds]
04:34 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
04:38 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 240 seconds]
04:43 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has joined #bitcoin-core-pr-reviews
04:49 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Ping timeout: 256 seconds]
05:00 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has joined #bitcoin-core-pr-reviews
05:05 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Ping timeout: 252 seconds]
05:17 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has joined #bitcoin-core-pr-reviews
05:23 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
05:27 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 240 seconds]
05:35 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Ping timeout: 256 seconds]
06:14 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
06:19 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 252 seconds]
07:05 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
07:10 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 268 seconds]
07:26 -!- jb55 [~jb55@S010660e327dca171.vc.shawcable.net] has quit [Ping timeout: 240 seconds]
07:26 < stickies-v> Question to learn about everyone's best practices: when you're reviewing a PR commit by commit, do you leave your comments on the commit or on the overall PR in the "Files changed" tab? The former seems preferable in theory, but will also label your comments as "Outdated", and if the author fixes your comment in a later commit you wouldn't see that right away, whereas you would when looking at Files changed. However, in this last
07:26 < stickies-v>  case, the author should probably just amend the first commit instead of fixing it later on?
07:31 < sipa> i generally start by reviewing commit by commit, and then potentially delete comments before submitting them if they happen to be fixed in a later commit
07:38 < stickies-v> That's a good approach to not forget anything without having to keep everything in memory. Thanks for sharing!
07:39 < lightlike> I don't usually use the "files changed" view unless I want to add a code suggestion, which seems to be possible only there.
07:43 < stickies-v> Yeah that's been bothering me for a long time now, I don't understand why they don't add code suggestions on the Commits page too. Alright, two makes a crowd, per-commit comments clearly in the win
07:46 < sipa> i think bitcoin core as a projwct also cares more than average about per-commit code qualuty
07:50 < sipa> oops, that was garbled
08:00 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
08:05 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 268 seconds]
08:18 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
08:37 -!- gleb7 [~gleb@178.150.137.228] has joined #bitcoin-core-pr-reviews
08:41 -!- firsttimeprrevie [~firsttime@p50882413.dip0.t-ipconnect.de] has joined #bitcoin-core-pr-reviews
08:43 -!- b10c [uid500648@id-500648.ilkley.irccloud.com] has joined #bitcoin-core-pr-reviews
08:49 -!- Talkless [~Talkless@mail.dargis.net] has joined #bitcoin-core-pr-reviews
08:57 -!- effexzi [uid474242@id-474242.ilkley.irccloud.com] has joined #bitcoin-core-pr-reviews
08:58 -!- arythmetic [~arythmeti@38.112.106.67] has joined #bitcoin-core-pr-reviews
08:58 -!- svav [~svav@82-69-86-143.dsl.in-addr.zen.co.uk] has joined #bitcoin-core-pr-reviews
09:00 -!- ccdle12 [~ccdle12@243.222.90.149.rev.vodafone.pt] has joined #bitcoin-core-pr-reviews
09:00 < gleb7> #startmeeting
09:00 < gleb7> hi?
09:00 < glozow> hi!
09:00 < jnewbery> hi!
09:00 < arythmetic> hi
09:00 < stickies-v> oi
09:00 < ccdle12> hi
09:00 < dergoegge> hi
09:00 < svav> Hi
09:00 < effexzi> Hi
09:00 < firsttimeprrevie> hi
09:00 < lightlike> hi
09:00 < larryruane> Hi
09:01 < gleb7> i don't know half of the attendees, cool! I also saw some new names already reviewing the PR
09:01 < gleb7> Today we gonna talk about erlay, a long-lasting project :)
09:01 -!- kouloumos [~kouloumos@62.74.91.195] has joined #bitcoin-core-pr-reviews
09:01 < gleb7> More specifically, about a first chunk of it (apart from merging minisketch library to the repo). THis is a first p2p-level part
09:02 < gleb7> First usual question
09:02 < gleb7> Did you review the PR? Concept ACK, approach ACK, tested ACK, or NACK?
09:02 < stickies-v> Concept ACK, hooray for increased connectivity!
09:02 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Remote host closed the connection]
09:03 < glozow> y, though i realized i should also zoom out and look at the overall PR + try running an erlay node and stuff
09:03 < arythmetic> No, just lurking today :)
09:03 < jnewbery> glozow: I agree. It's hard to concept ACK/aproach ACK without looking at the full changeset
09:03 < gleb7> yeah, looking at the overall PR is a good idea for someone interested in this stuff. Original PR also links to 2 issues i made about understanding/measuring performance of this, which should help
09:04 < glozow> yeah, the stats look impressive and the measurement process thorough, but i'm trusting not verifying 😱
09:05 < gleb7> I tried to make reproducing easy, and at least couple people re-verified it, sooo :)
09:05 < gleb7> Looking forward for your verification too
09:05 < gleb7> For me making/reviewing prs splitted in parts is something new, and I don't think we have much experience with that in general (i recall a handfull of projects going through this)
09:06 < gleb7> Which leads us to the next set question, which is probably my favorite
09:06 < gleb7> 2a. What are the benefits of splitting PRs into smaller parts?
09:07 < gleb7> For the context, Erlay PR is 40-something commits, which is definitely more than average, and line-wise as well
09:07 < stickies-v> It makes reviewing much easier. Good and atomic commits already help a lot with this, but it's nice if you can fit the entire PR in your mind model, so ideally it's not too large
09:08 < glozow> i think people review more thoroughly when the PR is smaller (probably not a good thing but seems so)
09:08 < dergoegge> i think github is pretty bad for code review on large change sets with multiple people reviewing different parts, splitting it up makes it easier to have focused review
09:08 -!- azor [~azor@200.109.55.50] has joined #bitcoin-core-pr-reviews
09:09 < gleb7> Yeah, so all these mean that we're probably gonna get less bugs, better code quality, and probably faster than all-at-once approach
09:09 < glozow> dergoegge: +1, if you have more than 150 comments it just becomes impossible
09:09 < stickies-v> It also reduces code going stale since the non-controversial code can be merged more quickly while the more controversial code is discussed in another PR
09:10 < gleb7> My first version was 7 commits I think and oh god it was hard to navigate even by myself, we already went through a lot of structural changes since then. Now we're here
09:10 < jnewbery> I think one downside is that if you're not reviewing the full change set, then you're slightly trusting the author that they're taking you in the right direction
09:10 < gleb7> Yeah, this is the next question :) 2b Are there any drawbacks to this approach?
09:11 < gleb7> jnewbery: this is indeed an issue. Not exactly for this PR, where it's a pretty general thing with not so many design choices.
09:11 < glozow> it might be weird if the sub-PRs don't do much by themselves. lightlike had a good comment about the fact that, if a release was branched off from this PR, nodes would be sending each other meaningless SENDRECONs
09:11 < gleb7> But I believe the next couple PRs would require more context from future chunks indeed.
09:12 < gleb7> glozow: I spent some time thinking about this too. I think this is first time we're merging a p2p message which is useless on its own.
09:13 < jnewbery> eg Carl split https://github.com/bitcoin/bitcoin/pull/20158 into many smaller PRs. Either you do a concept review of the full PR, or you're trusting that Carl is taking us in the right direction and just review the mechanical code changes in the sub-PRs
09:14 < lightlike> I think it's important to not only look at the current PR chunk, but also at the whole thing (simulations/mainnet testing) to get to a concept ACK.
09:16 < gleb7> Yeah, and making the whole thing accessible for reviewers is the whole other challenge. I hope, participants of this PR will have time to look at the big thing too :)
09:16 < glozow> not to shill my own bags but i've been trying to split the package mempool accept PRs into chunks where each one might be useful, e.g. package testmempoolaccept, rbf improvements, etc. i'm not sure if people like that or if it just seems disconnected but 🤷
09:17 < gleb7> glozow: i'll be honest, when i review your prs, I usually trust you and other reviewers w.r.t the big direction
09:18 < gleb7> Making sure it doesn't break anything terribly of course. They are usually just good things on them own, like, decoupling and stuff
09:18 < gleb7> Okay, let's move on to the actual code changes
09:18 < gleb7> When are the nodes supposed to announce Erlay support? What should they consider before doing so and in which cases would it be clearly meaningless
09:18 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
09:18 < gleb7> This is something we've been discussing in the PR already. 
09:19 < stickies-v> We should consider if the peer is blocks-only, and if it supports wtxid relay. If both aren not fulfilled, SENDRECON should not be sent.
09:19 < gleb7> stickies-v: right. do you know why the wtxid relay thing matter?
09:20 < gleb7> anyone is welcome to answer too :)
09:20 < stickies-v> because sketches are based on the wtxids of transactions
09:21 < gleb7> exactly, this is something covered in a BIP. This will be used later on in Erlay commits, but for now it was sufficient to compare the code to BIP.
09:21 < gleb7> There is a dependency on wtxid relay protocol.
09:22 < larryruane> quick side-note, in case anyone wants to run the unit test, you'll need to add `test/txreconciliation_tests.cpp` to `src/Makefile.test.include`, and also change the unit test to `#include <node/txreconciliation.h>` .... was this intentional, that the unit test isn't being built yet?
09:22 < gleb7> larryruane: not at all, good catch.
09:22 < larryruane> (i'll make a comment on the PR)
09:23 < gleb7> Thank you! I was probably moving or renaming something and forgot to do this.
09:23 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 265 seconds]
09:23 < gleb7> Next question.  What is the overall handshake and “registration for reconciliation” protocol flow?
09:25 < svav> First, send a new p2p message sendrecon
09:25 < larryruane> after (existing) version handshake, then the peers have a defined order for trading recon salts (?), depending on who is inbound and who is outbound
09:25 < stickies-v> In the VERSION message, each peer can include some fields to indicate recon support. If such a VERSION message from a peer is received, we PreRegister the peer and send out a SENDRECON message. If we receive a SENDRECON message back from the peer and everything is valid, we initialize the reconciliation state for the peer.
09:25 < larryruane> whoever is outbound goes first
09:26 -!- azor [~azor@200.109.55.50] has quit [Quit: Connection closed]
09:26 -!- azor [~azor@200.109.55.50] has joined #bitcoin-core-pr-reviews
09:26 < gleb7> larryruane: hmmm, why do you think so?
09:27 < larryruane> did I get it wrong, I was looking at https://github.com/bitcoin/bitcoin/pull/23443/commits/b55cbf63e15766bdabcac1c08b4cbfea0badeb04#diff-62e6a7c4c23e68b88bd585db25bb0a10e6ccda2d7fff2f05769bc0a1ad81dcdcR38
09:27 < gleb7> Okay, good to know this is somewhat confusing.
09:28 < larryruane> :) note i'm easily confused
09:28 < gleb7> The 'we_initiate_recon' variable refers to actual transaction reconciliations in the future.
09:28 < gleb7> Not the handshake SENDRECON stuff we're dealing with here.
09:28 -!- jb55 [~jb55@S0106606c634dbdd3.vc.shawcable.net] has joined #bitcoin-core-pr-reviews
09:29 < gleb7> No, this is perfect I think. If you're confused, someone else will also be. And this code should be understandable even without my presence so yeah, I probably should check how good are those comments.
09:29 < lightlike> stickies-v: I don't think anything is changed in the VERSION message to explicitely indicate recon support.
09:30 < stickies-v> lightlike oh you're right! 
09:31 < stickies-v> that also explains the confusion I had about one of your comments regarding peers getting disconnected about sending a SENDRECON message unexpectedly
09:31 < gleb7> The PR *used to* bump the general Bitcoin p2p protocol version (maybe a month ago), but I dropped this. Does anyone know why?
09:32 < gleb7> I'm referring to `P2P_VERSION = 70016`
09:33 < stickies-v> I'm not quite familiar with this, but could it be that an increased p2p protocol version would mean Erlay nodes won't communicate with pre-Erlay nodes anymore, and there's no real need to do that?
09:33 < lightlike> maybe because it is not really necessary for things to work?
09:34 -!- gleb77 [~gleb77@185.151.105.211] has joined #bitcoin-core-pr-reviews
09:34 < glozow> when do we change protocol version? when there'd be a compatibility issue?
09:35 < gleb77> Last time we changed the protocol version for wtxids
09:35 < lightlike> I think they would still be ale communicate, just via the old flooding mechanism (because the SENDRECON messages would be tied to the new protocol version).
09:35 < lightlike> *able to communicate
09:36 < jnewbery> previous protocol versions: https://github.com/bitcoin/bitcoin/blob/926fc2a0d4ff64cf2ff8e1dfa64eca2ebd24e090/src/version.h#L1-L39
09:36 -!- gleb7 [~gleb@178.150.137.228] has quit [Ping timeout: 252 seconds]
09:37 < glozow> jnewbery: ah thanks
09:37 < gleb77> I actually don't have a good answer for this. At some point i just realized we don't need to bump it, so i dropped it.
09:38 < gleb77> There is clearly no issues of Erlay peers talking to non-Erlay peers.
09:38 < jnewbery> Generally, I don't think we need a new protocol version if we're just adding new optional message types. Nodes should ignore messages that they don't know the meaning of (although this hasn't always been the case for alternative implementations)
09:39 < gleb77> Yeah, this now goes out of scope of this PR. Probably deserves a blogpost, about protocol version and also service bits and messages like SENDRECON/SENDCMCPT
09:39 < gleb77> Let's move on
09:39 < gleb77> What is the reason for generating reconciliation salt (m_local_salts)? How is it generated and why?
09:39 -!- grettke [~grettke@cpe-65-29-228-30.wi.res.rr.com] has joined #bitcoin-core-pr-reviews
09:40 < sipa> hi!
09:41 < stickies-v> Since we use short IDs instead of the full ID, multiple tx's could have the same short ID. An attacker could calculate such colliding transactions, but by having each peer calculate their own salt (and used in calculating the short IDs), these transactions would only collide on that specific link and not with the entire network
09:41 < glozow> also you could possibly tell which nodes are the same node if they use the same salt
09:41 < glozow> same node on different networks*
09:42 < svav> Before sending SENDRECON, a node is supposed to "pre-register" the peer by generating and storing an associated reconciliation salt component.
09:42 < sipa> salts are per connection (and side on that connection), not per node, right?
09:42 < glozow> yea
09:43 < stickies-v> It's generated via GetRand(UINT64_MAX)
09:43 < gleb77> All correct answers :)
09:44 -!- azor [~azor@200.109.55.50] has quit [Quit: Connection closed]
09:44 < gleb77> We have 2 reasons for making salts per-connection, not per-node
09:44 < stickies-v> gleb77 as to your 'why?' part, I seem to remember we have multiple Rand functions, some faster than others. Is GetRand the fastest one we can use? I'd think it doesn't need to be very secure?
09:44 < lightlike> both sides contribute 1/2 of the salt and they order and combine them, so they end up with the same salt
09:45 < gleb77> stickies-v: honestly, I didn't think much about the exact random function... i think we use GetRand in a bunch of other places, and this is the number of bits i needed so i used it
09:45 < gleb77> My question was more about combining the salts, etc.
09:45 < lightlike> speed probably doesn't  matter, it's only done once per connection.
09:46 < gleb77> Can anyone guess what would be beneficial of having per-node salts (as opposed to per-connection we do)
09:46 < jnewbery> Very good code comment about our different random functions: https://github.com/bitcoin/bitcoin/blob/926fc2a0d4ff64cf2ff8e1dfa64eca2ebd24e090/src/random.h#L18-L58
09:46 < gleb77> jnewbery: thank you, i wasn't aware
09:46 < glozow> reusing salts means u can cache stuff i guess
09:47 < stickies-v> Hmm but `m_local_salts` from your question doesn't combine anything yet right? That only happens in RegisterPeer when we're updating m_states?
09:47 < gleb77> glozow: maybe, but there's a really big reason
09:47 < gleb77> stickies-v: yeah, hence the 'local' part of the name :)
09:47 < jnewbery> stickies-v: right, the local salt is generated when sending the `sendrecon`, and then combined with the remoate salt when receiving the `sendrecon`
09:48 < gleb77> Okay, so having per-connection salt is a little problematic in terms of the efficiency gains.
09:48 < gleb77> Because nodes receiving announcements can't de-duplicate same transactions (they are hashed with different salts)
09:49 < gleb77> So we ended up sending 1 extra full WTXID message. If we had per-node salt, they would be able to de-duplicate and send/request TX right away after short id
09:49 < gleb77> You will think about this challenge we had to solve after reviewing later parts of Erlay I guess :)
09:50 < gleb77> Let's jump right to question 9
09:50 < gleb77> Why might we use a std::unordered_map instead of a std::map to track salts and states?
09:52 < larryruane> more efficient
09:52 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
09:52 < larryruane> (in time, and probably in space)
09:52 < glozow> std::map is really an ordered_map, and we don't need the map to be ordered. afaik also faster lookup times
09:53 < gleb77> larryruane: I guess this answers "why we want?", second part of the question :)
09:53 < larryruane> glozow: right, map lookup is O(log(n)), unordered map is O(1)
09:54 < gleb77> Yeah, the benefits of reads and writes are massive.
09:54 < gleb77> Probably not that a big deal for couple hundred of records we will have here, but still, there's literally no disadvantage I think in this case, right?
09:56 < stickies-v> I did some quick googling and from that I gather that unordered_map uses more memory than map, but I honestly woulnd't know haha
09:56 < stickies-v> https://thispointer.com/map-vs-unordered_map-when-to-choose-one-over-another/
09:56 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 240 seconds]
09:56 < larryruane> my understanding is, many of our existing maps could be unordered_maps, but unordered maps weren't added (to std at least) until after a bunch of the core code had been written
09:56 < gleb77> stickies-v: cool, something i indeed have to refresh in my memory
09:57 < gleb77> okay, i think this is a good chill note to wrap up the meeting
09:58 < gleb77> I hope most of us got a good impression of the PR, and got inspired about different aspects of it to go ahead and review :)
09:59 < jnewbery> larryruane: right, I think std::unordered_map was only added in c++11
09:59 < larryruane> thank you gleb77! this was great!
09:59 < stickies-v> gleb77 if you still have a bit of time, could you just give some intuition about what happens when we have 2 tx's with colliding short IDs? to make it simple, assuming this happens accidentally and not through an attack
09:59 < glozow> thanks gleb77 :)
09:59 < lightlike> thank you gleb77 !
09:59 < gleb77> thank you guys, see you on github :)
09:59 < dergoegge> thank you gleb77!
10:00 < svav> Thanks gleb77
10:00 < lightlike> gleb77: btw, are your mainnet erlay nodes up? I tried to connect earlier but wasn't able to.
10:00 < gleb77> stickies-v: In the worst case (when they arrive at both nodes right before reconciliation or something like that), they will be "cancelled out" in reconciliation. Those particular colliding transactions won't be exchanged. No other txs or state affected.
10:01 < gleb77> lightlike: I probably have to restart them, I will do that tomorrow. Also, feel free to comment in the corresponding testing issue with your IPs, if you feel so
10:02 < gleb77> lightlike: https://github.com/naumenkogs/txrelaysim/issues/8
10:02 < glozow> gleb77: seems not too bad. hopefully, both nodes will just get those transactions from other peers with different salts
10:02 < glozow> #endmeeting
10:03 -!- svav [~svav@82-69-86-143.dsl.in-addr.zen.co.uk] has quit [Quit: Connection closed]
10:04 < gleb77> glozow: yeah, that's what should happen
10:04 < stickies-v> gleb77 cheers, that helps, thank you!
10:06 -!- ccdle12 [~ccdle12@243.222.90.149.rev.vodafone.pt] has quit [Quit: Client closed]
10:06 -!- gleb7 [~gleb@178.150.137.228] has joined #bitcoin-core-pr-reviews
10:08 -!- firsttimeprrevie [~firsttime@p50882413.dip0.t-ipconnect.de] has quit [Quit: Connection closed]
10:09 -!- kouloumos [~kouloumos@62.74.91.195] has quit [Quit: Connection closed]
10:10 -!- gleb77 [~gleb77@185.151.105.211] has quit [Ping timeout: 256 seconds]
10:11 -!- observer2 [~observer@cpe-23-242-148-67.socal.res.rr.com] has joined #bitcoin-core-pr-reviews
10:16 -!- observer2 [~observer@cpe-23-242-148-67.socal.res.rr.com] has quit [Client Quit]
10:20 -!- yo_soy [~yo_soy@172.92.166.62] has joined #bitcoin-core-pr-reviews
10:28 -!- robot-dreams [sid463268@uxbridge.irccloud.com] has quit [Read error: Connection reset by peer]
10:28 -!- larryruane [sid473749@uxbridge.irccloud.com] has quit [Read error: Connection reset by peer]
10:28 -!- yo_soy [~yo_soy@172.92.166.62] has quit [Quit: Connection closed]
10:28 -!- yo_soy [~yo_soy@172.92.166.62] has joined #bitcoin-core-pr-reviews
10:29 -!- robot-dreams [sid463268@id-463268.uxbridge.irccloud.com] has joined #bitcoin-core-pr-reviews
10:30 -!- larryruane [sid473749@id-473749.uxbridge.irccloud.com] has joined #bitcoin-core-pr-reviews
10:38 -!- grettke [~grettke@cpe-65-29-228-30.wi.res.rr.com] has quit [Quit: Textual IRC Client: www.textualapp.com]
10:41 < stickies-v> jnewbery good catch re un-included modules (https://github.com/bitcoin/bitcoin/pull/23443#discussion_r765026580), do you have a systematic/quick way of checking for that? The best way I know is to go to each declaration, see which header file it's in, and then check if that header file is included, which I feel is unnecessarily laborious
10:42 -!- grettke [~grettke@cpe-65-29-228-30.wi.res.rr.com] has joined #bitcoin-core-pr-reviews
10:45 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
10:47 < sipa> stickies-v: in theory, iwyu :)
10:47 < sipa> though i doubt it works on our codebase
10:50 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 265 seconds]
11:02 -!- andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has quit [Remote host closed the connection]
11:02 -!- andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has joined #bitcoin-core-pr-reviews
11:09 -!- jb55 [~jb55@S0106606c634dbdd3.vc.shawcable.net] has quit [Ping timeout: 265 seconds]
11:12 < stickies-v> thanks for the pointer, looks like there's already some discussion going about iwyu here: https://github.com/bitcoin/bitcoin/issues/15442
11:13 < stickies-v> shame that it's not a perfect solution yet
11:13 -!- yo_soy [~yo_soy@172.92.166.62] has quit [Quit: Connection closed]
11:20 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
11:24 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 240 seconds]
12:02 -!- Talkless [~Talkless@mail.dargis.net] has quit [Quit: Konversation terminated!]
12:04 -!- andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has quit [Remote host closed the connection]
12:07 -!- johnzweng [~johnzweng@zweng.at] has joined #bitcoin-core-pr-reviews
12:10 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
12:14 -!- johnzwen- [~johnzweng@zweng.at] has quit [Quit: Leaving...]
12:15 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 265 seconds]
12:44 -!- Common [~Common@096-033-221-075.res.spectrum.com] has quit [Ping timeout: 240 seconds]
12:45 -!- Common [~Common@096-033-221-075.res.spectrum.com] has joined #bitcoin-core-pr-reviews
12:50 -!- Common [~Common@096-033-221-075.res.spectrum.com] has quit [Ping timeout: 240 seconds]
12:56 -!- tecnecio [~tecnecio_@161.red-2-139-121.dynamicip.rima-tde.net] has joined #bitcoin-core-pr-reviews
12:58 -!- tecnecio [~tecnecio_@161.red-2-139-121.dynamicip.rima-tde.net] has quit [Client Quit]
13:00 -!- tecnecio [~tecnecio_@161.red-2-139-121.dynamicip.rima-tde.net] has joined #bitcoin-core-pr-reviews
13:00 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
13:04 -!- effexzi [uid474242@id-474242.ilkley.irccloud.com] has quit [Quit: Connection closed for inactivity]
13:05 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 240 seconds]
13:23 -!- b10c [uid500648@id-500648.ilkley.irccloud.com] has quit [Quit: Connection closed for inactivity]
13:24 -!- Common [~Common@096-033-221-075.res.spectrum.com] has joined #bitcoin-core-pr-reviews
13:25 -!- Common_ [~Common@096-033-221-075.res.spectrum.com] has joined #bitcoin-core-pr-reviews
13:27 -!- Common__ [~Common@2600:6c5c:7000:300:88d4:e8a7:3b96:1bbe] has joined #bitcoin-core-pr-reviews
13:29 -!- Common [~Common@096-033-221-075.res.spectrum.com] has quit [Ping timeout: 240 seconds]
13:30 -!- Common_ [~Common@096-033-221-075.res.spectrum.com] has quit [Ping timeout: 240 seconds]
13:31 -!- arythmetic [~arythmeti@38.112.106.67] has quit [Remote host closed the connection]
13:32 -!- Common__ [~Common@2600:6c5c:7000:300:88d4:e8a7:3b96:1bbe] has quit [Ping timeout: 252 seconds]
13:39 -!- Common [~Common@096-033-221-075.res.spectrum.com] has joined #bitcoin-core-pr-reviews
13:41 -!- Common [~Common@096-033-221-075.res.spectrum.com] has quit [Client Quit]
13:44 -!- Common [~Common@096-033-221-075.res.spectrum.com] has joined #bitcoin-core-pr-reviews
13:44 -!- Common [~Common@096-033-221-075.res.spectrum.com] has quit [Changing host]
13:44 -!- Common [~Common@user/common] has joined #bitcoin-core-pr-reviews
13:52 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
13:56 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 268 seconds]
14:09 -!- brunoerg [~brunoerg@187.183.47.88] has joined #bitcoin-core-pr-reviews
14:25 -!- brunoerg [~brunoerg@187.183.47.88] has quit [Ping timeout: 240 seconds]
14:42 -!- tecnecio [~tecnecio_@161.red-2-139-121.dynamicip.rima-tde.net] has quit [Remote host closed the connection]
14:55 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
14:59 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 265 seconds]
15:21 -!- tecnecio [~tecnecio_@161.red-2-139-121.dynamicip.rima-tde.net] has joined #bitcoin-core-pr-reviews
15:35 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
15:39 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 268 seconds]
15:43 -!- TheRec [~toto@user/therec] has quit [Ping timeout: 268 seconds]
15:44 -!- luke-jr [~luke-jr@user/luke-jr] has quit [Read error: Connection reset by peer]
15:45 -!- TheRec [~toto@84-75-225-47.dclient.hispeed.ch] has joined #bitcoin-core-pr-reviews
15:45 -!- TheRec [~toto@84-75-225-47.dclient.hispeed.ch] has quit [Changing host]
15:45 -!- TheRec [~toto@user/therec] has joined #bitcoin-core-pr-reviews
15:46 -!- luke-jr [~luke-jr@user/luke-jr] has joined #bitcoin-core-pr-reviews
16:05 -!- jb55 [~jb55@S010660e327dca171.vc.shawcable.net] has joined #bitcoin-core-pr-reviews
16:21 -!- tecnecio [~tecnecio_@161.red-2-139-121.dynamicip.rima-tde.net] has quit [Quit: Leaving]
16:23 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
16:28 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 240 seconds]
16:58 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
17:02 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 252 seconds]
17:47 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
17:51 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 265 seconds]
18:29 -!- virtu [~virtu@p5b15b83b.dip0.t-ipconnect.de] has quit [Ping timeout: 265 seconds]
18:31 -!- virtu [~virtu@p5b15b54f.dip0.t-ipconnect.de] has joined #bitcoin-core-pr-reviews
18:39 -!- brunoerg [~brunoerg@187.183.47.88] has joined #bitcoin-core-pr-reviews
18:43 -!- brunoerg [~brunoerg@187.183.47.88] has quit [Ping timeout: 252 seconds]
18:58 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
19:01 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has joined #bitcoin-core-pr-reviews
19:04 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 265 seconds]
19:20 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Remote host closed the connection]
19:24 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has joined #bitcoin-core-pr-reviews
19:34 -!- brunoerg [~brunoerg@187.183.47.88] has joined #bitcoin-core-pr-reviews
19:38 -!- brunoerg [~brunoerg@187.183.47.88] has quit [Ping timeout: 240 seconds]
19:40 -!- Kaizen_K_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has joined #bitcoin-core-pr-reviews
19:40 -!- Kaizen___ [~Kaizen_Ki@184.105.214.201] has joined #bitcoin-core-pr-reviews
19:43 -!- Kaizen_Kintsugi_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Ping timeout: 252 seconds]
19:44 -!- grettke [~grettke@cpe-65-29-228-30.wi.res.rr.com] has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
19:44 -!- Kaizen_K_ [Kaizen_Kin@gateway/vpn/protonvpn/kaizenkintsugi/x-74018745] has quit [Ping timeout: 265 seconds]
19:57 -!- grettke [~grettke@cpe-65-29-228-30.wi.res.rr.com] has joined #bitcoin-core-pr-reviews
19:58 -!- gleb74 [~gleb@178.150.137.228] has joined #bitcoin-core-pr-reviews
19:59 -!- gleb7 [~gleb@178.150.137.228] has quit [Ping timeout: 252 seconds]
19:59 -!- gleb74 is now known as gleb7
20:26 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
20:30 -!- gleb7 [~gleb@178.150.137.228] has quit [Ping timeout: 252 seconds]
20:31 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 240 seconds]
20:45 -!- gleb74 [~gleb@178.150.137.228] has joined #bitcoin-core-pr-reviews
21:01 -!- Kaizen___ [~Kaizen_Ki@184.105.214.201] has quit [Remote host closed the connection]
21:04 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
21:17 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@184.105.214.201] has joined #bitcoin-core-pr-reviews
21:21 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 265 seconds]
21:45 -!- lol [~lol@host-92-7-40-233.as13285.net] has joined #bitcoin-core-pr-reviews
21:46 -!- masud [~masud@host-92-7-40-233.as13285.net] has joined #bitcoin-core-pr-reviews
21:46 -!- lol [~lol@host-92-7-40-233.as13285.net] has quit [Quit: Connection closed]
21:48 -!- masud [~masud@host-92-7-40-233.as13285.net] has left #bitcoin-core-pr-reviews [WeeChat 3.3]
21:50 -!- masud [~masud@host-92-7-40-233.as13285.net] has joined #bitcoin-core-pr-reviews
21:50 -!- masud [~masud@host-92-7-40-233.as13285.net] has left #bitcoin-core-pr-reviews [WeeChat 3.3]
21:54 -!- masud [~masud@host-92-7-40-233.as13285.net] has joined #bitcoin-core-pr-reviews
22:03 -!- masud [~masud@host-92-7-40-233.as13285.net] has left #bitcoin-core-pr-reviews [WeeChat 3.3]
22:06 -!- brunoerg [~brunoerg@187.183.47.88] has joined #bitcoin-core-pr-reviews
22:20 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@184.105.214.201] has quit [Ping timeout: 265 seconds]
22:24 -!- brunoerg [~brunoerg@187.183.47.88] has quit [Ping timeout: 240 seconds]
22:33 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@184.105.214.201] has joined #bitcoin-core-pr-reviews
22:37 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@184.105.214.201] has quit [Ping timeout: 265 seconds]
22:49 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@184.105.214.201] has joined #bitcoin-core-pr-reviews
22:51 -!- gleb74 [~gleb@178.150.137.228] has quit [Quit: Ping timeout (120 seconds)]
22:52 -!- gleb74 [~gleb@178.150.137.228] has joined #bitcoin-core-pr-reviews
22:54 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@184.105.214.201] has quit [Ping timeout: 265 seconds]
22:55 -!- brunoerg [~brunoerg@187.183.47.88] has joined #bitcoin-core-pr-reviews
22:59 -!- brunoerg [~brunoerg@187.183.47.88] has quit [Ping timeout: 252 seconds]
23:07 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@184.105.214.201] has joined #bitcoin-core-pr-reviews
23:08 -!- Common [~Common@user/common] has quit [Read error: Connection reset by peer]
23:09 -!- Common [~Common@096-033-221-075.res.spectrum.com] has joined #bitcoin-core-pr-reviews
23:12 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@184.105.214.201] has quit [Ping timeout: 265 seconds]
23:25 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@184.105.214.201] has joined #bitcoin-core-pr-reviews
23:33 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@184.105.214.201] has quit [Ping timeout: 256 seconds]
23:46 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has joined #bitcoin-core-pr-reviews
23:46 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@184.105.214.201] has joined #bitcoin-core-pr-reviews
23:50 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:edc5:5a05:30a9:6f11] has quit [Ping timeout: 265 seconds]
23:51 -!- Kaizen_Kintsugi_ [~Kaizen_Ki@184.105.214.201] has quit [Ping timeout: 265 seconds]
--- Log closed Thu Dec 09 00:00:56 2021