Refactoring and minor improvement for self-advertisements (
p2p) Oct 7, 2020
The PR branch HEAD was 25e674e at the time of this review club meeting.
The Bitcoin network uses
addr messages to communicate addresses (the
locations of nodes). To read more about the P2P message, you can see these
Sometimes, we send our own address to our peers so it can be
propagated to new nodes on the network. It can be hard to get new peers if
nobody knows about you :)
Keep in mind that connections are asymmetric. We distinguish between inbound
and outbound peers based on who initiated the connection. Think about what
that entails for address discovery. If Node A initiates a connection to Node
B, do they both necessarily know each others addresses?
There are two code paths for checking & potentially relaying our address:
When we are
version message from a new connection,
When we are starting the
loop for a peer.
Prior to this PR, these two code paths had similar but slightly different
logic for advertising their addresses. This PR unifies the logical flows,
which also entails some slight behavioral changes. Reviewing this PR requires
understanding those differences to ensure they are safe. In some cases, we
might have multiple addresses we can use to refer to ourselves. We’ll dig
into why that is during review club!
Why do we advertise our own address? Is this necessary? When?
How often do we advertise our address? Why is this on a Poisson timer? Does
this frequency make sense?
Why might we have multiple addresses for ourselves? Where do they come from?
What are the behavioral changes in this PR?
PushAddress, will this necessarily send an
addr P2P message?
When you relay an
addr out to a peer, what does the receiving peer do with
the message? What would you anticipate occurring on the network as a whole?
1 19:00 <amiti> #startmeeting
8 19:00 <amiti> hi everyone!
11 19:00 <robot-dreams> hi
15 19:00 <amiti> anyone here for the first time?
19 19:01 <amiti> welcome all :)
21 19:02 <amiti> this week, we’ll be chatting about ✨ addr relay ✨
22 19:02 <pinheadmz> hooray! thats a great kind of relay!
23 19:02 <amiti> propagating node addresses is crucial for nodes find one another to open connections. I find this a really interesting area of the p2p network, and one that I’m just starting to learn about. So just want to be clear that I’m not an expert & I might not have all the answers, but I’ve been digging in and I’m excited to have this sesh to learn together!
24 19:02 <sipa> about time that we *address* this issue
26 19:02 ⚡ spiral_ ba dum tsh
27 19:02 <felixweis> announce all the peers!
28 19:03 <amiti> let's get started!
29 19:03 <@jnewbery> are you relay going to make that joke, sipa?
30 19:03 <amiti> who's had the chance to look at the PR? y/n
42 19:03 <amiti> ( but to be clear, these puns are great. keep em coming )
43 19:04 <amiti> wow nice! lots of review
44 19:04 <Murch> peering over the verge :)
45 19:04 <amiti> ok, so lets start with the fundamentals- why do we advertise our own address?
46 19:04 <Murch> Cuz we want more friends :'(
47 19:05 <stacie> because outbound peers do not necessarily have your address, and like Murch said, you want more friends :)
48 19:05 <willcl_ark> We want to get new inbound connections and populate addrman
49 19:05 <dhruvm> So we can have incoming connections which can help us discover transactions and blocks faster
50 19:05 <emzy> So that others know us.
51 19:05 <spiral_> announce a concrete location to the network where we can be located
52 19:05 <gloriazhao> do we do it even if inbounds aren't enabled?
53 19:05 <amiti> yup! exactly. its to make sure other nodes know how to find us
54 19:05 <Murch> gloriazhao: Excellent question.
55 19:06 <gloriazhao> thank u very murch
56 19:06 <amiti> anybody know the answer to gloriazhao's question?
57 19:06 <Murch> No, but I'm curious as well ^^
58 19:06 <dhruvm> @gloriazhao: i know that addr gossip continues even in -connect mode without -listen
59 19:06 <pinheadmz> instinct is no
60 19:06 <robot-dreams> I think we don't if inbounds aren't enabled or if we're in IDB?
61 19:07 <dhruvm> but i do not know if we will advertise ourselves without -listen
62 19:07 <robot-dreams> (though I'm not sure if I'm interpreting the `fListen` condition correctly)
63 19:07 <Murch> dhruvm: But `-connect` only regulates outbound, right?
64 19:07 <@jnewbery> the gloval fListen is relevant here
65 19:07 <@jnewbery> *global
67 19:07 <amiti> hi! welcome
68 19:08 <jrawsthorne> Thanks amiti
69 19:08 <willcl_ark> Looks like we don't, if -listen is disabled?
70 19:08 <amiti> yeah, I believe `fListen` indicates if the node should accept connections from the outside, and is checked when we are deciding whether to attempt self advertisement
71 19:08 <dhruvm> Murch: yes. with -connect, without -listen, there's no point to growing addrman for ourselves but we still make getaddr calls to the -connect peer
72 19:08 <lightlike> I don't understand completely why this way of advertising was chosen, considering that our peer knows our address already - wouldn't it work as well if a given node regularly sends the address of some of its peers to other peers?
73 19:09 <sipa> lightlike: not if we connected out to them
74 19:09 <amiti> great question lightlike!
75 19:09 <@jnewbery> (fListen is checked for self-advertisement before and after this PR)
76 19:09 <robot-dreams> Related to lightlike's question: do we include our own address in the `version` message when initiating an outbound connection? Alternatively, is that address available to the receiver due to some lower-level network details (e.g. in the TCP header, which I don't know the details of)?
77 19:09 <dhruvm> lightlike: even if they know our address, nodes only share 23% of their addrman with their peers
78 19:09 <pinheadmz> robot-dreams its not in the version message
79 19:10 <pinheadmz> but we can send an ADDR message right after handshake
80 19:10 <sipa> jnewbery: you knew i had to make that joke if nobody else did
81 19:10 <felixweis> dhruvm: don't advertise all the peers!
83 19:11 <amiti> ok, so to dig in- what address information is contained in the version message?
84 19:11 <@jnewbery> dhruvm: that 23% is specifically for getaddr responses. The other way of sharing addresses is through gossiping them periodically, which isn't affected by the 23% constant
86 19:11 <dhruvm> @jnewbery: I see.
87 19:11 <sipa> jnewbery: the period gossip is only for new addresses; it's not from addrman at all
89 19:12 <@jnewbery> sipa: correct
90 19:12 <amiti> ok, so there's two concepts being discussed here: 1. do outbound connections know our address? 2. what is the difference between GETADDR and other ADDR relay?
91 19:13 <pinheadmz> amiti there is a net_addr for both sender and receiver in version
92 19:13 <amiti> lets start with #2, because its essentially been answered. can someone summarize the difference between these two methods for sharing addresses?
93 19:13 <@jnewbery> sipa: oops, yes. What I said was confusing.
94 19:13 <amiti> pinheadmz: yup :)
95 19:13 <Murch> Re 1.: sipa seems to indicate above that they might not, but if it's in the VER message, that would seem incorrect?
96 19:13 <dhruvm> @jnewbery: sipa: Does that mean addr relay is unaffected by the 23% constraint? But getaddr is. If so, out node must initiate the addr, else there's a small chance it's never talked about.
97 19:13 <sipa> CAddress addrMe = CAddress(CService(), nLocalNodeServices);
98 19:13 <sipa> connman.PushMessage(&pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, (uint64_t)nLocalNodeServices, nTime, addrYou, addrMe,
99 19:13 <Murch> Or is the joke that we could lie?
100 19:13 <sipa> nonce, strSubVersion, nNodeStartingHeight, ::g_relay_txes && pnode.m_tx_relay != nullptr));
101 19:14 <sipa> ^ i believe the "our address" in the version message is a dummy these days
102 19:15 <felixweis> so during handshake we learn about our own IP
103 19:15 <amiti> sipa: wait, what? so you're saying the version message has fields for "my address" and "your address", but the first is a dummy, so really we're just sending over "your address"?
105 19:15 <lightlike> but no matter what we put in the version message - shouldn't our outbound peer have some idea of what our address is, considering it communicates with us via p2p?
106 19:16 <pinheadmz> lightlike could be wrong but i think thats kinda handled by the OS- like the handle for the tcp connection
107 19:16 <sipa> amiti: indeed
108 19:16 <pinheadmz> you might see localAddr in getpeerinfo
109 19:16 <sipa> amiti: let me find the change that did that, it's ancient
110 19:16 <robot-dreams> amiti: if we receive a `getaddr`, we respond immediately with an `addr`; there's also a separate loop that sends an `addr` on average every 30 seconds
111 19:16 <amiti> sipa: oh wow, interesting.
112 19:17 <Murch> willcl_ark: looks very relevant.
113 19:17 <willcl_ark> Each bitcoind (presumably) just sees "new connection from (local) port xxxx". It's only "sure" of addresses for outbound connections it makes?
114 19:17 <amiti> robot-dreams: def yes to getaddr/addr response. didn't realize that we send out an addr every ~30 seconds. where is that ?
115 19:17 <sipa> amiti: PR 8740
116 19:17 <willcl_ark> Murch: the older linked issue too (#3088) also had some additional context
117 19:18 <robot-dreams> amiti: I'm looking at `AVG_ADDRESS_BROADCAST_INTERVAL`
118 19:18 <amiti> robot-dreams, sipa: thanks! looks like I'll have lots more to dig into after this session :)
119 19:19 <sipa> in any case, the "my address" function is fullfilled by just sending out addr messages for our own IP
120 19:19 <felixweis> external IP resolution services were the main way to get the IP to announce in the IRC channels?
122 19:20 <willcl_ark> Seems like sometimes an outbound might have a better idea of our address than we do ourselves
123 19:20 <felixweis> willcl_ark: almost always at least for anything that uses NAT
124 19:20 <willcl_ark> no wait, inbounds ...
125 19:20 ⚡ spiral_ fantasizes about all these address-discovery challenges going away with onion addressing
126 19:20 <willcl_ark> yeah
127 19:21 <amiti> but lets keep moving forward. the aspect I wanted to highlight was whether or not advertising our own address is actually necessary. since the peer must have some info about our address whether they are outbound or inbound, seems like there's a possibility of an alternate announcement method. I've thought about tradeoffs of each, maybe we can loop back to this question and dig in more if we have time
128 19:21 <amiti> so- how often do we advertise our address?
129 19:21 <emzy> willcl_ark: Also IPv6 fixes this :)
130 19:21 <Murch> gloriazhao: As in, if my node's outbound peer asks me for my address, I don't tell him? Could be a privacy protection.
131 19:21 <robot-dreams> sipa: If the `addr_from` field of version isn't used, how do we discover "peer's view of our address, which might be more useful than our own view"?
132 19:21 <Murch> gloriazhao: To prevent outbound peers from eclipse attacking easily
133 19:21 <willcl_ark> emzy: hooray! Now if only my ISP would move forward with it...
134 19:22 <sipa> // This asymmetric behavior for inbound and outbound connections was introduced
135 19:22 <sipa> // to prevent a fingerprinting attack: an attacker can send specific fake addresses
136 19:22 <sipa> // to users' AddrMan and later request them by sending getaddr messages.
137 19:22 <sipa> // Making nodes which are behind NAT and can only make outgoing connections ignore
138 19:22 <sipa> // the getaddr message mitigates the attack.
139 19:22 <ajonas> murch: yeah, I think that coinscope protection and what not
140 19:22 <gloriazhao> i think it's to protect the outbound actually, yes?
141 19:22 <gloriazhao> owait no, jk
142 19:23 <sipa> robot-dreams: eh, terminology is confusing... the sender of the version message does not report their address, but does include the address they are using for the peer... so the receiver of the version message learns how they were reached
143 19:23 <ajonas> if I recall correctly, exploiting the getaddr timestamps was used to infer network topology
144 19:24 <sipa> ajonas: indeed
145 19:24 <amiti> ajonas: yeah
146 19:24 <amiti> anyone? how often do we advertise our address?
147 19:24 <lightlike> amiti: on average we advertise once in 24 hours (AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL)
148 19:24 <amiti> lightlike: thanks!
149 19:25 <Murch> That seems like a fairly long interval in an age where a lot of home connections are connected at least once per day
150 19:25 <Murch> s/connected/disconnected
151 19:25 <amiti> and also, its on a Poisson timer
152 19:25 <amiti> does this frequency make sense? why is it on a poisson timer?
153 19:25 <amiti> (two separate questions)
154 19:26 <sipa> it's also filtered by addr_known, so the exact timer probably doesn't matter much
155 19:26 <ares_> if we use the information of a received version message as our own IP, doesn't that make us susceptible to attacks if a peer deliberately announces a wrong IP?
156 19:26 <felixweis> is auto dis/reconnect common for ISPs in the US/other countries too? new IP every 24 hours
157 19:26 <robot-dreams> sipa: Thanks! Now it makes sense what `addrMe` means in `ProcessMessage` when processing an incoming `version`.
158 19:26 <amiti> ares_ : good question!
159 19:26 <lightlike> Murch: but we advertise on the beginning of each connection (not only after 24 hours). That's why m_next_local_addr_send needs to be initialized to 0.
160 19:27 <sipa> ares_: there is a list of known "local IP" values; we only use the reported ones inside version messages to score those relative to each other
161 19:27 <Murch> lightlike: Thnaks, that makes sense
162 19:27 <willcl_ark> Perhaps we want the frequency to be _close_ to every 24h, but also to defeat any possible fingerprinting of exactly 24h?
163 19:27 <sipa> ares_: we don't use it to learn what our own IP(s) are in the first place
164 19:28 <sipa> i forgot, is the local relay timer per connection or not?
165 19:28 <ares_> sipa: oh, okay. then that's not a concern
166 19:28 <Murch> amiti: to prevent that announcements can be used to infer topology via timing?
167 19:28 <felixweis> willcl_ark: i think thats correct, makes it harder to guess when exactly we came online
168 19:28 <robot-dreams> ares_: Yeah, good point. Does the `IsPeerAddrLocalGood` check protect us against the "deliberately announces a wrong IP" attack?
169 19:29 <amiti> oh right, I want to clarify there are two methods that trigger us to self advertise: 1. when we are processing a VERSION message a peer sent (so right after opening a new connection) 2. on an ongoing basis, ~1 / day
170 19:29 <@jnewbery> sipa: local relay timer is per-peer
171 19:29 <amiti> willcl_ark, murch: yeah, I think generally poisson is used for privacy benefits, but I was wondering what is trying to be kept private here. we're literally announcing our address which is public information
172 19:30 <sipa> jnewbery: thanks... in that case i don't think the poisson timing does anything, as the peer knows when the connection was established anyway
173 19:30 <gloriazhao> what can someone do with information about when we came online?
174 19:30 <amiti> as murch said, there might be the possibility of topology inference
175 19:30 <sipa> no, i think it may have just have been added as a "poisson all the things!" change
176 19:30 <amiti> but another element is trying to prevent any synchronized network events to make sure there aren't random bandwidth spikes
177 19:30 <sipa> i don't see the benefit in this case
178 19:30 <willcl_ark> "In bitcoin we like pseudo-randomness?"
179 19:30 <felixweis> poisson processes are cool tho...
180 19:30 <Murch> Sounds fishy anyway.
181 19:31 <gloriazhao> murch just won best pun of the day
182 19:31 <willcl_ark> Another way of looking at the question; "why not poisson things?" :)
183 19:31 <spiral_> gloriazhao I agree, seems like obfuscating our start time has a privacy benefit
184 19:31 <sipa> well it's actually an exponential distribution, not a poisson one ;)
185 19:31 <ares_> robot-dreams: just looking at the function for the first time. not sure which of those checks corresponds to what sipa was saying (i.e., comparing to a list of known IPs)
186 19:31 <@jnewbery> I think that the default distribution for any randomized timer that results in network messages should be poisson, and we should only use a non-poisson distribution if there's a good reason to.
187 19:31 <Murch> spiral_: but we seem to announce/advertise to each of our peers when coming online anyway
188 19:31 <robot-dreams> amiti: Would it make sense to apply Poisson timing to "send out `addr`" but NOT apply Poisson timing "add our own address to the next `addr` to be sent"?
189 19:32 <willcl_ark> sipa: are you saying `PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL)` is lying to me?
190 19:32 <spiral_> Murch e.g. your point being it's not total obfuscation?
191 19:32 <amiti> robot-dreams: good question! seems like one for the floor rather than me specifically :)
192 19:32 <sipa> willcl_ark: it simulates a poisson process, by generating intervals that are exponentiall distribution :)
193 19:32 <willcl_ark> oh no; a fake poisson
194 19:32 <sipa> *exponentially *distributed
195 19:32 <felixweis> simulation theory confirmed
196 19:33 <Murch> spiral_: I mean, if we're still online after 24h, they can just use the IP to collect the two events. If we have a new IP, we do a new announcement anyway.
197 19:33 <sipa> willcl_ark: no no, that is literally how poisson processes work; i'm just commenting on the fact that the name may imply it generates poisson-distributed values, which is not the case
198 19:33 <emzy> felixweis: :D
199 19:33 <spiral_> Murch I see your point now :]
200 19:33 <Murch> spiral_: So, I'm starting to see why sipa said that he isn't aware of a privacy benefit
201 19:33 <willcl_ark> sipa: phew
202 19:34 <stacie> robot-dreams - earlier you mentioned the separate loop that sends an `addr` every 30 seconds (via AVG_ADDRESS_BROADCAST_INTERVAL), how is that different from the advertising that is done once every ~24 hrs (AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL)?
203 19:34 <lightlike> Does anyone know why is method 1 (advertising during VERSION only for outbounds) is necessary - if we just skipped that, wouldn't we self-advertise via SendMessages a few milliseconds later anyway, considering that m_next_local_addr_send is initialized to 0?
204 19:34 ⚡ spiral_ just now got the "fishy" joke
205 19:34 <robot-dreams> stacie: yeah, great point. My understanding was that `AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL` is for "add our address to the next outgoing batch" but not for actually doing any sending
206 19:34 <sipa> lightlike: i believe it may be pointless
207 19:35 <stacie> ah, got it
208 19:35 <@jnewbery> is it important that we send our own addr to addr_fetch peers?
209 19:35 <sipa> robot-dreams: that's my belief too
210 19:35 <willcl_ark> lightlike: looks like a good PR
211 19:36 <Murch> Breaking: Bitcoin Core to reduce advertising by 50%
212 19:36 <sipa> Murch: also there is a addr_known filter where we remember which addrs the peer is already aware of, and won't send a second time
213 19:36 <amiti> jnewbery: for those unfamiliar, can you share what an addr_fetch peer is?
214 19:36 <Murch> sipa: I see.
215 19:36 <amiti> or, can anybody else answer? :)
216 19:36 <sipa> so i think even the 24h-poisson delay may result in it not being sent a second time, as the old address may still be in the addr known filter
217 19:37 <amiti> sipa: agreed.
218 19:37 <spiral_> sipa would that be a performance-degrading facet of the code?
219 19:37 <sipa> spiral_: no, how?
220 19:38 <@jnewbery> addr_fetch are short-lived connections that we make to peers where we send a getaddr, wait for the addr response (this is the one that may contain up to 23% of addresses from the peer's addrman) and then disconnect
221 19:38 <amiti> I didn't do the math, but I imagine the time is takes the filter to roll would be very variable based on eg. if you are accepting inbounds (even though its a per-peer filter, you would have more address traffic you send out)
222 19:38 <sipa> amiti: indeed
223 19:38 <amiti> okay, 38 minutes in, time for question 3 😛
224 19:38 <amiti> why might we have multiple addresses for ourselves?
225 19:39 <amiti> where do they come from?
226 19:39 <Murch> jnewbery: Sounds very enabling to topology crawlers like bitnodes :p
227 19:39 <@jnewbery> I ask whether it's important to send our own address to them because if so, then having the PushAddress() call inside the VERSION message processing might affect whether we send them our address or not
228 19:39 <Murch> amiti: Because we might live in a weird internal network, where we think we have a different address than the global ip
229 19:39 <spiral_> sipa I was thinking that if the address was unintentionally stale it might cause some errors -- more so from ignorant speculation on my part ;)
230 19:40 <Murch> And some of our peers could be in the same sub network
231 19:40 <felixweis> different interfaces? if we connect to a peer on the local network they might see a different address than one on the global internet
232 19:40 <robot-dreams> Is there any IPv4 / IPv6 stuff thrown in there as well?
233 19:40 <Murch> Or, Onion, IPv4, IPv6?
234 19:40 <amiti> murch, felixweis: yup, those are def some use cases
235 19:41 <amiti> I found it interesting to compare what my computer thinks my ip address is (using `ifconfig | grep inet` vs what googling "what is my ip address" returns. very different!
236 19:41 <Murch> We could even have more than one internet connection!
237 19:41 <jrawsthorne> Can we listen on multiple hosts/ports at the same time?
238 19:41 <sipa> jrawsthorne: sure
239 19:41 <felixweis> we can listen on v4 and v6 simulataneous
240 19:42 <sipa> you can specify -listen multiple times too, if you have multiple ipv4/ipv6 addresses you're reachable on
241 19:42 <felixweis> i believe by default we listen on all the interfaces
243 19:42 <amiti> there's also the `-bind` command line arg where users can give addresses they want to listen to
244 19:42 <emzy> In IPv6 you could have more then one Internet connection at home and use both.
245 19:43 <emzy> Ok. Maybe nearly nobody uses this feature.
246 19:44 <amiti> hahahha, so there are lots of reasons we might have multiple local addresses!
247 19:44 <spiral_> amiti: wouldn't it be the case that `ifconfig | grep inet` is always going to list a different IP than a IP-query service unless you're connected right to the ISP modem, aka if there's a gateway in front of you `ifconfig | grep inet` is just your ip from a router?
248 19:44 <emzy> Already outed myself as IPv6 fanboy.
249 19:44 <sipa> amiti: yeah, it probably means you're behind a local NAT?
250 19:44 <Murch> amiti: I'd say that's why we want to know how other peers see us in the first place. Cuz we often don't even know.
251 19:44 <willcl_ark> Your router could also forward multiple ports to your bitcoind machine
252 19:45 <sipa> emzy: aren't we all?
253 19:45 <sipa> Murch: no, it's not used for that (too easy to lie)
254 19:45 <emzy> sipa: I hope so. IPv4 is deprecated.
255 19:45 <Murch> sipa: It is used for our self-advertisement via that peer.
256 19:46 <Murch> to clarify "cuz we often don't know _how they see us_"
257 19:46 <sipa> Murch: yes, but we determine what our local addresses are through a different mechanism (-externalip, upnp, local interface if it's public, ...)
258 19:46 <amiti> spiral_, sipa: right, makes sense. was just cool to see a specific example :) my impression is that the majority of the time its going to be different?
259 19:46 <Murch> yeah, I was not clear
260 19:46 <sipa> Murch: then we use "how they see us" to score those list of addresses
261 19:47 <amiti> sipa: are you talking about the weighting logic of `mapLocalHost` and `nScore`?
262 19:47 <spiral_> Are there any defensive benefits of our peers telling us *what they believe is our network location*? E.g. if all peers are honest with our reported location, if an accurate message makes it to us, and one peer has an aberrant address, our node might learn of a MITM interference
263 19:47 <sipa> amiti: yes
264 19:47 <felixweis> whats the rationale for switching form * to &
265 19:47 <amiti> sipa: I found it a bit hard to parse, do you have a high level overview of how the scores work that you could share?
266 19:48 <@jnewbery> sipa: oh, so if a peer tells us our address and we don't already know the address through one of those other mechanisms, we'll never use that as our self-advertised address to other peers?
267 19:48 <sipa> jnewbery: correct
268 19:48 <sipa> amiti: it depends on how you define "majority of the time"... for home users, sure
269 19:48 <amiti> sipa, jnewbery: ooooo
270 19:48 <amiti> wow! this stuff is wild!
271 19:49 <sipa> amiti: if you run a bitcoind as an internet service, on a server in datacenter somewhere your local interface will have a publicly routable IP usually
272 19:49 <amiti> sipa: ah, I see
274 19:50 <sipa> if you use bitcoind behind a NAT you need to use -externalip to configure your public IP, or use UPnP (which makes bitcoind talk to your router and ask it for its public IP)
275 19:50 <amiti> oh snap, we only have 11 minutes left. umm maybe we should talk about the changes in this PR 😂
276 19:50 <sipa> amiti: actually, which PR is it?
277 19:50 <sipa> i don't think you mentioned that ;)
278 19:50 <amiti> can anyone overview what this PR is doing, and what are the behavioral changes
279 19:50 <felixweis> amiti: or if you come to chaos communication congress
280 19:50 <@jnewbery> felixweis: since gleb was basically touching every line of the function I suggested he change the signature. I think the only reasons to use a raw pointer instead of a reference are (1) if you want to be able to pass null; or (2) if you want to reseat the pointer (point it at something else). Neither of those are the case here, so pass-by-reference communicates intent better
281 19:50 <amiti> sipa: lolol 19843
282 19:50 <amiti> sipa: gotcha! thanks thats very helpful
283 19:51 <sipa> felixweis: is 37c3 happening?
284 19:51 <felixweis> every laptop/smarphone/raspiblitz gets a publicly routable address at europes biggest hacker conference
285 19:51 <amiti> felixweis: I have no idea what chaos communication congress is, but I like it. I'm definitely in.
286 19:51 <Murch> felixweis, jnewbery: Thanks, I was wondering as well.
287 19:51 <robot-dreams> amiti: PR overview: there used to be two different code paths for adding our own address to the "addresses to be sent on next `addr`" list, and this PR consolidates them.
288 19:52 <felixweis> sipa: certainly not :(
289 19:52 <robot-dreams> As for behavior change, now we might randomly override our local address with the peer's view of us when responding to the peer's initial `version`.
290 19:52 <emzy> sipa: 37C3 will be online.
291 19:52 <amiti> robot-dreams: yeah! thats my understanding as well
292 19:52 <robot-dreams> However, my guess is this could only happen if we connect to an outbound peer who already knew about us?
293 19:53 <felixweis> jnewbery: thanks. looked like a c++ style quesion but i don't know too much about it so i wasn't sure to ask
294 19:53 <amiti> right so the behavior change comes from the logic used to decide which local address to relay
295 19:53 <Murch> robot-dreams: Wasn't it only for the self-advertisement after establishing the new connection?
296 19:54 <robot-dreams> Murhc: Yes, what I'm saying is, after the PR, could the address we self-advertise could be different?
297 19:54 <Murch> Yes, but only on a per-peer basis
298 19:54 <robot-dreams> Murch* sorry
299 19:54 <felixweis> amiti: hope to see you at one event in the not all to distant future
300 19:55 <amiti> okay and to squeeze in the last couple questions, lets just thread this conversation
301 19:55 <Murch> I'm not sure whether it could already be part of the VER handshake
302 19:55 <amiti> 5. When you PushAddress, will this necessarily send an addr P2P message? (hint: we've already discussed this)
303 19:55 <amiti> 6. When you relay an addr out to a peer, what does the receiving peer do with the message?
304 19:55 <elle> amiti: 5) no. it might be replaced by another one later on if the vAddrToSend is too full. It will also not be added to vAddrToSend (and hence not sent) if m_addr_known filter has a matching address.
305 19:56 <Murch> amiti: fanmail and stickers. Definitely to send fanmail and stickers.
306 19:56 <amiti> elle: very nice!
307 19:56 <amiti> murch: o.0 ?
308 19:56 <Murch> err, it adds it to the list of peers it will advertise?
309 19:57 <amiti> ahhahahahha oh I get the joke now
310 19:57 <Murch> if one of the peer's peers asks for more peers
311 19:57 <sipa> needs more interrobang
312 19:58 <amiti> murch: yeah, adds to addrman as one thing
313 19:58 <@jnewbery> elle: great answer! In reality, I think it's uncommon for vAddrToSend to fill up except when we process a GETADDR (which can only happen once for each connection)
314 19:58 <amiti> but also, when a node receives an addr message, if there's < 10 of them & a couple other conditions are met, it will relay it out to 1-2 other peers
315 19:58 <willcl_ark> Also, since #18991 the address would not propagate into our cached AddrMan for some time
316 19:59 <elle> jnewbery: ok cool, thanks. yeah i see MAX_ADDR_TO_SEND=1000 which is pretty big!
317 19:59 <amiti> willcl_ark: ya, so that impacts the GETADDR responses I believe
318 20:00 <amiti> okay, final minute... any burning questions?
320 20:00 <@jnewbery> elle: right, and when we receive a gossipped address, we'll add to the vAddrToSend vectors for at most 2 peers.
321 20:00 <ajonas> thanks amiti
322 20:00 <amiti> and thats a wrap!
324 20:00 <robot-dreams> thanks!
325 20:00 <felixweis> thanks amiti!
326 20:00 <emzy> thanks all!
327 20:01 <stacie> thanks amiti!
328 20:01 <spiral_> thank amiti
329 20:01 <@jnewbery> great pun review club meeting. Thanks amiti!
330 20:01 <willcl_ark> thanks amiti!
331 20:01 <gloriazhao> thanks amiti!
332 20:01 <ajonas> addriós amigos!
333 20:01 <amiti> thanks all, that was awesome! great questions. I learned a lot & have more I want to dig into :)
334 20:01 <amiti> #endmeeting