19:00:31 <wumpus> #startmeeting
19:00:31 <lightningbot> Meeting started Thu Oct 17 19:00:31 2019 UTC.  The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:31 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:00:32 <jnewbery> hi
19:00:34 <warren> hi
19:00:35 <kanzure> hi
19:00:38 <gleb> hi
19:00:41 <moneyball> hi
19:00:44 <wumpus> #bitcoin-core-dev 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
19:00:48 <jonasschnelli> hi
19:00:49 <fanquake> hi
19:01:05 <dongcarl> hi
19:01:10 <jeremyrubin> hi
19:01:13 <wumpus> one proposed topic for today: taproot proposal next steps; possible review sessions (moneyball)
19:01:37 <sipa> hi
19:01:49 <moneyball> wumpus: i assume we'll do high priority list first?
19:01:50 <aj> hola
19:02:13 <promag> hi
19:02:19 <wumpus> moneyball: yes, let's start with that (I wait a bit in case people have additional last minute topics to propose)
19:02:55 <wumpus> #topic High priority for review
19:03:02 <achow101> hi
19:03:12 <wumpus> https://github.com/bitcoin/bitcoin/projects/8
19:03:18 <promag> please add 17135
19:03:30 <wumpus> #17135
19:03:32 <gribble> https://github.com/bitcoin/bitcoin/issues/17135 | gui: Make polling in ClientModel asynchronous by promag · Pull Request #17135 · bitcoin/bitcoin · GitHub
19:04:06 <fanquake> promag: can you fill out the description in that PR? Why is it high-prio?
19:04:30 <wumpus> promag: ok done
19:04:58 <promag> fanquake: IMO they are necessary for 0.19, also #16963 #17120
19:05:01 <promag> ty
19:05:01 <gribble> https://github.com/bitcoin/bitcoin/issues/16963 | wallet: Fix unique_ptr usage in boost::signals2 by promag · Pull Request #16963 · bitcoin/bitcoin · GitHub
19:05:03 <wumpus> yes, would be nice if you also say what it is blocking
19:05:04 <gribble> https://github.com/bitcoin/bitcoin/issues/17120 | gui: Fix start timer from non QThread by promag · Pull Request #17120 · bitcoin/bitcoin · GitHub
19:05:19 <wumpus> seems to big a change for 0.19.0 at least
19:05:24 <wumpus> 0.19.1 maybe
19:06:38 <promag> I'll update description sure
19:06:46 <fanquake> I'll re-review 16963
19:07:16 <fanquake> I'm not sure there's too much else to discuss high-prio wise other than hopefully people are doing some rc1 testing
19:07:18 <promag> but it's meant to fix #17112
19:07:20 <gribble> https://github.com/bitcoin/bitcoin/issues/17112 | v0.19.0rc1 GUI repeatedly not responding · Issue #17112 · bitcoin/bitcoin · GitHub
19:07:34 <fanquake> Maybe jump into the taproot discussion?
19:07:49 <wumpus> promag: that's just too deep a rabbit hole to fix before the release, is it really a reversion in 0.19?
19:08:09 <promag> yes
19:08:16 <wumpus> where did it happen then?
19:08:32 <promag> see https://github.com/bitcoin/bitcoin/issues/17112#issuecomment-541659632
19:09:18 <wumpus> ok
19:09:35 <wumpus> I still think adding the GUI async stuff last minute is risky
19:09:59 <sipa> is there a simpler way to fix the issue itself?
19:10:00 <promag> well it's still a HP decision
19:10:16 <sipa> harry potter? health points? hewlett-packard?
19:10:17 <wumpus> I mean it's the obvious thing to do long term but between rcs?
19:10:31 <promag> heh, high prio
19:10:42 <sipa> oh lol, sorry, that should have been obvious :)
19:11:18 <promag> wumpus: it's fine if we postpone the that change, but let's decide that then :p
19:11:36 <aj> gui hangs would cause a lot of user complaints though?
19:11:55 <wumpus> yes, but we've always have lots of GUI hangs during initial sync
19:12:07 <jonasschnelli> Yes. This has been there for a long time
19:12:16 <wumpus> this lock just makes it slightly worse I guess
19:12:25 <jonasschnelli> No need for risky fixes in rc-timeframe
19:12:28 <sipa> if it's a visible regression it seems reasonable to fix it, but is it possible to just say undo the change that caused it rather than fixing the root issue?
19:12:36 <jonasschnelli> Better do proper fixes in 0.20
19:12:36 <wumpus> but this is always been a problem and I don't think we're going to solve it for 0.19.0
19:12:45 <wumpus> can we fix it by reverting something instead?
19:12:46 <promag> Jackielove4u: tested that PR in win/linux/macos and compared to 0.18
19:12:51 <wumpus> (on the 0.19 branch I mean)
19:12:54 <promag> 0.19rc is really bad in that regards
19:12:54 <wumpus> instead of adding more code
19:12:59 <jonasschnelli> Its not an easy fix
19:13:02 <jonasschnelli> It requires conceptual changes
19:13:17 <wumpus> no, it's not an easy fix, it's changing things in a very differnt place than where the problem was introduced
19:13:18 <sipa> jonasschnelli: well there was a specific PR that caused it, or at least worsened it
19:13:30 <promag> yeah revert is another option I guess, MarcoFalke_?
19:13:35 <sipa> i'd like to know if we can revert that PR instead
19:13:38 <wumpus> so this basically means we're delaying 0.19.0 indefnintely
19:13:40 <jonasschnelli> sipa: maybe a part of it. But GUI unresponsivenes was always an issue
19:13:41 <wumpus> until the GUI is async
19:13:54 <jonasschnelli> The GUI was not created to be async
19:13:56 <sipa> jonasschnelli: i'm very much aware; but i'm not talking about fixing the root issue, only the regression
19:13:57 <wumpus> which is not soemthing I can get behind, sorry
19:14:08 <promag> BTW, the same approach is already used in WalletController
19:14:56 <jonasschnelli> sipa: sure. If we can fix the regression in a sane way we may want it in 0.19. But unclear of 17120 fixes that
19:15:11 <jonasschnelli> s/of/if
19:15:50 <sipa> jonasschnelli: i'm literally suggesting reverting the PR that caused it, nothing more
19:16:03 <fanquake> so revert #14193 ?
19:16:07 <wumpus> that would be ok with me
19:16:07 <gribble> https://github.com/bitcoin/bitcoin/issues/14193 | validation: Add missing mempool locks by MarcoFalke · Pull Request #14193 · bitcoin/bitcoin · GitHub
19:16:28 <valwal> hi
19:16:35 <promag> ok, I'll revert it in 0.19 and see how it goes
19:16:46 <fanquake> Does that not mean we are back with whatever problems that was meant to fix?
19:16:51 <wumpus> what we're arguing against is making changes to GUI asynchronicity before the 0.19.0 release, I think if it's possible to revert the locking change that caused it in the first place that's a more direct and safer option
19:17:01 <jonasschnelli> Yes
19:17:26 <wumpus> but  #17135 adds an extra thread, that's not a trivial fix
19:17:28 <gribble> https://github.com/bitcoin/bitcoin/issues/17135 | gui: Make polling in ClientModel asynchronous by promag · Pull Request #17135 · bitcoin/bitcoin · GitHub
19:17:40 <wumpus> all we know it might make locking issues worse somewhere
19:17:41 <jonasschnelli> Reverting #14193 (merged in july, invasive) may be not super easy
19:17:44 <gribble> https://github.com/bitcoin/bitcoin/issues/14193 | validation: Add missing mempool locks by MarcoFalke · Pull Request #14193 · bitcoin/bitcoin · GitHub
19:17:47 <fanquake> MarcoFalke: Had you seen inconsistent mempool reads prior to 14193?
19:18:52 <wumpus> which is okay for a release schedule for 0.20.0 where there's a long time to go before the actual release, but not something to do last minute
19:19:15 <promag> I'll just submit the revert commit to 0.19, ask for gitian build and let's see how testing goes
19:19:23 <wumpus> promag: thanks!
19:19:40 <jonasschnelli> promag: nice!
19:19:45 <achow101> could just add a known issue to the release notes and tell people to not mess around in the gui during IBD?
19:20:04 <wumpus> achow101: that would be the fallback then, yes
19:20:18 <promag> achow101: I think it's not just ibd - not sure
19:20:30 <achow101> i would be somewhat surprised if people were actually doing things in the gui during ibd
19:21:01 <jonasschnelli> achow101: I guess the UX stopper is, if someone wants to get an address or so while he is syncing the last 2-3 days
19:21:11 <wumpus> not only IBD but also when you catch up after having not run it for a while
19:21:13 <jonasschnelli> (laptop users, etc.)
19:21:21 <wumpus> that's usually the most annoying time for it to happen
19:21:26 <jonasschnelli> indeed
19:21:29 <wumpus> espeiclaly when you want to send a transaction quickly
19:21:31 <promag> the problem is more evident in windows because windows show that app is not responding that can cause some frustration
19:21:36 <sipa> #17135 does not look crazy, and may be worth considered... but my preference is reverting
19:21:38 <gribble> https://github.com/bitcoin/bitcoin/issues/17135 | gui: Make polling in ClientModel asynchronous by promag · Pull Request #17135 · bitcoin/bitcoin · GitHub
19:21:44 <wumpus> but anyhow this is not new, and can't be solved before 0.19.0
19:22:07 <wumpus> sipa: it's not crazy but it does add a new thread
19:22:14 <jonasschnelli> I tested 17135 a bit and I still had freezes. It's better but not solved.
19:22:23 <wumpus> I think that's a big change between rcs
19:22:28 <sipa> wumpus: agree it's a big change to do at this stage
19:22:47 <jonasschnelli> Lets aim for the mempool locks revert and add more fixes in 0.20
19:22:54 <promag> jonasschnelli: I'd love to know more about that :D
19:22:56 <wumpus> and... it's annoying but not a crash issue
19:23:20 <achow101> would reverting the locks cause other problems?
19:23:21 <promag> wumpus: yeah but people can force kill upon these hangs
19:23:23 <jonasschnelli> promag: yeah. Currently setting up a test env to better reproduce those locks
19:23:25 <achow101> I guess it wouldn't really be a regression..
19:23:59 * jonasschnelli wishes the GUI would just work via RPC
19:24:06 <warren> deadlock can happen in headless bitcoind?
19:24:24 <sipa> warren: this discussion is about Qt GUI only
19:24:28 <warren> ok thx
19:24:30 <promag> next topic?
19:24:50 <fanquake> cc moneyball
19:24:55 <moneyball> Hi
19:25:14 <jnewbery> MarcoFalke has been responding but his messages aren't getting through
19:25:16 <wumpus> #topic Taproot proposal (moneyball)
19:25:36 <moneyball> aj, harding, and i have been discussing organizing a review of the schnorr/taproot/tapscript proposals
19:25:37 <wumpus> jnewbery: oh no :( is he logged in to nickserv
19:25:42 <moneyball> our thinking is here https://docs.google.com/document/d/1G48-yhZMLLMZW68Bq59h_FBi__pwFKoWKF1D385I38Y/edit#
19:26:00 <moneyball> it would be a 7 week series ending by end of year
19:26:07 <promag> jnewbery: MarcoFalke_ been there too :(
19:26:46 <wumpus> it looks like he isn't logged in-- you can't post here in that case, and worse, you don't really get feedback--I had the same problem some meetings ago
19:26:47 <moneyball> i raise it here because (a) create awareness (b) solicit feedback on the idea (c) solicit ideas for how to invite high quality broad reviewers
19:27:13 <moneyball> i'm thinking we will communicate this on bitcoin-dev, lightning-dev, optech newsletters + slack to member companies
19:28:26 <sipa> sounds like a great idea to get feedback
19:28:30 <moneyball> the goal of this is to generate high quality feedback, give us further confidence in the proposal, give the Core project confidence to proceed with code review and QA of the existing PR, and to improve the decentralization / broader participation of review of the proposal
19:28:43 <sipa> there is no PR
19:28:52 <moneyball> sorry, branch
19:29:02 <aj> to proceed with getting an implementation that we can PR :)
19:29:18 <sipa> yeah
19:29:40 <MarcoFalke> test
19:29:42 <jnewbery> (b) I think it's a good idea
19:29:47 <sipa> MarcoFalke: reading you loud and clear
19:29:47 <aj> MarcoFalke: success
19:29:53 <MarcoFalke> I was wondering why no one responeded to me for days
19:29:57 <jonasschnelli> heh
19:29:59 <sipa> oww
19:30:19 <fanquake> moneyball sounds good. I see some australian friendly time slots as well
19:30:19 <jnewbery> welcome back, MarcoFalke
19:30:21 <wumpus> oh crap
19:30:36 <promag> lol
19:30:57 <achow101> moneyball: is the idea to kind of do it in the style of the pr review club?
19:31:02 <wumpus> kafkaIRC
19:31:28 <sipa> wumpus: IRC where only some people see what certain others are saying... sounds like twitter to me
19:31:38 <moneyball> achow101: aj has proposed a small group study structure then come together as a larger group once a week via IRC for Q&A (see the doc for details)
19:31:49 <MarcoFalke> I think the issue was the znc put an underscore behind my name and bitcoin-core-dev only allows logged in users to post?
19:31:54 <MarcoFalke> (sorry ot)
19:31:56 <moneyball> it will definitely require diligence and commitment, but i think it is necessary for a high quality review of the proposal
19:31:59 <wumpus> sipa: yes it seems the same kind of shadowban idea
19:33:08 <wumpus> MarcoFalke: yes, it allows only logged in users to post, though you can log in while having an alternative nick (by using /msg nickserv login <registered_user> <pass> AFAIK instead of just /... login <PASS>)
19:33:15 <moneyball> "once a week" = twice a week at different times to provide good global coverage
19:33:39 <wumpus> MarcoFalke: the crazy thing is that freenode doesn't seem to send an error message anymore in that case
19:33:57 <sipa> 7 weeks seems like a lot, but i also agree it's not something that can be done in 1-2 hours
19:33:59 <jeremyrubin> i think it would make sense to talk about deployment/acceptance criteria
19:34:47 <wumpus> MarcoFalke: the reason for "only registered users can post" is for some spam avoidance, it used to be really bad at some point, maybe it's no longer necessary I don't know
19:34:48 <sipa> jeremyrubin: imho that's an entirely different discussion; we first need to know if the ecosystem is on board including all the details it entails (to the extent they care), then we can talk about activation
19:35:06 <warren> MarcoFalke: strongly recommend figuring out the nickserv SASL or TLS cert authentication, then you'll never connect without login
19:35:32 <wumpus> warren: somehow it did happen to me once
19:35:44 <achow101> moneyball: who are the participants? open to public?
19:35:47 <warren> wumpus: yeah server splits or other weird conditions
19:35:57 <jeremyrubin> sipa: thats what i said kinda
19:36:34 <jeremyrubin> "we first need to know if the ecosystem is on board" == acceptance criteria
19:37:17 <jeremyrubin> knowing what that means, i agree, is a separate discussion
19:37:50 <moneyball> achow101: open to public. some level of moderation may be needed just to keep folks on track and avoid trolls. i mention above the proposed method of outreach. if others have ideas let me know.
19:38:08 <jeremyrubin> but an impt one for taproot and other stuff too, separate from the actual instance
19:38:25 <aj> sipa: i don't think shrinking it below 5 "sessions" works, and multiple sessions a week seems a bit intense, 7 weeks seems the max before hitting xmas/new year, and 5 + 2 weeks padding seems okayish
19:38:42 <sipa> jeremyrubin: i have no idea what you're trying to say
19:38:50 <sipa> aj: ok
19:39:01 <jeremyrubin> ok
19:39:11 <jeremyrubin> can chat out of band later
19:39:17 <sipa> ok
19:40:19 <moneyball> also feel free to comment directly in the doc if you have ideas or concerns
19:40:51 <moneyball> thanks everyone! hope to see many of you participate. it is a fairly large commitment but as aj points out in the doc:
19:40:55 <moneyball> https://www.irccloud.com/pastebin/igNUbnnf/
19:41:02 <wumpus> thanks!
19:41:15 <sipa> sgtm
19:43:28 <MarcoFalke> #proposedmeetingtopic address relay and spv clients
19:43:35 <gleb> I wanted to mention that in #17163 we're discussion whether we should stop relaying addresses to light clients or just limit relay to the cases where it does not hurt addr propagation.
19:43:35 <gleb> It's not clear for us whether SPV should sync their addr database with random peers from the network. Input welcome!
19:43:37 <gribble> https://github.com/bitcoin/bitcoin/issues/17163 | p2p: Avoid relaying ADDR messages to SPV clients by naumenkogs · Pull Request #17163 · bitcoin/bitcoin · GitHub
19:44:45 <gleb> I guess we tend to "allowing SPV to ask for addresses does not hurt", so the latter, but I'd like to hear more opinions.
19:45:00 <MarcoFalke> I guess we switched topics?
19:45:05 <MarcoFalke> #topic address relay and spv clients
19:46:23 <gleb> To be clear: currently when we get a short ADDR message (less than 10), we choose 1 or 2 peers to relay forward. That's the primary way to announce new nodes in the network. Currently it's possible that we choose SPV which will throw it on the floor, which is unfortunate.
19:47:26 <aj> how about just biassing against picking peers for the 1 or 2 to relay for if those peers haven't sent us ADDR messages already, if that makes sense?
19:47:36 <achow101> I think it would hurt SPV clients to not receive addr messages
19:47:46 <achow101> *for them to not receive addr messages
19:48:55 <gleb> I also mentioned above that an explicit service flag is maybe a good idea, to decouple addr-relay from SPV/non-SPV reasoning.
19:49:41 <gleb> Or whatever other explicit way you can imagine. aj: Biasing is sort of that, but implicit.
19:50:36 <gleb> Anyway, just wanted to mention that this discussion is taking place in #17163. Let's continue there. Thanks.
19:50:38 <gribble> https://github.com/bitcoin/bitcoin/issues/17163 | p2p: Avoid relaying ADDR messages to SPV clients by naumenkogs · Pull Request #17163 · bitcoin/bitcoin · GitHub
19:50:45 <achow101> gleb: are you certain that spv clients discard addr messages?
19:51:22 <MarcoFalke> as mentioned on the pull request, SPV clients should not deal with addr messages. I can only see ways in which they shoot themselves in the foot
19:51:30 <wumpus> maybe most current ones do, but it's not a necessary coupling
19:51:40 <sipa> MarcoFalke: i don't understand that comment
19:51:54 <sipa> and i don't understand what SPV has to do with it
19:52:12 <MarcoFalke> Though, there might be a node without NODE_NETWORK set that wants addr messages
19:52:13 <wumpus> I don't get why SPV clients couldn't implement full address management instead of blindly trusting DNS seeders
19:52:18 <MarcoFalke> So that coupling doesn't make sense
19:52:46 <achow101> i don't understand what about spv makes it such that they don't need addrs
19:52:49 <wumpus> right
19:52:58 <MarcoFalke> wumpus: I doubt implementing address management is trivial
19:53:07 <MarcoFalke> See for example feeler connections
19:53:08 <wumpus> no one is talking about 'trivial'
19:53:17 <sipa> MarcoFalke: i'm sure it's nontrivial, but it's nontrivial for everyone
19:53:28 <wumpus> but why couldn't they if they wanted?
19:54:02 <wumpus> so the reasoning is 'SPV implementations tend to be trivial, so they won't implement something as complex as addr handling'
19:54:04 <sipa> i agree partially with luke-jr that it's reasonable for lightweight clients to instead rely on a trusted server... but not all light clients do that; and within those that do, i think doing actual ip address management is far more reasonable that blindly using dns seeds
19:54:14 <wumpus> which more or less makes sense but it's very indirect
19:55:00 <warren> The "throwing to the floor" part is concerning but I'm thinking address relay is best effort. It is reasonable to need to connect to multiple/many peers before you get any quality addresses. There is no guaratee that a peer you try first is honest.
19:55:12 <BlueMatt> what does wasting a service bit cost? we've got a bunch of 'em :p
19:55:51 <achow101> wumpus: but at the same time, they are probably using some library like bitcoinj that already handles it for them (at least I think bitcoinj uses addrs)
19:55:54 <MarcoFalke> would the service bit be for "i want addr messages" or "i send addr messages"
19:56:10 <wumpus> achow101: that's another assumption though, based on current software
19:56:18 <wumpus> achow101: I"m not sure that should determine the protocol
19:56:34 <sipa> i think a service bit makes sense (e.g. explicitly opting out of address relay), but i think that's an independent question of whether we want to bias our own relay away from nodes we assume won't participate in relaying further
19:56:47 <gleb> MarcoFalke: Your peer shouldn't care whether you promise to send them something or not
19:56:56 <MarcoFalke> I also think it makes sense to make this more explicit with service bits or a new message type
19:57:02 <MarcoFalke> Would addrv2 help with that?
19:57:22 <wumpus> it could be added to that
19:57:31 <dongcarl> addrv2 has a message to indicate that I want addrv2 messages
19:57:57 <dongcarl> I'm hearing we want some kind of more complex negotiation?
19:58:02 <wumpus> the same mechanism (say, a new message) for requesting addrv2 messages could be used to request *NO* addr messages
19:58:22 <MarcoFalke> gleb: It is useful to know whether a peer might not relay addr messages, but still wants them
19:58:26 <wumpus> dongcarl: I don't think it would be more complex
19:58:38 <wumpus> just a third option (besides addr and addrv2)
19:58:50 <gleb> MarcoFalke: yeah, if they want, they should signalize. But whether they send us or not — we don't care.
19:58:59 <gleb> Okay, it seems like I change a PR to avoid forwarding ADDR to SPV, but still allow SPV to ask for addresses.
19:59:26 <warren> +1
19:59:31 <gleb> And then we should expand addrv2 spec for this further change separately.
19:59:33 <sipa> that sounds reasonable
19:59:38 <achow101> how are you determining a node is spv? no node_network?
19:59:51 <gleb> and no node_network_limited.
19:59:58 <aj> (we also drop ADDRs on the floor if they're from a node we've set as block-relay-only per #15759)
20:00:01 <gribble> https://github.com/bitcoin/bitcoin/issues/15759 | p2p: Add 2 outbound block-relay-only connections by sdaftuar · Pull Request #15759 · bitcoin/bitcoin · GitHub
20:00:07 <achow101> that'd affect old pruned nodes tho
20:00:24 <wumpus> aj: oh? block relay only also means no addr relay?
20:00:27 <gleb> Yeah, but we already don't forward short addr to "block_relay_only" :)
20:00:30 <wumpus> aj: I kind of wondered that
20:00:40 <BlueMatt> time
20:00:49 <aj> wumpus: only for the 2 of 10 outbounds when we do tx's but have a couple of block-relay-only nodes
20:01:13 <wumpus> #endmeeting