15:00:45 #startmeeting 15:00:45 Meeting started Tue Oct 20 15:00:45 2020 UTC. The chair is jnewbery. Information about MeetBot at http://wiki.debian.org/MeetBot. 15:00:45 Useful Commands: #action #agreed #help #info #idea #link #topic. 15:00:54 #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:01:00 amiti fjahr jeremyrubin lightlike emilengler jonatack hebasto jb55 elichai2 15:01:02 hi 15:01:03 hi folks! 15:01:08 Hi 15:01:09 hi 15:01:09 yeah -noXXX for a list setting clears the list. hopefully you never have to rely on this! 15:01:10 hi 15:01:11 hi 15:01:12 hi 15:01:22 hallo 15:01:28 heya 15:01:31 hi 15:01:50 first of all, congrats everyone for getting so many PRs reviewed and merged before feature freeze! 15:01:53 hi 15:02:16 we got addrv2, transaction request overhaul, taproot and anchor connections all merged 15:02:37 Just one proposed topic today: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/P2P-IRC-meetings#20-oct-2020 15:02:40 https://github.com/bitcoin/bitcoin/issues/20 | JSON-RPC callback · Issue #20 · bitcoin/bitcoin · GitHub 15:02:54 Before we do that, does anyone have any updates or want to share what they're working on/prioritising? 15:03:45 ok, our single topic is: 15:03:46 Remove timestamps from addr messages? It seems like the timestamp is only used to leak information about our recent connectivity. It doesn't look like we use it to make decisions about who to connect to. (sdaftuar/jnewbery) 15:03:57 sdaftuar: do you want to explain? 15:04:03 oy, i can try 15:04:40 i guess the background here is around looking at how addrman works, and what information it might leak about our peers (and whether or not that is ok, i guess) 15:04:51 hi 15:05:03 i was chatting with jnewbery about the interaction specifically with block-relay-only peers, where we really don't want to leak anything 15:05:40 and one observation was that right now in master, we basically directly leak the time at which we were connected to a block-relay-only peer after we disconnect from that peer and then include the address in a getaddr response 15:05:48 It does indeed leak info, but I never thought about solving the issue in this fashion... 15:05:59 we can fix that, but it led to wondering: what good does this time do anyone, anyway? 15:06:24 is it actually not used for anything? 15:06:32 our own software seems to barely use those times: we use it to sometimes filter out responses to getaddr requests, and we use to sometimes to evict things from the new table 15:06:33 There is a check in IsTerrible 15:06:39 Seeing if it’s older than a month 15:06:46 the fact it's not used by core addrman doesn't mean it's not used by some other bitcoin clients to decide its peering 15:06:50 but we do not use it for determining who to connect to, as far as i can tell. 15:08:44 From what I remember I think you are right, but those existing features are not nothing 15:08:44 using timestamps is already a well-known way of infering network topology: https://www.cs.umd.edu/projects/coinscope/coinscope.pdf 15:08:46 ariard: agreed. but i thought it might be worth polling people about how much good these timestamps can do, as they are necessarily unverifiable data? 15:08:56 CAddrMan::Good: "nTime is not updated here, to avoid leaking information about currently-connected peers." 15:09:16 sdaftuar: good question 15:09:48 gleb: i imagine we could replace their use in those two places without that much trouble 15:09:52 I think the way we use them, an adversary can’t really manipulate, because the checks are very moderate 15:10:05 eg by using nlasttry/nlastsuccess 15:10:29 even if someone bumps their own addrs too much, they don’t really become “better” 15:10:43 sdaftuar: and that won’t propagate through the network? 15:10:54 it will have effect only locally at every node 15:11:02 gleb: given that we don't use them for much, it seems there is only downside to us by potentially telling our peers who we were connected to and at what time? 15:12:16 Telling that a given address we’re relaying is not one year old... 15:12:36 we could still use nlastsuccess to filter out old addresses from our getaddr responses, i think? 15:12:39 sdaftuar: right, unverifiable data doesn't mean it can be useful even if it's gentleman-style of enforcment 15:12:59 a lot of alternatives p2p stack doesn't sanitize their addrs with a feeler connection 15:13:17 and they can retell this fact to other peers without connecting by themselves. Just tell what we told 15:14:03 interestingly, we don't update that time field when we successfully connect to a peer via a feeler connection, i believe. 15:14:09 I need to look better at the code. I’d be very happy to get rid of this stuff 15:14:27 i'll also have a look in more detail 15:14:30 ariard: tradeoff there seems to be between helping a [theoretical] alternative implementation make better decisions about who to connect to -vs- protecting our own privacy 15:14:33 it's very appealing 15:15:02 by default we should always lean towards protecting our own privacy, unless doing so would be detrimental to the network as a whole 15:15:23 jnewbery: I would favor protecting our own privacy, but as a good practice asking on the ml would be great, at least to warrant 15:15:38 jnewbery: I suggest ariard thinks more whether time stamps can help light clients a lot comparing to feeler strategy 15:16:16 +1 I don't have a complete understanding of these timestamps, but when I've looked at them in the past I've come to similar conclusions where they aren't used for much since they are unreliable and are easy to accidentally leak information 15:16:17 ariard: +1. This would be a de facto change to the p2p protocol. Circulating it on the mailing list would be good manners, at least 15:17:03 They are unreliable, but they also can’t be exploited actively I think (only leak info, passive exploit) 15:17:39 gleb: whether they can be exploited depends on how people are using them. i agree our software seems to be designed so that this is just an information leak 15:17:40 I think another piece of (almost) useless data that we could stop sharing is the start_height in the version message, but that's maybe a different discussion 15:18:04 yeah, exactly, they can't be exploited because we write logic to not rely on it 15:18:06 Even assuming they're used by some lightclient p2p stack, inviting ecosystem-wise not relying on them due to their distrusted nature would be better 15:18:44 we can stop using the nTime field without caring about what others do... if we'd start setting them differently (all zero? just the current time? random within some window?) we may want to seek opinions on the ml 15:19:12 agreed 15:19:23 i imagine we'd do those at different points in time anyway 15:20:03 Sipa: but Most of their use is already about setting them differently, it seems we mainly discussing dropping that :) 15:20:33 or, well, using them to filter out responses I guess. We sort of “promised” to use them? 15:20:43 whatever, i think we’re on the same page 15:20:49 just set them to zero, in case of randomness source breakup that's not a fingerprint for your node 15:21:43 Setting them to 0 would break compatibility 15:21:46 ariard: if our RNG has issues, we have bigger problems 15:22:03 old nodes think that ntime < 10000000 is trash iirc 15:22:12 ariard: and setting them to 0 would actively hurt relay chances on current code 15:22:48 we probably should randomize them within a week window from now or so 15:22:59 good to know, do we have other compatibility bounds to care about beyond ntime < 10000000 ? 15:23:07 gleb: if time is < 100000000, we set it to some recent time: https://github.com/bitcoin/bitcoin/blob/f5bd46a4cc6d395ce71ecb99852c1774235a249c/src/net_processing.cpp#L2573-L2574 15:24:00 Oh right, sorry, I’m on my phone 15:24:09 maybe we just set it to MAX_UINT32 15:24:33 i have another related topic to mention while we're discussing addrman-- i opened a PR to fix some interactions between addrman and block-relay-only peers. it's a reversal from the direction i was leaning before about how this should work, so wanted to mention it in case anyone wanted to discuss 15:25:05 cc amiti and jnewbery as this came up during a recent PR under review 15:26:06 #20187 15:26:08 https://github.com/bitcoin/bitcoin/issues/20187 | Addrman: test-before-evict bugfix and improvements for block-relay-only peers by sdaftuar · Pull Request #20187 · bitcoin/bitcoin · GitHub 15:26:58 the tl;dr is that after looking into how eviction works from the new and tried tables, i decided it all works better to make sure that our block-relay-only peers in fact get moved to the tried table 15:27:19 which necessitates invoking addrman functions on those addresses and changing addrman state of course 15:27:40 the change in net_processing seems reasonable to me. I haven't looked at the changes in net. 15:27:44 right 15:27:48 but lots of things to consider (particularly privacy issues that are hard to reason about) so if someone spots a problem i'd love to discuss 15:28:17 one particular problem is if the timestamps we return in getaddr messages for those peers will stick out somehow! 15:28:34 it's a balance beteeen not updating addrman to minimize detectability of block-only connections, and updating it to make sure we keep good ones 15:28:40 [13bitcoin] 15jonasschnelli opened pull request #20198: Show name, format and if uses descriptors in bitcoin-wallet tool (06master...062020/10/wallet_tool_sqlite) 02https://github.com/bitcoin/bitcoin/pull/20198 15:28:40 yep 15:28:51 the idea makes sense, I'll take a closer look at the code 15:29:48 (that's all i've got) 15:30:37 While we're on the subject of addrman, it seems strange to me that it's owned by CConnMan. I think it makes sense to pull it out into a separate component that's owned by the node context object, so other components can access it directly. 15:30:48 sdaftuar: do you believe there are more issues than fixed by your PR? 15:30:57 Is there areason not to do that? 15:31:15 what other components need access to addrman ? or might need in the future? 15:31:18 jnewbery: whatever works 15:31:26 ariard: net_processing and rpc 15:31:35 No opinion on moving components around 15:31:39 sipa: not at the moment, i dont' think. the only other addrman-related thing i'm worrying about is addr relay i think 15:31:39 and net 15:31:44 but that's a different type of issue 15:32:13 currently net_processing access addrman through some forwarding functions in cconnman 15:32:50 sounds good to move so 15:33:33 any other topics before we wrap up? Anyone have any review begs? 15:34:16 what outstanding p2p bugfixs/followups are required for current release ? 15:34:29 I plan to circle back soon to finish reviewing #19858 which looks pretty close 15:34:33 https://github.com/bitcoin/bitcoin/issues/19858 | Periodically make block-relay connections and sync headers by sdaftuar · Pull Request #19858 · bitcoin/bitcoin · GitHub 15:35:01 hi, I started looking into i2p support 15:35:02 jonatack: thanks -- guessing it won't be merged until after we branch off the next release though 15:35:04 ariard: you can find current release issues/prs here: https://github.com/bitcoin/bitcoin/milestone/45 15:35:16 in case that affects your review priorities! 15:35:42 (I have one review beg: the backport of wtxid relay to 0.20 #19606) 15:35:44 https://github.com/bitcoin/bitcoin/issues/19606 | Backport wtxid relay to v0.20 by jnewbery · Pull Request #19606 · bitcoin/bitcoin · GitHub 15:36:23 ok, seems like that's all. Thanks folks. See you in two weeks 15:36:25 vasild: nice! Oh, #20120 had 3 acks by vasild promag hebasto 15:36:26 #endmeeting