Net: CNetAddr: add support to (un)serialize as ADDRv2 (
p2p) Sep 16, 2020
The PR branch HEAD was 3eef1e6 at the time of this review club meeting.
Today’s PR 19845 is part of
19031, a proposed
implementation of the
addrv2 is a new p2p message format proposed in early 2019 by Wladimir J. van
der Laan to support networks that have longer endpoint addresses than fit in
the 128 bits/16 bytes of the current
Issue 18884 “Tor v3
support” is the meta-issue
for tracking overall progress on BIP155 and its implementation.
This work is urgent because Tor v2 was deprecated yesterday (September 15,
2020) with 0.4.4.x and will be obsoleted in 0.4.6.x (July 15, 2021). Tor v2 is
expected to be completely disabled in Tor client stable versions on October
15, 2021 as
here: “From today (July
2nd, 2020), the Internet has around 16 months to migrate from onion services
v2 to v3 once and for all.”
Why is Tor v2 EOL? According to
this Tor mailing list
“To very quickly summarize why we are deprecating, in one word: Safety. Onion
service v2 uses RSA1024 and 80 bit SHA1 (truncated) addresses. It also still
uses the TAP handshake which has been entirely removed from Tor for many years
now except v2 services. Its simplistic directory system exposes it to a
variety of enumeration and location-prediction attacks that give HSDir relays
too much power to enumerate or even block v2 services. Finally, v2 services
are not being developed nor maintained anymore. Only the most severe security
issues are being addressed.”
previous review club meeting discussing the parent PR 19031
provides more background and context. Network address types newly recognized by PR 19845
Next-generation Tor v3 Onion
“Better crypto (replaced SHA1/DH/RSA1024 with SHA3/ed25519/curve25519),
improved directory protocol leaking much less information to directory
servers, a smaller surface for targeted attacks, better onion address
security against impersonation, and a more extensible introduction/rendezvous
Invisible Internet Project (I2P) - “The I2P network
provides strong privacy protections for communication over the
Internet. Many activities that would risk your privacy on the public
Internet can be conducted anonymously inside I2P.”
Cjdns - “Cjdns implements an encrypted
IPv6 network using public-key cryptography for address allocation and a
distributed hash table for routing. This provides near-zero-configuration
networking, and prevents many of the security and scalability issues that
plague existing networks.” With PR 19845, you can make peer connections to these network address types
by using the
addnode RPC or the
options. Here’s how:
./src/bitcoin-cli help addnode
./src/bitcoind -h | grep -A4 "\-addnode=\|\-connect="
rumouring these address types, more is needed. For instance, PR
19954—the remaining step of 19031—enables running bitcoind as a Tor v3
service. Technical Notes
Today’s PR 19845 adds the ability to serialize and deserialize internet
addrv2 format. It also adds Tor v3 address parsing and builds
“Implement Keccak and SHA3_256”. The latter is needed for Tor v3 support, as
the conversion from BIP155 encoding to .onion notation uses an SHA3-based
addrv2 feature is enabled by OR-ing an integer flag,
into the stream version.
The PR makes use of the custom
Span type, which was introduced into Bitcoin
PR 12886 and
represents a vector-like view to a range of contiguous elements in memory
std::span in C++20. If you are not familiar with
Span, it was
discussed in a
recent review club meeting.
Tor v3 address parsing should follow
which is also summarized
in the BIP. Questions
Warm-up question #1: Visually, how can you tell the difference between a Tor
v2 and v3 address?
Warm-up question #2: List all the network address types that Bitcoin Core
can support after this PR. What is the size in bytes (address length) for
each of them? What is the maximum address length Bitcoin Core will be able to
Warm-up question #3: Is
addrv2 deserialization faster or slower than v1? By
roughly how much?
Did you review the PR? Bonus: Did anyone also review
19841, “Implement Keccak and
What steps did you take to review this PR? Did you review on GitHub, or in
your local dev environment? Did you review commit-by-commit, or the whole
diff? What did you review first: the code, the tests, or
Did you verify that the code corresponds to the spec for each network address
type? Any thoughts on the test coverage? Do you see anything that is not tested or
could also be tested?
When should the new
sendaddrv2 message type, aka “send me addrv2”, be sent?
What does this implementation do if an unknown or non-validateable network
address type is encountered? How about a known network with wrong length? Are
these considered misbehavior? Discouraged? Not relayed? Ignored? Why?
What is meant by “variable-length addresses” in BIP155 and when referring to
How will the new address formats be saved in
peers.dat, which currently
persists addresses in 16-byte IPv6 format?
Why was the boolean
pad parameter added to
How do you think Bitcoin Core should make the transition from Tor v2 to v3?
All at once, or v3 first opt-in, then default (and v2 opt-in or deprecated)?
In which releases?
1 19:00 <jonatack> #startmeeting
3 19:00 <jonatack> Hi all! Welcome to this week's episode of the Bitcoin Core PR Review club!
4 19:00 <jonatack> #topic This week, we are looking at PR 19845 - "Net: CNetAddr: add support to (un)serialize as ADDRv2" (p2p)
7 19:00 <michaelfolkson> hi
10 19:00 <jonatack> Anyone here for the first time?
13 19:01 <troygiorshev> hi
14 19:01 <jonatack> Let's multithread this and dig in. Warm-up question #1: Visually, how can you tell the difference between a Tor v2 and v3 address?
17 19:02 <jonatack> Warm-up question #2: List all the network address types that Bitcoin Core can support after this PR. What is the size in bytes (address length) for each of them? hint: src/netaddress.h::95-114
18 19:02 <emzy> Lengh of the onion address.
19 19:02 <shaunsun> v3 is longer right?
22 19:02 <urethane> v3 is much longer
23 19:02 <jonatack> emzy: shaunsun: urethane: yes
24 19:02 <michaelfolkson> 32 bytes vs 10 bytes
25 19:02 <jonatack> A tor v2 address is 15 characters in length, not counting the .onion suffix: 57qr3yd1nyntf5k.onion
26 19:02 <jonatack> A tor v3 address is 56 characters: 7zvj7a2imdgkdbg4f2dryd5rgtrn7upivr5eeij4cicjh65pooxeshid.onion
27 19:03 <jonatack> (visually)
28 19:03 <urethane> e.g. openpgp.org v3: zkaan2xfbuxia2wpf7ofnkbz6r5zdbbvxbunvp5g2iebopbfc4
29 19:03 <emzy> ipv4, ipv6, torv2, torv3, I2P, CJDNS.
30 19:03 <urethane> .onion
31 19:03 <sipa> it's that much longer because it's not just the payload going from 10 to 32 bytes, but also a version number and checksum are added
32 19:04 <jonatack> emzy: right, ipv4 4 bytes; ipv6 16 bytes; torv2 10 bytes; torv3 32 bytes; i2p 32 bytes; cjdns 16 bytes
33 19:04 <jonatack> Question 2 *BONUS*: describe any changes going from addrv1 to addrv2, in the size of addrv1-supported addresses, e.g. ipv4, ipv6, torv2?
34 19:04 <vasild> also "internal" is supported :)
36 19:05 <sipa> cjdns could be 15 bytes, i think?
37 19:05 <vasild> ah, right!
38 19:05 <pinheadmz> the serialization of all adress types is different in addrv2
39 19:05 <michaelfolkson> Is it padded to make it 16?
40 19:06 <pinheadmz> UnserializeV1Stream() vs UnserializeV2Stream() in netaddress.h
41 19:06 <vasild> hmm, do we want to rely on the fact that they will never change the 1 byte prefix to something else?
42 19:06 <sipa> michaelfolkson: cjdns nominally uses IPv6 addresses in range fc00::/8
43 19:06 <sipa> since the first byte is always 0xfc, we could drop it from relay, as it's already explicitly tagged with the cjdns marker
44 19:06 <sipa> vasild: good question; perhaps not worth it
45 19:07 <jonatack> Anyone wish to describe changes going from addrv1 to addrv2, in the size of addrv1-supported addresses?
46 19:08 <pinheadmz> oh are we not using 16 bytes for ipv4 anymore?
47 19:08 <pinheadmz> because we have more speciifc types instead of trying to use 16 bytes for "everything"
48 19:08 <sipa> pinheadmz: indeed!
49 19:08 <jonatack> no, it's now much smaller
50 19:08 <jonatack> significantly
51 19:08 ⚡ pinheadmz fist bump booyah
52 19:09 <jonatack> ipv4: 16 -> 6 bytes
53 19:09 <jonatack> ipv6: 16 -> 18 bytes
54 19:09 <jonatack> oops, it's bigger, but...
55 19:09 <jonatack> torv2: 16 -> 12 bytes
56 19:09 <jonatack> estimate the bandwidth savings
57 19:10 ⚡ urethane dreams of gigamegs
59 19:10 <jonatack> iirc vasild estimated ~ half less bandwidth in a recent irc discussion
61 19:11 <jonatack> Quick: What is the maximum address length Bitcoin Core will be able to support?
62 19:11 <vasild> hmm, is there a way to dump all addrman database in a human readable form?
63 19:11 <michaelfolkson> 512
64 19:11 <wiz> hmm, I guess it's not possible to fit a v3 onion in a DNS Seed unless we implement TXT records or something eh
66 19:12 <jonatack> michaelfolkson: yes!
67 19:12 <vasild> wiz: right
68 19:12 <michaelfolkson> Which is....huge
69 19:12 <jonatack> wiz: right
70 19:12 <wumpus> that is true, though, DNS seeds are unusable through tor anyhow
71 19:12 <sipa> wiz: indeed, but on the other hand... if you're using tor you probably don't want to use DNS seeds (directly) in the first place (and bitcoin core doesn't)
73 19:13 <sipa> instead we ask an exit node to do the resolving, connect to one of the seed's results, send it a getaddr, and disconnect when the response comes back
74 19:13 <sipa> which will transparently keep working for addrv2, and support torv3 just fine
75 19:13 <wumpus> (this is also, in general, the case for other overlay networks)
76 19:14 <jonatack> Question: did you review the PR? (y / n)
78 19:14 <wiz> so what's the purpose of the onioncat encoded AAAA records that start with fd87:d87e:eb43::/48 then? totally unused?
80 19:14 <emzy> sipa: so without clearnet nodes this will not work. But clearnet is not dead yet.
81 19:14 <urethane> wumpus is it "DNS seeds are unusable through tor anyhow" or DNS seeds are ill-advised given user expectations of a node operator using tor?
82 19:14 <sipa> wiz: relay of onion addresses to non-tor nodes
83 19:14 <emzy> y (only tested)
84 19:15 <wiz> okay, so do we still want to support that for v3 onions in some new encoding or TXT records?
85 19:15 <wumpus> urethane: they are not usable through Tor. You can do DNS lookups through Tor (through a SOCKS5 extension), in principle, but not the kind of query that returns multiple results.
86 19:15 <sipa> wiz: that would be a ton of work, i'm afraid
87 19:15 <sipa> (as in: implement our own dns resolver that works over tor...)
88 19:16 <urethane> wumpus ty
89 19:16 <wumpus> urethane: they're also super-unreliable, because it's controlled by exit nodes
90 19:16 <wumpus> wiz: no, I think that's not necessary
91 19:16 <urethane> wumpus that's the security fallibility I thought you were referring to
93 19:17 <wumpus> emzy: it works without clearnet nodes; in that case it would rely on the hardcoded onion seeds in the source code though
94 19:18 <wumpus> (which is okay, some are just as reliable as the DNS seeds)
95 19:18 <shaunsun> partial y (was able to connect to v3 addresses)
96 19:18 <jonatack> shaunsun: nice
97 19:19 <pinheadmz> doesn't tor work with some kind of directory nodes to help route?
98 19:19 <emzy> wumpus: I will make a mental note to set one up then.
99 19:19 <pinheadmz> like isnt there a tor version of DNS in that sense
100 19:20 <sipa> pinheadmz: yes, but for hidden services and relays (I think) - not DNS
101 19:20 <wumpus> correct, Tor doesn't use names (for anything besides routing through exit nodes), only public keys
102 19:20 <pinheadmz> sure bc theres no IPs in tor anyway really - but see what im getting at? could there be a bitcoin core tor directory that served bootstrapping nodes over tor?
103 19:21 <pinheadmz> er, i guess thats the same as just connecting to a seed node over tor and getting addr messages
104 19:21 <emzy> pinheadmz: I think you can see a hardcodes onion seed as a dnsseed for tor.
105 19:21 <wumpus> but why? any node can give you other peers, it's decentralized on purpose
106 19:21 <pinheadmz> yeah
107 19:22 <jonatack> pinheadmz: "directory" node... just to recap, irc there are 3 basic types of tor nodes: bridge/gate, relays and exit; often a tor node has more than one of those roles
108 19:22 <urethane> pinheadmz "some kind of directory nodes to help route?" I think you're referring to what's called HSDirs in tor
109 19:22 <michaelfolkson> Won't it take a while to find a node that gives you Bitcoin peers? Connect to loads of Tor nodes that aren't interested in Bitcoin
110 19:23 <sipa> pinheadmz: "could there be a bitcoin core tor directory that served bootstapping nodes over tor?" -> that's called a seed node
111 19:23 <sipa> and it needs literally zero modifications to work :)
112 19:23 <pinheadmz> yep I realize i just circled back to what we already ahve
113 19:23 <vasild> contrib/seeds/nodes_main.txt
114 19:23 <wumpus> michaelfolkson: it will only connect to (supposed) bitcoin peers which will give you other bitcoin peers
115 19:24 <urethane> tor v2 doesn't have privacy for onion addresses from an HSDirs observer, tor v3 does have onion service privacy from HSDirs
116 19:24 <wumpus> torv3 definitely improved things in that regard
117 19:25 <urethane> for that reason alone, users should stop using v2 :)
118 19:25 <vasild> they will be forced to stop using v2 very soon
119 19:25 <vasild> in less than a year
120 19:26 <jonatack> snooping of HSDir relays is improved by v3? TIL
121 19:26 <michaelfolkson> Ok yeah. But you need to find (supposed) Bitcoin peers in the first place. Lots of trial and error pinging?
122 19:26 <wumpus> michaelfolkson: that's what the hardcoded peers are for, which are updated every major version
123 19:27 <wumpus> michaelfolkson: it will never ping random addresses to find bitcoin peers, never :)
124 19:27 <michaelfolkson> Ok thanks
125 19:27 <sipa> and the DNS seeds
126 19:27 <jonatack> michaelfolkson: if you have some good addnode peers in your conf file, maybe seednode too, it makes a big difference afaict
127 19:28 <wumpus> you could potentially do that in 32 bit IPv4, even thn it'd likely take a long time, but it would be pointless with 32 byte addresses
129 19:28 <urethane> list_of_every_ip_address.txt
130 19:29 <jonatack> I'm not sure there's much point in going through the "how did you review" questions
131 19:30 <jonatack> I did like the tests vasild added
132 19:30 <michaelfolkson> And Sjors' test suggestions
133 19:31 <vasild> I added two more (not pushed yet), guided by code coverage (seeing which line is not covered and writing test for it)
134 19:31 <wumpus> I only tested (have two running Torv3 nodes now) and did very light review FWIW
135 19:31 <michaelfolkson> What do you use for code coverage? Marco's site?
136 19:31 <jonatack> indeed, I think some addrv1 unit tests could be added to have equivalent coverage
137 19:32 <jonatack> michaelfolkson: i think vasild uses his own tool for that... vasild?
138 19:32 <wumpus> your code works on ARM Linux and FreeBSD x86_64
140 19:32 <vasild> michaelfolkson: no, I run clang's tools and then a script to hilight which lines in the coverage report have been modified by the patch
142 19:33 <michaelfolkson> Cool, thanks
143 19:33 <vasild> if some of travis or cirus can publish build products that can be integrated in CI
144 19:33 <jonatack> Q: When should the new sendaddrv2 message type, aka "send me addrv2", be sent?
145 19:34 <vasild> I mean - if during build we create coverage.html that can be browsed later
147 19:34 <brikk> i looked at adding a test for the padding=false case
148 19:34 <pinheadmz> makes it more like github side-by-side with the benefit of being able to scroll up and down past blobs of diff
149 19:35 <vasild> pinheadmz: and runs locally (I guess)! :)
150 19:35 <pinheadmz> yeah i always pull the branch
151 19:35 <jonatack> pinheadmz: i'm afraid you lost me with "more like GitHub"
152 19:35 <pinheadmz> mmm, in github "files changed" view, in side-by-side mode
153 19:36 <vasild> jonatack: that is a good question :)
154 19:36 <jonatack> vasild: which one, sendaddrv2 message?
155 19:36 <pinheadmz> thats how i like to review. but on github it only shows you the blbos that are different, i prefer to be able to scroll up and down the file (witihout clickthe expansion buttons)
156 19:36 <vasild> as early as possible :)
157 19:36 <vasild> yes, sendaddr
160 19:37 <jonatack> saying "sendaddrv2 SHOULD be sent after receiving the verack message from the peer" wasn't quite precise enough
161 19:38 <vasild> but we can't send it early enough to get the addrfrom in v2 format :(
162 19:38 <michaelfolkson> Yeah good comment
163 19:39 <jonatack> vasild: hmmmmm
164 19:39 <vasild> addrfrom which is part of the version message
165 19:39 <wumpus> jonatack: don't know why it's not precise enough; you can't quote say "before any other messages" because other BIPs might also introduce these kind of upgrade messages
166 19:40 <sipa> vasild: i think addrfrom is effectively unused?
167 19:40 <michaelfolkson> But Troy's point I think is to be clear on whether that means immediately after or at any point in time after
168 19:40 <wumpus> it's unused and always send as 127.0.0.1
169 19:40 <wumpus> only addrto is used
170 19:40 <wumpus> michaelfolkson: as I understood it, it can be any time after
171 19:41 <vasild> sipa: hm, right, it is just read from the stream and then discarded, even better!
172 19:41 <wumpus> michaelfolkson: there are no defined phases after VERACK
173 19:41 <michaelfolkson> Yup. So Troy wanted the comment to make that clear (that's my reading of it anyway)
174 19:42 <sipa> oh, but addrMe is used
175 19:42 <sipa> that's annoying
177 19:42 <troygiorshev> michaelfolkson: yup that's it!
178 19:42 <sipa> it's used to classify our local addresses
179 19:42 <wumpus> yes, that one is used
180 19:43 <jonatack> michaelfolkson: right. i just sort of wonder, "should", as opposed to what
181 19:43 <sipa> so... should BIP155 define a replacement?
182 19:43 <troygiorshev> I'm more looking to clarify (or specify) that there are no phases after VERACK.
183 19:43 <wumpus> though many implementations just send it as 127.0.0.1 no matter what
184 19:44 <wumpus> sipa: I thought about it but really didn't want to change the version message
185 19:44 <sipa> wumpus: i mean, the sendaddrv2 message or whatever could contain "oh btw, the address i used to connect to you is X (in addrv2 serialization)"
186 19:44 <wumpus> (and I'm happy about that because even changing the network version was super controversial)
187 19:45 <sipa> and the addrMe field in version would become a dummy
188 19:45 <wumpus> sipa: that would be possible yes
189 19:47 <wumpus> it definitely couldn't hurt
191 19:48 <vasild> there was some other proposal to add info to the sendaddrv2... what was it...
192 19:48 <wumpus> whether a node partakes in addr broadcasting at all
193 19:48 <sipa> heh the current BIP doesn't even include sendaddrv2?
194 19:48 <wumpus> gmaxwell's proposal
196 19:48 <wumpus> sipa: it's in vasild 's update PR
197 19:48 <jonatack> others: i think we are talking about PushNodeVersion() in net_processing.cpp
198 19:49 <wumpus> at some point we decided only the merge the updates to the BIP when the implementation was finalized
202 19:50 <wumpus> why are there more than one?
203 19:51 <jonatack> thanks, i had not seen PR 967
204 19:51 <wumpus> I mean, I think it's easier to have the proposed version of the BIP in one place
205 19:51 <sipa> you should ask luke-jr to merge those
206 19:51 <sipa> future changes can always be new PRs
207 19:51 <wumpus> well some other people claimed it would be better to merge only when things were sure/decided in the implementation
208 19:51 <sipa> i have been commenting on #907, not realizing it was a PR with a lot of changes in it already
209 19:52 <wumpus> don't have a strong opinion about it
211 19:52 <vasild> 907 started being a moving target, invalidating ACKs all the time, so I decided to not update it with the stuff from 967, also 907 contains mods which have been agreed on. While 967 has got zero discussion so far. I did not want to add some controversal stuff to the settled 907
212 19:52 <wumpus> but yes, having multiple PRs is really confusing here
213 19:52 <sipa> well, there is a BIP - it's confusing if there are multiple places where the currently-discussed version is held
214 19:52 <sipa> it has a number already; ideally that number means just one thing
215 19:52 <sipa> (or at least, one thing at any given point in time)
216 19:53 <wumpus> but having a lot fo small merges to the BIP wouldn't be great either
217 19:53 <michaelfolkson> Why does it matter?
218 19:53 <wumpus> it clutters the BIP repostiory
219 19:53 <wumpus> it's not meant for incremental development
220 19:54 <wumpus> you can have have the in-progress BIP somewhere else
221 19:54 <urethane> increases the mental cost of review and discussion
222 19:54 <sipa> vasild: fwiw, whether a BIP change gets merged is solely up to the author(s) and editor
223 19:54 <sipa> you don't need acks from anyone
224 19:54 <michaelfolkson> Hmm I'd have thought it was inevitable in cases such as this. Changes invalidate previous ACKs etc
226 19:54 <vasild> sipa: :-O
227 19:54 <jonatack> i've been reading the first one
228 19:55 <sipa> it's a proposal, you decide what the proposal is
229 19:55 <wumpus> it's nice to have ACKs from other people though
230 19:55 <sipa> wumpus: since you're the author, you should ack these changes ;)
232 19:56 <wumpus> I have (but not openly I guess)
233 19:56 <wumpus> I've always discussed with vasild about this
234 19:56 <sipa> of course
235 19:56 <michaelfolkson> Ok understood. So it is up to the author (vasild) to ask Luke to merge changes whenever vasild is happy with them
236 19:56 <sipa> michaelfolkson: wumpus, actually
237 19:56 <sipa> just saying - luke-jr's is probably just waiting for wumpus to ACK them
238 19:56 <vasild> I am the author of the PR, but not the BIP
239 19:56 <michaelfolkson> Ah yes sorry
240 19:57 <urethane> 3 minutes left :)
241 19:57 <wumpus> yes, I'll ACK them, np
242 19:58 <wumpus> as said, I had the impression that everything was in one PR, which tracked the new version of the BIP, which we'd merge when there was agreement on the implementation
243 19:58 <michaelfolkson> Did we cover all of Jon's questions?
244 19:58 <jonatack> PRs which also pick up work by dongcarl as well, and sipa for the crypto... it's a team effort
245 19:58 <wumpus> exactly because all kind of things come up during this
246 19:58 <vasild> sendaddrv2(your address is X, I promise to participate to gossip (but I may lie to you))
247 19:58 <jonatack> michaelfolkson: no, but the discussion was worth it
248 19:58 <michaelfolkson> Haha we did BAD
249 19:59 <jonatack> michaelfolkson: the variable-length question was a bit of a trick question
250 19:59 <jonatack> to check understanding
251 19:59 <jonatack> The addresses defined in BIP155 are of fixed size.
252 19:59 <jonatack> We don't actually handle any "variable-length" ones, just addresses of varying fixed lengths.
253 19:59 <jonatack> Why was the boolean pad parameter added to EncodeBase32()?
254 20:00 <jonatack> this seemed interesting
255 20:00 <jonatack> neither vasild nor i know
256 20:00 <jonatack> According to vasild: "that was ugly :/ but needed as some === signs appeared at the end of i2p addresses"
257 20:00 <brikk> thats how i see it too
258 20:00 <michaelfolkson> Hmm
259 20:00 <sipa> i probably implemented some standard base32 format, which includes the requirement to pad with = symbols
260 20:00 <brikk> but will there be side effects ?
261 20:00 <jonatack> for now, it's important to have the code recognize i2p addresses with the correct length and gossip them
262 20:00 <sipa> and I2P uses a slightly different standard that omits the padding
263 20:00 <jonatack> so that when we add full i2p support, assuming it happens later on,
264 20:00 <jonatack> then previous releases (beginning with the next one containing this code) can gossip i2p addresses
265 20:01 <brikk> i looked into a test for it to make sure it behaves, because there was none that I could find
266 20:01 <vasild> I guess the decode must be prepare to see non-multiple-of-8 input and pad it itself with = before decoding (once we start parsing i2p addresses)
267 20:01 <jonatack> brikk: great
268 20:01 <wumpus> to be clear, I2P support is not a goal for the initial PR
269 20:02 <jonatack> right, for later on
270 20:02 <jonatack> BHow do you think Bitcoin Core should make the transition from Tor v2 to v3? All at once, or v3 first opt-in, then default (and v2 opt-in or deprecated)?
271 20:02 <sipa> i don't think we should relay I2P addresses before there is support
272 20:02 <wumpus> we'd like torv3 support as soon as possible, I2P would be nice but can be done later if the gorundwork is done
273 20:02 <brikk> so is the pr doing a bit too much at this point?
274 20:02 <jonatack> Should we just bump to v3, or go thru first v3 opt-in, v2 default...
275 20:02 <wumpus> jonatack: first both, imo
276 20:03 <michaelfolkson> jonatack: So definitely not all at once. I'd say opt-in and then default
277 20:03 <sipa> brikk: the BIP defines I2P, so it makes sense to include the BIP-defined checks for it
278 20:03 <wumpus> jonatack: opt in is not necessary imo
279 20:03 <michaelfolkson> Oh
280 20:03 <sipa> that doesn't mean Bitcoin Core needs to relay them
281 20:03 <brikk> sipa: i see
282 20:03 <jonatack> #endmeeting
283 20:03 <wumpus> jonatack: there should be no reason why you'd not want v3 but want v2, but I can see a reason for keeping the v2 open for now, for older nodes
284 20:03 <urethane> I don't see the motivation to make it opt-in
285 20:03 <wumpus> sipa: right
286 20:04 <jonatack> wumpus: right
288 20:04 <michaelfolkson> So v2 would be deprecated at some point but a long time in future? Or never deprecated?
289 20:04 <@jnewbery> thanks jonatack! Great meeting. I didn't say much, but I found the discussion really interesting :)
290 20:04 <jonatack> we have 4 weeks to FF
291 20:04 <jonatack> thanks jnewbery!
292 20:04 <michaelfolkson> Thanks jonatack!
293 20:04 <wumpus> michaelfolkson: after Tor removes it from their codebase, we should too, I think
295 20:04 <troygiorshev> thanks jonatack!
296 20:05 <wumpus> thanks jonatack
297 20:05 <michaelfolkson> But people could be running old version of Tor?
298 20:05 <jonatack> please review the PR everyone :)
299 20:05 <emzy> Thanks jonatack!
300 20:05 <michaelfolkson> (even when Tor removes it)
301 20:05 <wumpus> michaelfolkson: uhm, Tor versions that don't support v3 are ancient by now, and actively dangerous to run
302 20:05 <michaelfolkson> Ok makes sense
303 20:05 <wumpus> they've supported it for a long time
304 20:06 <vasild> michaelfolkson: hmm, given that tor relays through some random tor nodes, I don't know what will happen if you run an old tor node that insists on doing v2 stuff and newer ones have dropped support for it
305 20:06 <sipa> torv3 is supported as of 0.3.2.9, released in january 2018
306 20:06 <sipa> that's not _that_ long ago
307 20:07 <wumpus> for Tor that's really long ago
308 20:07 <sipa> i guess :)
309 20:07 <wumpus> a lot happens there
310 20:07 <wumpus> it's not unlike Bitcoin in that regard
313 20:07 <jonatack> s/it's important to have the code recognize i2p addresses with the correct length and gossip them/just recognize them/
314 20:07 <sipa> vasild: i think so
315 20:08 <urethane> July 15th, 2021 0.4.6.x: Tor will no longer support v2 and support will be removed from the code base.
316 20:08 <urethane> October 15th, 2021 We will release new Tor client stable versions for all supported series that will disable v2.
317 20:08 <wumpus> in any case if you're still running those Tor versions by the time addrv2 is removed (and I suppose, the directories and such shut down), they won't be able to use hidden services at all I think
318 20:09 <sipa> i think the bigger question is when directory services etc will stop supporting v2
319 20:09 <vasild> sipa: my take on "gossip i2p in 0.21 is that, because only 22% run latest version, when we add i2p supprt in e.g. 0.22 then old nodes (0.21) will also gossip it
320 20:09 <wumpus> but I don't think bitcoind needs to support pre-addrv3 Tor
321 20:10 <sipa> vasild: i'm not sure that's a good idea, as nothing in the network can determine the existance of such addresses...
322 20:10 <vasild> sipa: worried about junk traffic?
323 20:10 <sipa> so little ways of purging garbage i2p someone may start rumouring
324 20:10 <sipa> more worried about garbage in addrmans
326 20:11 <sipa> i'd say first start gossipping it only between i2p peers, which actually have i2p connectivity
327 20:11 <vasild> same with torv2/torv3 addresses for a node that has no tor connectivity. Or IPv6 addresses for a node that has only IPv4 connectivity...
328 20:12 <sipa> once there is a tangible network, anything can start gossipping it, as it's useful to prevent partitions
329 20:12 <sipa> if there was a concrete plan to have i2p proxy support in the near future, maybe that could be different
330 20:14 <wumpus> i'd say first start gossipping it only between i2p peers, which actually have i2p connectivity <- agree
331 20:14 <jonatack> seems sensible
332 20:15 <sipa> for torv3 i think we expect a torv3-reachable set of bitcoind nodes to appear almost immediately
333 20:15 <wumpus> you could say so :)
334 20:16 <vasild> currently we gossip addresses from unreachable networks to just 1 peer (and addresses from reachable networks to 2 peers)
335 20:16 <sipa> vasild: 1.5!
336 20:16 <vasild> did it get merged?
339 20:17 <sipa> so i think that for "not known to exist" networks that number should probably be even lower, or 0
340 20:17 <vasild> so you propose to extend that logic to reachable: 2, unreachable-non-i2p 1.5, unreachable-i2p: 0
341 20:18 <sipa> same with cjdns
342 20:18 <vasild> unreachable-ipv4-ipv6-tor: 1.5
343 20:19 <vasild> reachable: 2, unreachable-ipv4-ipv6-torv2-torv3: 1.5, unreachable-anything-else: 0
344 20:22 <vasild> sipa: "more worried about garbage in addrmans" currently for a node to be able to protect itself from this it must have connectivity to all 3: ip4, ip6 and tor, right?
345 20:24 <sipa> assuming some reasonable number of honest nodes, it's not that black and white
346 20:24 <sipa> as ipv6-capable nodes will learn about useless ipv6 addresses being rumoured, and prune them
347 20:25 <sipa> hmm, maybe this isn't that big of a concern
348 20:25 <sipa> it just feels strange to me to permit addrman space to be taken up by addresses in a network we know doesn't ezist
349 20:26 <vasild> I will sleep over with this, maybe limit size in addrman also based on reachable/unreachable
350 20:36 <jonatack> michaelfolkson: thank you for adding to the bitcoinstackexchange answer! just saw that you mentioned it above
351 20:36 <sipa> he's been adding many :)