15:00:26 <jnewbery> #startmeeting
15:00:26 <lightningbot> Meeting started Tue Nov  3 15:00:26 2020 UTC.  The chair is jnewbery. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:00:26 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
15:00:33 <jnewbery> #bitcoin-core-dev P2P Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator aj Chris_Stewart_5 dongcarl gwillen jamesob ken281221 ryanofsky gleb moneyball kvaciral ariard digi_james
15:00:36 <vasild> hi
15:00:40 <jnewbery> amiti fjahr jeremyrubin lightlike emilengler jonatack hebasto jb55 elichai2
15:00:42 <phantomcircuit> hi
15:00:52 <emzy> hi
15:01:01 <aj> hi
15:01:12 <jnewbery> that ping thing is like a blockchain. It only gets longer, never shorter
15:01:18 <ariard> hi
15:01:23 <lightlike> hi
15:01:34 <wumpus> hi
15:01:37 <jonatack> hi
15:01:44 <kanzure> hi
15:01:54 <jnewbery> proposed topics are here: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/P2P-IRC-meetings#03-nov-2020
15:01:56 <gribble> https://github.com/bitcoin/bitcoin/issues/03 | Encrypt wallet · Issue #3 · bitcoin/bitcoin · GitHub
15:02:11 <jnewbery> Does anyone have any more topics that they want to propose now, or any general updates they want to give?
15:02:37 <sdaftuar> i just wanted to review beg #19858
15:02:42 <gribble> https://github.com/bitcoin/bitcoin/issues/19858 | Periodically make block-relay connections and sync headers by sdaftuar · Pull Request #19858 · bitcoin/bitcoin · GitHub
15:03:00 <sdaftuar> i've started to now page that PR out of my memory, so would like to make progress before i totally forget what ti is :)
15:03:35 <jnewbery> I think we haven't branched 0.21 yet, so we're still in feature freeze, right?
15:03:46 <wumpus> right
15:03:50 <aj> branch is due pretty soon though?
15:03:52 <sdaftuar> i assume so -- but i think if we collect acks now it could merge shortly after?
15:03:59 <amiti> hi
15:04:01 <luke-jr> kinda curious about the status of p2p encryption
15:04:05 <luke-jr> but jonas isn't here
15:04:19 <ariard> noted, I'll reack it
15:04:31 <jonatack> #18242 was just rebased and it builds cleanly
15:04:33 <wumpus> yes, feature freeze is only about what is merged, not what is reviewed :)
15:04:35 <gribble> https://github.com/bitcoin/bitcoin/issues/18242 | Add BIP324 encrypted p2p transport de-/serializer (only used in tests) by jonasschnelli · Pull Request #18242 · bitcoin/bitcoin · GitHub
15:04:45 <aj> "2020-11-01 split off 0.21" per 18947
15:04:53 <jonatack> (would be good to move it forward for 0.22)
15:04:54 <jnewbery> wumpus: while you're here, what's your expectation for when 0.21 will be branched?
15:05:48 <wumpus> there's still quite a list of PRs tagged with 0.21.0, these either need to be merged or removed from the milestone first https://github.com/bitcoin/bitcoin/pulls?q=is%3Aopen+is%3Apr+milestone%3A0.21.0
15:06:13 <wumpus> but I hope some time next week
15:06:33 <jnewbery> thanks!
15:06:38 <wumpus> next general meeting we should go over the list
15:06:46 <jnewbery> ok, onto the topics
15:06:57 <jnewbery> #topic addrman file versioning
15:08:00 <vasild> jnewbery: how did you suddenly come to the realization that old versions are not guaranteed to fail the parsing, out of the blue?
15:08:08 <jnewbery> #19954 meant that in future it won't be possible to do forwards-compatible changes to the peers.dat format. #20284 is suggested as a way to allow that again.
15:08:13 <gribble> https://github.com/bitcoin/bitcoin/issues/19954 | Complete the BIP155 implementation and upgrade to TORv3 by vasild · Pull Request #19954 · bitcoin/bitcoin · GitHub
15:08:14 <gribble> https://github.com/bitcoin/bitcoin/issues/20284 | addrman: ensure old versions dont parse peers.dat by vasild · Pull Request #20284 · bitcoin/bitcoin · GitHub
15:08:34 <jnewbery> vasild: I hadn't reviewed 19954 before
15:08:54 <jnewbery> I started looking at the addrman code this weekend
15:09:09 <vasild> actually in the future it will be possible to make forward-compatible changes, but the format version must not be incremented
15:09:13 <sdaftuar> forwards compatible? do you mean backwards compatible?
15:09:26 <sdaftuar> i'm confused why we can't write future code to parse whatever we need to
15:09:33 <vasild> backward compatible
15:09:46 <jnewbery> sdaftuar: I mean forwards compatible, i.e. you can upgrade, make changes to peers.dat and then downgrade and still parse peers.dat
15:10:03 <sdaftuar> that really sounds like backwards compatible to me, but thanks for clarifying what you mean
15:10:08 <jnewbery> I believe we generally want to allow people to downgrade at least one version, right? So people can back out upgrades
15:10:13 <vasild> the current code supports maximum format=3 and if it sees format=4 it will refuse to parse it
15:10:28 <hebasto> hi
15:10:35 <jnewbery> I think backwards compatible would mean that we're able to read old versions of peers.dat, which obviously we can always guarantee
15:10:52 <jnewbery> forwards compatible means that we're flexible with future peers.dat formatting
15:11:58 <jonatack> jnewbery: yes, i was reporting issues with that (upgrading, then downgrading) in 19954 iirc
15:11:59 <jnewbery> vasild: right, so the current code on master is not forwards compatible with formats > 4
15:12:10 <vasild> >= 4
15:12:23 <jnewbery> sorry yes, >= 4
15:12:41 <vasild> yes, if changes are made to the format it must stay at format=3 if we want 0.21 to parse it
15:12:55 <vasild> (as of the current master)
15:13:03 <luke-jr> it would be annoying if users lost their addrman upgrading or downgrading
15:13:10 <vasild> pr20284 changes that
15:13:52 <jnewbery> luke-jr: they already lost part of their addrman upgrading to 0.20 because of https://github.com/bitcoin/bitcoin/pull/16702#discussion_r515608294, but I guess no-one noticed
15:13:57 <luke-jr> :/
15:15:28 <vasild> pr20284 makes it possible to say in peers.dat "this is format=5, but anybody who can read format=3 can also read this one"
15:16:29 <luke-jr> maybe the filename should just be changed if that's not the case, in the future?
15:16:30 <jnewbery> vasild: yes, that's the functionality we want before the 0.21 release. Just want to make sure people are happy with repurposing nKeySize for versioning.
15:17:03 <sdaftuar> jnewbery: sorry if this is a digression, but i might be misunderstanding the issue in #16702 -- it looks like the version check there just causes potential rebucketing, not losing addresses?
15:17:07 <gribble> https://github.com/bitcoin/bitcoin/issues/16702 | p2p: supplying and using asmap to improve IP bucketing in addrman by naumenkogs · Pull Request #16702 · bitcoin/bitcoin · GitHub
15:17:42 <wumpus> I think it's a hack but also a clever way of doing so, and he also introduces a real versioning mechanism for next time, so it's only for once
15:17:48 <sdaftuar> so the issue that does appear to be there is that downgrading after running 0.20 may be a problem
15:18:06 <jnewbery> sdaftuar: it means that entries in new tables can only appear in one new table (or even zero if there's a collision)
15:18:17 <wumpus> definitely don't have any alternative proposals
15:18:36 <jnewbery> the point of the serialization format is that entries can appear in multiple new tables
15:18:44 <vasild> luke-jr: right, filename change is another option
15:18:44 <sdaftuar> wumpus: we could rename the peers.dat file for 0.21, and migrate data from the old file to the new one?
15:19:39 <wumpus> sdaftuar: that's a good suggestion too
15:19:39 <sdaftuar> jnewbery: hmm i need to read that more carefully then.
15:19:59 <jnewbery> sdaftuar: that would definitely prevent being able to keep your addrman data over a downgrade
15:20:06 <wumpus> though it'd require updating a lot of documentation all over the place describing bitcoin core's files
15:20:21 <wumpus> everyone is used to "peers.dat"
15:20:38 <luke-jr> how often does anyone mess with peers.dat directly?
15:20:56 <wumpus> I have no idea
15:21:06 <vasild> btw, I did consider the file name change, but preferred the versioning inside the file, instead of using the filesystem for versioning
15:21:28 <wumpus> yes, I slightly prefer this as well
15:21:51 <sdaftuar> i think it's okay if once in a while we have to give users annoying instructions for downgrading.
15:21:52 <luke-jr> IMO it depends on compatibility
15:22:16 <luke-jr> if downgrading doesn't lose the address book, same filename is good; otherwise, a new name avoids losing the old addrman for the old ver
15:22:35 <jnewbery> what would the annoying instructions be?
15:22:37 <sdaftuar> we could add a tool that either converts formats, or an RPC or something
15:22:38 <luke-jr> ^
15:22:41 <wumpus> sdaftuar: as I understand older versions will just ignore the incompatible peers.dat, and create a new empty one. But yes I agree. Downgrading is quite a rare event anyhow.
15:22:42 <luke-jr> ew
15:23:06 <sdaftuar> what did we do the last time we changed block file name conventions?
15:23:09 <wumpus> sdaftuar: no one is going to go through that much work for their peers table
15:23:28 <luke-jr> sdaftuar: iirc hard linked
15:23:42 <sdaftuar> luke-jr: that doesn't sound backwards compatible?
15:23:46 <luke-jr> why not?
15:24:07 <jonatack> I must have lost my peers.dat a couple dozen times while testing the addrv2 PRs. within the next year, anyone still using tor v2 will be forced to upgrade to 0.21 anyway.
15:24:11 <sdaftuar> oh we hardlinked new and old filenames together, i see
15:24:23 <wumpus> I'm not sure it makes sense to extensively describe alternatives here, we have vasild's work, unless someone really strongly disagrees with it let's review it and get it merged asap
15:24:26 <jonatack> unless they don't use tor
15:24:42 <jnewbery> I don't think there's any need to change file names. It should be perfectly possible to have internal versioning for changes that are forwards compatible and not forwards compatible. I think that's what 20284 does, but I haven't reviewed it yet.
15:25:06 <wumpus> assuming this really needs to go in for 0.21.0-otherwise there's no hurry
15:25:18 <aj> afaics, for forwards compatible changes, we just don't bump the version number?
15:25:58 <sipa> argh, timezones
15:26:07 <vasild> I started a 0.20.1 node on my new peers.dat and it parsed it without emitting errors, so it is not just "theoretical"
15:26:26 <vasild> surely it parsed everything as garbage
15:26:56 <wumpus> it's definitely not theoretical
15:27:14 <vasild> I don't know why during testing of that change, some weeks ago I always got some read/parse error, which fooled me that old versions fail to parse always
15:27:30 <jonatack> vasild: same
15:27:37 <vasild> :-D
15:28:35 <jnewbery> aj: I think there needs to be versioning to show that there are semantic changes, even if they are forwards compatible
15:28:39 <wumpus> garbage addresses would be kind of nasty, especially if they might also be gossiped and thus pollute the rest of the network too
15:28:55 <luke-jr> jnewbery: point being if it isn't forwards compatible, the rename keeps the old file for the old version if you downgrade
15:29:43 <jnewbery> luke-jr: that's a good point
15:30:17 <jnewbery> So the action is to review 20284, unless anyone has anything important to add
15:31:07 <jnewbery> #topic I2P support, some background at vasild/bitcoin/wiki/I2P-connectivity (vasild)
15:31:31 <jonatack> When you lose your peers.dat, a new one doesn't seem to take very long to get up to a decent size (40-50k peers)
15:31:47 <vasild> https://github.com/vasild/bitcoin/wiki/I2P-connectivity
15:32:03 <jnewbery> jonatack: it probably takes longer for your tried tables to get populated
15:32:06 <sdaftuar> jonatack: the whole network losing their peers.dat in the event of downgarde might be a scary idea though (imagine a bug in 0.21, say)
15:32:21 <sipa> jonatack: that may be deceptive though; just because you have a lot of rumoured IP addresses, they aren't necessarily good
15:32:29 <jonatack> good points, thanks
15:32:57 <aj> jnewbery: if you bump the version from 5 to 6 (eg), and 6 is "unknown" it will be treated as non-supported, so won't be forward compatible. you need major/minor versions if you want to do that
15:33:29 <jnewbery> aj: I think that's effectively what 20284 does
15:33:52 <sipa> can we imagine any backward but not forward compatible change?
15:33:58 <vasild> aj:
15:34:00 <vasild> 16:15 < vasild> pr20284 makes it possible to say in peers.dat "this is format=5, but anybody who can
15:34:04 <vasild> read format=3 can also read this one"
15:34:44 <aj> vasild, jnewbery: i believe you're both mistaken... it just supports a range: "this code supports versions 3..5". introducing version 6 won't make old code support it
15:34:54 <jnewbery> sipa: after #19954, all changes are not forward compatible (because 0.21 onwards will reject any version greater than the one they know)
15:34:59 <gribble> https://github.com/bitcoin/bitcoin/issues/19954 | Complete the BIP155 implementation and upgrade to TORv3 by vasild · Pull Request #19954 · bitcoin/bitcoin · GitHub
15:35:08 <aj> sipa: backward compatible is easy: if version == 4: ... elif version == 5: ...
15:35:17 <jnewbery> aj: +1
15:35:26 <sipa> jnewbery: yes, i mean semantically
15:35:59 <sipa> is there any change we can imagine to peers.dat that would remain readable by new versions?
15:36:17 <aj> sipa: readable by old code, you mean?
15:36:29 <sipa> oh
15:36:38 * sdaftuar is super confused
15:36:39 <sipa> do i have my forward/backward mixed up?
15:36:59 <aj> new code, old peers.dat == backwards compatible
15:37:05 <sdaftuar> sipa: i thought john has his forward/backward mixed up, for what it's worth.  and maybe aj too.
15:37:05 <aj> old code, new peers.dat == forwards compatible
15:37:16 <vasild> awkward
15:37:28 <sdaftuar> vasild: +1
15:37:31 <aj> "the format is ___ compatible" is how i'm interpreting it
15:37:47 <sipa> the goal here is to make sure that new versions can keep reading old files?
15:38:00 <jnewbery> sdaftuar: wikipedia seems to agree with our definitions: Backward compatibility (sometimes known as backwards compatibility) is a property of a system, product, or technology that allows for interoperability with an older legacy system, or with input designed for such a system
15:38:02 <vasild> no
15:38:13 <jnewbery> where the input here is peers.dat
15:38:16 <vasild> sipa: new code can always read old files
15:38:28 <jnewbery> old peers.dat, new addrman is backwards compatibility
15:38:46 <jnewbery> new peers.dat, old addrman is forwards compatibility
15:38:47 <sipa> ok so this is about new files being readable by old code? can we imagine a change where that's useful?
15:38:58 <sdaftuar> sipa: it's about being able to downgrade
15:39:05 <sdaftuar> and not losing all your data
15:39:13 <jnewbery> i.e. you right the code in a way that it can handle future changes to format that you don't know about, which is potentially important for downgrades
15:40:00 <sipa> sdaftuar: yes, my question is: can we imagine any change to peers.dat where that's even feasible?
15:40:23 <luke-jr> sipa: if you don't use Tor, this change is?
15:40:31 <sdaftuar> sipa: well it depends on what's on the table? eg if we wrote out the old format for the addresses that are supported by the old format in a file that is readable by old software, then sure
15:40:33 <jnewbery> yes, for example the change from 0 to 1 and 1 to 2 both allowed old versions to read new files
15:41:05 <sipa> jnewbery: ah, indeed!
15:41:18 <sipa> thanks
15:41:26 <vasild> sipa: you authored that! :-)
15:41:35 <luke-jr> hmm, we could in theory even split a peers-torv3.dat for those, and keep peers.dat as-is and compatibel?
15:41:36 <sipa> vasild: it's early
15:42:03 <sdaftuar> luke-jr: interesting idea
15:42:04 <aj> or put the torv3 addresses as an add-on at the end of the file or something?
15:42:30 <wumpus> where does that stop, though... would we have peers-i2p.dat as well?
15:42:35 <jonatack> if anyone following along is wondering, we're talking about enum class Format in addrman.h::L269
15:42:58 <luke-jr> wumpus: why not?
15:43:09 <sipa> this seems hard
15:43:10 <luke-jr> is there a reason to throw it all in the same file?
15:43:10 <jnewbery> jonatack: well more generally about the Serialize and Unserialize methods for CAddrMan
15:43:12 <sdaftuar> wumpus: perhaps eventually files get merged, once old software no longer is a plausible downgrade target?
15:43:34 <sipa> peers.dat files aren't just lists of addresses
15:43:47 <sipa> they dump the tables and their layout too
15:43:47 <jnewbery> we could replace peers.dat with sqlite :)
15:43:53 <wumpus> I'm really not sure this is worth so much worrying about, if we're really worried about people downgrading why not make a backup at first run with 0.21.0?
15:44:05 <wumpus> then if people downgrade they can restore the backup and *shrug*
15:44:07 <sipa> if you split the file in two, you can't just merge them back without losing informatiom
15:44:10 <luke-jr> jnewbery: that might not be a bad idea
15:44:24 <sdaftuar> wumpus: yes i agree with that type of suggestion too, along the lines of "sometimes it can be annoying to downgrade"
15:44:38 <luke-jr> wumpus: how is that different from just using a new filename, except requiring an extra step to downgrade?
15:45:00 <wumpus> luke-jr: it keeps the filename the same for the bulk of the users
15:45:23 <wumpus> the non-downgrading path is likely be far most popular, or at least one'd hope :)
15:45:50 <wumpus> in any case, we needa solution here fairly quickly
15:46:21 <jnewbery> wumpus: I agree that it's more likely, but I think we should strive to make downgrades across a single version seamless, just in case we ship a bad bug and need people to roll back
15:46:36 <jnewbery> *upgrading is the more likely path than downgrading
15:46:55 <jnewbery> ok, we're running out of time and there are a couple more topics, so perhaps we should move on
15:46:56 <wumpus> I'm not sure last-minute changes lke 'sort peers.dat per network' or other complete changes to handling peers database
15:47:17 <sipa> wumpus: agree
15:47:29 <jnewbery> vasild: did you have anything you wanted to add about I2P support?
15:47:49 <ariard> jnewbery: I'm fine to report mine next time
15:48:20 <jnewbery> ariard: thanks. Maybe a good idea so people have some time to think about it before the meeting
15:48:25 <vasild> Anybody interested in I2P support - see https://github.com/vasild/bitcoin/wiki/I2P-connectivity and discuss here, I guess after the meeting or anytime
15:48:55 <sdaftuar> i've got a question about i2p (applies to any new exotic network, even tor stuff that we're doing) - what is our undestanding for how these addresses get gossiped to peers that support the network type?
15:48:59 <wumpus> jnewbery: yes, agree, it's kind of nasty that this issue comes up so last minute
15:49:38 <sipa> sdaftuar: i2p/cjdns do not get rumoured by the current code, at all
15:50:14 <vasild> sipa: no, they would get rumored if the node has connectivity to i2p
15:50:26 <vasild> or am I mistaken...
15:50:28 <wumpus> cjdns not because it uses IPv6 addresses in a range we consider local / unroutable?
15:50:34 <sipa> vasild: which isn't possible.on the current code :)
15:50:40 <sipa> see #20119
15:50:41 <gribble> https://github.com/bitcoin/bitcoin/issues/20119 | BIP155 follow-ups by sipa · Pull Request #20119 · bitcoin/bitcoin · GitHub
15:50:54 <sdaftuar> so the basic idea is that if we know how to reach a network, we'll rumor it to 2 peers or so
15:51:18 <sdaftuar> and if we don't, it still goes to 1 i think? is that correct?
15:51:30 <sipa> sdaftuar: there are classes now
15:51:34 <vasild> sdaftuar: yes
15:51:34 <jnewbery> sdaftuar: 1.5 now I think
15:51:50 <sipa> 1) reachable networks get rumoured 2x
15:51:55 <vasild> sdaftuar: see RelayAddress() in the case fReachable==true
15:52:09 <sipa> 2) unreachable ipv4/ipv6/torv2/torv3 get rumoured 1.5x
15:52:18 <jonatack> see #19728
15:52:20 <gribble> https://github.com/bitcoin/bitcoin/issues/19728 | Increase the ip address relay branching factor for unreachable networks by sipa · Pull Request #19728 · bitcoin/bitcoin · GitHub
15:52:24 <sipa> 3) unreachable others do not get rumoured at all
15:52:43 <vasild> #20254 makes i2p reachable
15:52:45 <gribble> https://github.com/bitcoin/bitcoin/issues/20254 | Add I2P support using statically configured destinations by vasild · Pull Request #20254 · bitcoin/bitcoin · GitHub
15:54:41 <vasild> btw, in I2P we can see the peer's address and be sure data comes from him (the address is a hash of his public key)
15:54:43 <sdaftuar> sipa: thanks. one more question, how do we decide what address to advertise ourselves as, if we're reachable in multiple ways?
15:54:57 <luke-jr> vasild: same for Tor? :p
15:55:31 <vasild> luke-jr: no, in Tor we don't see the address of the peer that connected to us
15:55:47 <luke-jr> oh, right
15:55:59 <sipa> "tor has no from address"
15:56:03 <luke-jr> XD
15:56:08 <wumpus> heh
15:56:12 <vasild> so in I2P we have P2P encryption by default :-)
15:56:25 <jnewbery> sdaftuar: that logic is in AdvertiseLocal() in net.cpp
15:56:35 <sipa> vasild: with public identities, though :(
15:56:48 <vasild> yes
15:57:04 <luke-jr> kinda by design in i2p?
15:57:16 <vasild> yes
15:57:39 <sipa> sdaftuar: among the local addresses compatible with that peer, the one that has received the most mentions in incomimg version messages
15:58:07 <sipa> which reminds me: we can't put torv3/i2p/cjdns in version messages, so those will never receive any mentions
15:58:09 <sdaftuar> sipa: it seems like for something like adding a new newtork type, sometimes we'll want to advertise our i2p address instead right?
15:58:40 <sdaftuar> sipa: oh taht seems important
15:58:59 <jnewbery> do we need a new version version? :)
15:59:14 <sdaftuar> jnewbery: i think we can just add data on to the end and no one will mind
15:59:55 <sipa> i'm not sure this is useful for those networks
16:00:08 <sipa> as there isn't any useful compatibility matrix for them
16:00:15 <jnewbery> that's time!
16:00:18 <jnewbery> #endmeeting