15:00:26 #startmeeting 15:00:26 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 Useful Commands: #action #agreed #help #info #idea #link #topic. 15:00:33 #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 hi 15:00:40 amiti fjahr jeremyrubin lightlike emilengler jonatack hebasto jb55 elichai2 15:00:42 hi 15:00:52 hi 15:01:01 hi 15:01:12 that ping thing is like a blockchain. It only gets longer, never shorter 15:01:18 hi 15:01:23 hi 15:01:34 hi 15:01:37 hi 15:01:44 hi 15:01:54 proposed topics are here: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/P2P-IRC-meetings#03-nov-2020 15:01:56 https://github.com/bitcoin/bitcoin/issues/03 | Encrypt wallet · Issue #3 · bitcoin/bitcoin · GitHub 15:02:11 Does anyone have any more topics that they want to propose now, or any general updates they want to give? 15:02:37 i just wanted to review beg #19858 15:02:42 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 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 I think we haven't branched 0.21 yet, so we're still in feature freeze, right? 15:03:46 right 15:03:50 branch is due pretty soon though? 15:03:52 i assume so -- but i think if we collect acks now it could merge shortly after? 15:03:59 hi 15:04:01 kinda curious about the status of p2p encryption 15:04:05 but jonas isn't here 15:04:19 noted, I'll reack it 15:04:31 #18242 was just rebased and it builds cleanly 15:04:33 yes, feature freeze is only about what is merged, not what is reviewed :) 15:04:35 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 "2020-11-01 split off 0.21" per 18947 15:04:53 (would be good to move it forward for 0.22) 15:04:54 wumpus: while you're here, what's your expectation for when 0.21 will be branched? 15:05:48 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 but I hope some time next week 15:06:33 thanks! 15:06:38 next general meeting we should go over the list 15:06:46 ok, onto the topics 15:06:57 #topic addrman file versioning 15:08:00 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 #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 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 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 vasild: I hadn't reviewed 19954 before 15:08:54 I started looking at the addrman code this weekend 15:09:09 actually in the future it will be possible to make forward-compatible changes, but the format version must not be incremented 15:09:13 forwards compatible? do you mean backwards compatible? 15:09:26 i'm confused why we can't write future code to parse whatever we need to 15:09:33 backward compatible 15:09:46 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 that really sounds like backwards compatible to me, but thanks for clarifying what you mean 15:10:08 I believe we generally want to allow people to downgrade at least one version, right? So people can back out upgrades 15:10:13 the current code supports maximum format=3 and if it sees format=4 it will refuse to parse it 15:10:28 hi 15:10:35 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 forwards compatible means that we're flexible with future peers.dat formatting 15:11:58 jnewbery: yes, i was reporting issues with that (upgrading, then downgrading) in 19954 iirc 15:11:59 vasild: right, so the current code on master is not forwards compatible with formats > 4 15:12:10 >= 4 15:12:23 sorry yes, >= 4 15:12:41 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 (as of the current master) 15:13:03 it would be annoying if users lost their addrman upgrading or downgrading 15:13:10 pr20284 changes that 15:13:52 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 :/ 15:15:28 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 maybe the filename should just be changed if that's not the case, in the future? 15:16:30 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 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 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 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 so the issue that does appear to be there is that downgrading after running 0.20 may be a problem 15:18:06 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 definitely don't have any alternative proposals 15:18:36 the point of the serialization format is that entries can appear in multiple new tables 15:18:44 luke-jr: right, filename change is another option 15:18:44 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 sdaftuar: that's a good suggestion too 15:19:39 jnewbery: hmm i need to read that more carefully then. 15:19:59 sdaftuar: that would definitely prevent being able to keep your addrman data over a downgrade 15:20:06 though it'd require updating a lot of documentation all over the place describing bitcoin core's files 15:20:21 everyone is used to "peers.dat" 15:20:38 how often does anyone mess with peers.dat directly? 15:20:56 I have no idea 15:21:06 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 yes, I slightly prefer this as well 15:21:51 i think it's okay if once in a while we have to give users annoying instructions for downgrading. 15:21:52 IMO it depends on compatibility 15:22:16 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 what would the annoying instructions be? 15:22:37 we could add a tool that either converts formats, or an RPC or something 15:22:38 ^ 15:22:41 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 ew 15:23:06 what did we do the last time we changed block file name conventions? 15:23:09 sdaftuar: no one is going to go through that much work for their peers table 15:23:28 sdaftuar: iirc hard linked 15:23:42 luke-jr: that doesn't sound backwards compatible? 15:23:46 why not? 15:24:07 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 oh we hardlinked new and old filenames together, i see 15:24:23 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 unless they don't use tor 15:24:42 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 assuming this really needs to go in for 0.21.0-otherwise there's no hurry 15:25:18 afaics, for forwards compatible changes, we just don't bump the version number? 15:25:58 argh, timezones 15:26:07 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 surely it parsed everything as garbage 15:26:56 it's definitely not theoretical 15:27:14 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 vasild: same 15:27:37 :-D 15:28:35 aj: I think there needs to be versioning to show that there are semantic changes, even if they are forwards compatible 15:28:39 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 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 luke-jr: that's a good point 15:30:17 So the action is to review 20284, unless anyone has anything important to add 15:31:07 #topic I2P support, some background at vasild/bitcoin/wiki/I2P-connectivity (vasild) 15:31:31 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 https://github.com/vasild/bitcoin/wiki/I2P-connectivity 15:32:03 jonatack: it probably takes longer for your tried tables to get populated 15:32:06 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 jonatack: that may be deceptive though; just because you have a lot of rumoured IP addresses, they aren't necessarily good 15:32:29 good points, thanks 15:32:57 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 aj: I think that's effectively what 20284 does 15:33:52 can we imagine any backward but not forward compatible change? 15:33:58 aj: 15:34:00 16:15 < vasild> pr20284 makes it possible to say in peers.dat "this is format=5, but anybody who can 15:34:04 read format=3 can also read this one" 15:34:44 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 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 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 sipa: backward compatible is easy: if version == 4: ... elif version == 5: ... 15:35:17 aj: +1 15:35:26 jnewbery: yes, i mean semantically 15:35:59 is there any change we can imagine to peers.dat that would remain readable by new versions? 15:36:17 sipa: readable by old code, you mean? 15:36:29 oh 15:36:38 * sdaftuar is super confused 15:36:39 do i have my forward/backward mixed up? 15:36:59 new code, old peers.dat == backwards compatible 15:37:05 sipa: i thought john has his forward/backward mixed up, for what it's worth. and maybe aj too. 15:37:05 old code, new peers.dat == forwards compatible 15:37:16 awkward 15:37:28 vasild: +1 15:37:31 "the format is ___ compatible" is how i'm interpreting it 15:37:47 the goal here is to make sure that new versions can keep reading old files? 15:38:00 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 no 15:38:13 where the input here is peers.dat 15:38:16 sipa: new code can always read old files 15:38:28 old peers.dat, new addrman is backwards compatibility 15:38:46 new peers.dat, old addrman is forwards compatibility 15:38:47 ok so this is about new files being readable by old code? can we imagine a change where that's useful? 15:38:58 sipa: it's about being able to downgrade 15:39:05 and not losing all your data 15:39:13 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 sdaftuar: yes, my question is: can we imagine any change to peers.dat where that's even feasible? 15:40:23 sipa: if you don't use Tor, this change is? 15:40:31 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 yes, for example the change from 0 to 1 and 1 to 2 both allowed old versions to read new files 15:41:05 jnewbery: ah, indeed! 15:41:18 thanks 15:41:26 sipa: you authored that! :-) 15:41:35 hmm, we could in theory even split a peers-torv3.dat for those, and keep peers.dat as-is and compatibel? 15:41:36 vasild: it's early 15:42:03 luke-jr: interesting idea 15:42:04 or put the torv3 addresses as an add-on at the end of the file or something? 15:42:30 where does that stop, though... would we have peers-i2p.dat as well? 15:42:35 if anyone following along is wondering, we're talking about enum class Format in addrman.h::L269 15:42:58 wumpus: why not? 15:43:09 this seems hard 15:43:10 is there a reason to throw it all in the same file? 15:43:10 jonatack: well more generally about the Serialize and Unserialize methods for CAddrMan 15:43:12 wumpus: perhaps eventually files get merged, once old software no longer is a plausible downgrade target? 15:43:34 peers.dat files aren't just lists of addresses 15:43:47 they dump the tables and their layout too 15:43:47 we could replace peers.dat with sqlite :) 15:43:53 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 then if people downgrade they can restore the backup and *shrug* 15:44:07 if you split the file in two, you can't just merge them back without losing informatiom 15:44:10 jnewbery: that might not be a bad idea 15:44:24 wumpus: yes i agree with that type of suggestion too, along the lines of "sometimes it can be annoying to downgrade" 15:44:38 wumpus: how is that different from just using a new filename, except requiring an extra step to downgrade? 15:45:00 luke-jr: it keeps the filename the same for the bulk of the users 15:45:23 the non-downgrading path is likely be far most popular, or at least one'd hope :) 15:45:50 in any case, we needa solution here fairly quickly 15:46:21 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 *upgrading is the more likely path than downgrading 15:46:55 ok, we're running out of time and there are a couple more topics, so perhaps we should move on 15:46:56 I'm not sure last-minute changes lke 'sort peers.dat per network' or other complete changes to handling peers database 15:47:17 wumpus: agree 15:47:29 vasild: did you have anything you wanted to add about I2P support? 15:47:49 jnewbery: I'm fine to report mine next time 15:48:20 ariard: thanks. Maybe a good idea so people have some time to think about it before the meeting 15:48:25 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 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 jnewbery: yes, agree, it's kind of nasty that this issue comes up so last minute 15:49:38 sdaftuar: i2p/cjdns do not get rumoured by the current code, at all 15:50:14 sipa: no, they would get rumored if the node has connectivity to i2p 15:50:26 or am I mistaken... 15:50:28 cjdns not because it uses IPv6 addresses in a range we consider local / unroutable? 15:50:34 vasild: which isn't possible.on the current code :) 15:50:40 see #20119 15:50:41 https://github.com/bitcoin/bitcoin/issues/20119 | BIP155 follow-ups by sipa · Pull Request #20119 · bitcoin/bitcoin · GitHub 15:50:54 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 and if we don't, it still goes to 1 i think? is that correct? 15:51:30 sdaftuar: there are classes now 15:51:34 sdaftuar: yes 15:51:34 sdaftuar: 1.5 now I think 15:51:50 1) reachable networks get rumoured 2x 15:51:55 sdaftuar: see RelayAddress() in the case fReachable==true 15:52:09 2) unreachable ipv4/ipv6/torv2/torv3 get rumoured 1.5x 15:52:18 see #19728 15:52:20 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 3) unreachable others do not get rumoured at all 15:52:43 #20254 makes i2p reachable 15:52:45 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 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 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 vasild: same for Tor? :p 15:55:31 luke-jr: no, in Tor we don't see the address of the peer that connected to us 15:55:47 oh, right 15:55:59 "tor has no from address" 15:56:03 XD 15:56:08 heh 15:56:12 so in I2P we have P2P encryption by default :-) 15:56:25 sdaftuar: that logic is in AdvertiseLocal() in net.cpp 15:56:35 vasild: with public identities, though :( 15:56:48 yes 15:57:04 kinda by design in i2p? 15:57:16 yes 15:57:39 sdaftuar: among the local addresses compatible with that peer, the one that has received the most mentions in incomimg version messages 15:58:07 which reminds me: we can't put torv3/i2p/cjdns in version messages, so those will never receive any mentions 15:58:09 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 sipa: oh taht seems important 15:58:59 do we need a new version version? :) 15:59:14 jnewbery: i think we can just add data on to the end and no one will mind 15:59:55 i'm not sure this is useful for those networks 16:00:08 as there isn't any useful compatibility matrix for them 16:00:15 that's time! 16:00:18 #endmeeting