15:00:45 <jnewbery> #startmeeting
15:00:45 <lightningbot> 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 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
15:00:54 <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:01:00 <jnewbery> amiti fjahr jeremyrubin lightlike emilengler jonatack hebasto jb55 elichai2
15:01:02 <jonasschnelli> hi
15:01:03 <jnewbery> hi folks!
15:01:08 <gleb> Hi
15:01:09 <fanquake> hi
15:01:09 <ryanofsky> yeah -noXXX for a list setting clears the list. hopefully you never have to rely on this!
15:01:10 <amiti> hi
15:01:11 <sipa> hi
15:01:12 <ariard> hi
15:01:22 <jonatack> hallo
15:01:28 <sdaftuar> heya
15:01:31 <ajonas> hi
15:01:50 <jnewbery> first of all, congrats everyone for getting so many PRs reviewed and merged before feature freeze!
15:01:53 <awesome_doge> hi
15:02:16 <jnewbery> we got addrv2, transaction request overhaul, taproot and anchor connections all merged
15:02:37 <jnewbery> Just one proposed topic today: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/P2P-IRC-meetings#20-oct-2020
15:02:40 <gribble> https://github.com/bitcoin/bitcoin/issues/20 | JSON-RPC callback · Issue #20 · bitcoin/bitcoin · GitHub
15:02:54 <jnewbery> Before we do that, does anyone have any updates or want to share what they're working on/prioritising?
15:03:45 <jnewbery> ok, our single topic is:
15:03:46 <jnewbery> 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 <jnewbery> sdaftuar: do you want to explain?
15:04:03 <sdaftuar> oy, i can try
15:04:40 <sdaftuar> 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 <emzy> hi
15:05:03 <sdaftuar> 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 <sdaftuar> 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 <gleb> It does indeed leak info, but I never thought about solving the issue in this fashion...
15:05:59 <sdaftuar> we can fix that, but it led to wondering: what good does this time do anyone, anyway?
15:06:24 <sipa> is it actually not used for anything?
15:06:32 <sdaftuar> 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 <gleb> There is a check in IsTerrible
15:06:39 <gleb> Seeing if it’s older than a month
15:06:46 <ariard> 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 <sdaftuar> but we do not use it for determining who to connect to, as far as i can tell.
15:08:44 <gleb> From what I remember I think you are right, but those existing features are not nothing
15:08:44 <jnewbery> using timestamps is already a well-known way of infering network topology: https://www.cs.umd.edu/projects/coinscope/coinscope.pdf
15:08:46 <sdaftuar> 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 <jonatack> CAddrMan::Good: "nTime is not updated here, to avoid leaking information about currently-connected peers."
15:09:16 <sipa> sdaftuar: good question
15:09:48 <sdaftuar> gleb: i imagine we could replace their use in those two places without that much trouble
15:09:52 <gleb> I think the way we use them, an adversary can’t really manipulate, because the checks are very moderate
15:10:05 <sdaftuar> eg by using nlasttry/nlastsuccess
15:10:29 <gleb> even if someone bumps their own addrs too much, they don’t really become “better”
15:10:43 <gleb> sdaftuar: and that won’t propagate through the network?
15:10:54 <gleb> it will have effect only locally at every node
15:11:02 <sdaftuar> 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 <gleb> Telling that a given address we’re relaying is not one year old...
15:12:36 <sdaftuar> we could still use nlastsuccess to filter out old addresses from our getaddr responses, i think?
15:12:39 <ariard> sdaftuar: right,  unverifiable data doesn't mean it can be useful even if it's gentleman-style of enforcment
15:12:59 <ariard> a lot of alternatives p2p stack doesn't sanitize their addrs with a feeler connection
15:13:17 <gleb> and they can retell this fact to other peers without connecting by themselves. Just tell what we told
15:14:03 <sdaftuar> interestingly, we don't update that time field when we successfully connect to a peer via a feeler connection, i believe.
15:14:09 <gleb> I need to look better at the code. I’d be very happy to get rid of this stuff
15:14:27 <sipa> i'll also have a look in more detail
15:14:30 <jnewbery> 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 <sipa> it's very appealing
15:15:02 <jnewbery> 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 <ariard> 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 <gleb> jnewbery: I suggest ariard thinks more whether time stamps can help light clients a lot comparing to feeler strategy
15:16:16 <amiti> +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 <jnewbery> 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 <gleb> They are unreliable, but they also can’t be exploited actively I think  (only leak info, passive exploit)
15:17:39 <sdaftuar> 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 <jnewbery> 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 <amiti> yeah, exactly, they can't be exploited because we write logic to not rely on it
15:18:06 <ariard> 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 <sipa> 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 <sdaftuar> agreed
15:19:23 <sipa> i imagine we'd do those at different points in time anyway
15:20:03 <gleb> Sipa: but Most of their use is already about setting them differently, it seems we mainly discussing dropping that :)
15:20:33 <gleb> or, well, using them to filter out responses I guess. We sort of “promised” to use them?
15:20:43 <gleb> whatever, i think we’re on the same page
15:20:49 <ariard> just set them to zero, in case of randomness source breakup that's not a fingerprint for your node
15:21:43 <gleb> Setting them to 0 would break compatibility
15:21:46 <sipa> ariard: if our RNG has issues, we have bigger problems
15:22:03 <gleb> old nodes think that ntime < 10000000 is trash iirc
15:22:12 <sipa> ariard: and setting them to 0 would actively hurt relay chances on current code
15:22:48 <gleb> we probably should randomize them within a week window from now or so
15:22:59 <ariard> good to know, do we have other compatibility bounds to care about beyond ntime < 10000000 ?
15:23:07 <jnewbery> 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 <gleb> Oh right, sorry, I’m on my phone
15:24:09 <jnewbery> maybe we just set it to MAX_UINT32
15:24:33 <sdaftuar> 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 <sdaftuar> cc amiti and jnewbery as this came up during a recent PR under review
15:26:06 <jnewbery> #20187
15:26:08 <gribble> 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 <sdaftuar> 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 <sdaftuar> which necessitates invoking addrman functions on those addresses and changing addrman state of course
15:27:40 <jnewbery> the change in net_processing seems reasonable to me. I haven't looked at the changes in net.
15:27:44 <sipa> right
15:27:48 <sdaftuar> 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 <sdaftuar> one particular problem is if the timestamps we return in getaddr messages for those peers will stick out somehow!
15:28:34 <sipa> 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 <bitcoin-git> [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 <sdaftuar> yep
15:28:51 <amiti> the idea makes sense, I'll take a closer look at the code
15:29:48 <sdaftuar> (that's all i've got)
15:30:37 <jnewbery> 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 <sipa> sdaftuar: do you believe there are more issues than fixed by your PR?
15:30:57 <jnewbery> Is there areason not to do that?
15:31:15 <ariard> what other components need access to addrman ? or might need in the future?
15:31:18 <sipa> jnewbery: whatever works
15:31:26 <jnewbery> ariard: net_processing and rpc
15:31:35 <gleb> No opinion on moving components around
15:31:39 <sdaftuar> 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 <sipa> and net
15:31:44 <sdaftuar> but that's a different type of issue
15:32:13 <jnewbery> currently net_processing access addrman through some forwarding functions in cconnman
15:32:50 <ariard> sounds good to move so
15:33:33 <jnewbery> any other topics before we wrap up? Anyone have any review begs?
15:34:16 <ariard> what outstanding p2p bugfixs/followups are required for current release ?
15:34:29 <jonatack> I plan to circle back soon to finish reviewing #19858 which looks pretty close
15:34:33 <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:35:01 <vasild> hi, I started looking into i2p support
15:35:02 <sdaftuar> jonatack: thanks -- guessing it won't be merged until after we branch off the next release though
15:35:04 <jnewbery> ariard: you can find current release issues/prs here: https://github.com/bitcoin/bitcoin/milestone/45
15:35:16 <sdaftuar> in case that affects your review priorities!
15:35:42 <jnewbery> (I have one review beg: the backport of wtxid relay to 0.20 #19606)
15:35:44 <gribble> https://github.com/bitcoin/bitcoin/issues/19606 | Backport wtxid relay to v0.20 by jnewbery · Pull Request #19606 · bitcoin/bitcoin · GitHub
15:36:23 <jnewbery> ok, seems like that's all. Thanks folks. See you in two weeks
15:36:25 <jonatack> vasild: nice! Oh, #20120 had 3 acks by vasild promag hebasto
15:36:26 <jnewbery> #endmeeting