19:01:12 <wumpus> #startmeeting
19:01:12 <lightningbot> Meeting started Thu Apr  2 19:01:12 2020 UTC.  The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:01:12 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:01:16 <jonasschnelli> hi
19:01:21 <sipsorcery> hi
19:01:27 <hebasto> hi
19:01:28 <fjahr> hi
19:01:28 <cfields> hi
19:01:34 <sipa> hi
19:01:44 <amiti> hi
19:01: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 ariard digi_james amiti fjahr
19:01:46 <wumpus> jeremyrubin lightlike emilengler jonatack hebasto jb55
19:01:57 <jkczyz> hi
19:01:59 <jb55> hi
19:02:07 <achow101> hi
19:02:12 <jonatack_> hi
19:02:24 <nehan_> hi
19:02:43 <wumpus> no proposed topics for this week, it appears
19:02:46 <elichai2> Hi
19:02:48 <wumpus> any last minute ones?
19:03:07 <jeremyrubin> hi
19:03:14 <luke-jr> wumpus: I saw one earlier.. :p
19:03:17 <jeremyrubin> Hope everyone is safe & healthy :)
19:03:19 <luke-jr> 0.20 bugfixes
19:03:22 <luke-jr> or smth like that
19:03:33 <wumpus> FWIW it's time to do the branch-off for 0.20
19:03:38 <wumpus> and release rc1
19:03:59 <wumpus> luke-jr: don't see it in http://gnusha.org/bitcoin-core-dev/proposedmeetingtopics.txt, but that makes sense
19:04:03 <luke-jr> IMO #18192 and #18465 should be fixed before rc1
19:04:05 <gribble> https://github.com/bitcoin/bitcoin/issues/18192 | Bugfix: Wallet: Safely deal with change in the address book by luke-jr · Pull Request #18192 · bitcoin/bitcoin · GitHub
19:04:06 <gribble> https://github.com/bitcoin/bitcoin/issues/18465 | bitcoin-tx (and probably others) fails to build without libevent · Issue #18465 · bitcoin/bitcoin · GitHub
19:04:22 <luke-jr> could perhaps still branch off anyway, as they're not really ready :/
19:05:06 <luke-jr> 18192 only affects avoid-reuse wallets, but the harm is irrepairable once affected
19:05:13 <wumpus> those are not even tagged for 0.20
19:05:30 <promag> hi
19:06:05 <wumpus> unless they're realy urgent might include them in a later rc or 0.20.1
19:06:19 <sipa> are they 0.20 regressions?
19:06:51 <dongcarl> hi
19:06:52 <luke-jr> not regressions, no
19:07:22 <luke-jr> I suppose 18465 has an easy workaround
19:07:39 <sipa> when was 18192 introduced?
19:07:49 <sipa> or when was the problem it solves introduced?
19:07:52 <luke-jr> when avoid-reuse wallets were introduced, 0.19.0 IIRC
19:08:44 <sipa> this sounds moderately serious... i suspect we just haven't heard about it much because few people enable that setting, i expect?
19:08:53 <luke-jr> maybe; I don't know how popular it is
19:09:19 <luke-jr> IMO if we don't fix it, we should at least put it in rel notes as a known issue
19:09:27 <achow101> i doubt people really use multiwallet or create non-default wallets
19:09:41 <luke-jr> achow101: it's not multiwallet-related
19:09:54 <achow101> luke-jr: but you have to be using the multiwallet feature to enable it
19:10:06 <achow101> i.e. create a new wallet
19:10:10 <luke-jr> hmm
19:12:13 <luke-jr> also from earlier [16:52:31] <MarcoFalke> Looks like #18487 , #18487 and #18487 are the last three things to get in before branch-off?
19:12:14 <achow101> oh wait, we can use the setwalletflag rpc to enable it. but that requires knowing what you're doing. it's not exposed in the gui
19:12:15 <gribble> https://github.com/bitcoin/bitcoin/issues/18487 | rpc: Fix rpcRunLater race in walletpassphrase by promag · Pull Request #18487 · bitcoin/bitcoin · GitHub
19:12:15 <gribble> https://github.com/bitcoin/bitcoin/issues/18487 | rpc: Fix rpcRunLater race in walletpassphrase by promag · Pull Request #18487 · bitcoin/bitcoin · GitHub
19:12:16 <gribble> https://github.com/bitcoin/bitcoin/issues/18487 | rpc: Fix rpcRunLater race in walletpassphrase by promag · Pull Request #18487 · bitcoin/bitcoin · GitHub
19:12:31 <luke-jr> rtt
19:12:33 <luke-jr> err*
19:12:38 <luke-jr> [16:53:06] <MarcoFalke> * #18458 #18506
19:12:40 <gribble> https://github.com/bitcoin/bitcoin/issues/18458 | net: Add missing cs_vNodes lock by MarcoFalke · Pull Request #18458 · bitcoin/bitcoin · GitHub
19:12:41 <gribble> https://github.com/bitcoin/bitcoin/issues/18506 | net: Hardcoded seeds update for 0.20 by laanwj · Pull Request #18506 · bitcoin/bitcoin · GitHub
19:13:00 <promag> lol
19:14:26 <wumpus> yes those are tagged https://github.com/bitcoin/bitcoin/pulls?q=is%3Aopen+is%3Apr+milestone%3A0.20.0
19:14:54 <luke-jr> 18192 & 18465 should probably at least get tagged, even if they end up slipping the release
19:15:43 <wumpus> ok
19:17:02 <jonatack_> another avoid_reuse related bugfix is #17824 (2 acks)
19:17:06 <gribble> https://github.com/bitcoin/bitcoin/issues/17824 | wallet: Prefer full destination groups in coin selection by fjahr · Pull Request #17824 · bitcoin/bitcoin · GitHub
19:17:56 <wumpus> this adds way more things last-minute for 0.20 than I expected
19:18:03 <jonatack_> (not saying it's as urgent)
19:18:18 <sipa> 17824 seems like a bigger change
19:18:26 <wumpus> we could also add a note to the release notes that avoid-reuse is buggy
19:18:46 <wumpus> or disable it
19:19:07 <wumpus> oh wait it was introduced in 0.19 not 0.20
19:19:12 <wumpus> so not that
19:22:01 <sipa> maybe let's prioritize review on #18192, and see how far we get?
19:22:03 <gribble> https://github.com/bitcoin/bitcoin/issues/18192 | Bugfix: Wallet: Safely deal with change in the address book by luke-jr · Pull Request #18192 · bitcoin/bitcoin · GitHub
19:22:10 <sipa> if not, a release note can always be added
19:23:12 <wumpus> let's set a new deadline for the branch-off then?
19:23:40 <wumpus> we missed yesterday at least :)
19:23:50 <luke-jr> why not just branch off now?
19:24:06 <promag> +1
19:24:13 <wumpus> I want to do rc1 release at the same time
19:24:18 <promag> oh kk
19:24:56 <sipa> seems reasonable to aim to do them simultaneously
19:24:58 <jeremyrubin> #proposedmeetingtopic limited use of boost in consensus for backports
19:25:02 * luke-jr shrugs
19:25:38 <wumpus> I don't see a reason to do a branch-off without immediately doing rc1, that would just result in more backporting work
19:26:03 <wumpus> oh no, no boost topic please
19:26:13 <jeremyrubin> lol :|
19:26:14 <sipa> at least not when we have known issues to solve still
19:26:38 <luke-jr> jeremyrubin: we'd need to adapt the build system.. isn't it avoidable?
19:26:46 <jeremyrubin> luke-jr: no it isn't
19:27:09 <wumpus> sipa: right
19:27:21 <luke-jr> what kind of bugfix requires boost? :/
19:27:33 <jeremyrubin> anyways we can discuss after the meeting if needed or when it's the topics turn
19:27:44 <jeremyrubin> but I don't think anyone wants to discuss it now
19:27:52 <jeremyrubin> Except maybe you and I
19:28:30 <wumpus> well it's not like we have other topics
19:28:42 <luke-jr> ^
19:28:45 <jeremyrubin> Ah I took your request literally
19:28:46 <wumpus> #topic limited use of boost in consensus for backports
19:29:16 <jeremyrubin> Well; I think on the 0.21 horizon is upgrading to c++17, one feature of which means things like std::option being available
19:29:34 <jeremyrubin> We're currently reviewing code for things like #18401
19:29:37 <gribble> https://github.com/bitcoin/bitcoin/issues/18401 | Refactor: Initialize PrecomputedTransactionData in CheckInputScripts by jnewbery · Pull Request #18401 · bitcoin/bitcoin · GitHub
19:29:56 <jeremyrubin> Which IMO should be properly written/refactored using option types
19:30:05 <jeremyrubin> See https://github.com/bitcoin/bitcoin/pull/17977#discussion_r370948973
19:30:09 <sipa> if it came at no cost, sure
19:30:36 <sipa> but the level of review necessary for those things already far outweighs the (imho) minimal complication of just splitting variables up into a bool + other var
19:30:48 <jeremyrubin> issue exists outside of taproot
19:31:05 <sipa> sure
19:31:16 <sipa> i wasn't speaking about that specifically
19:31:16 <jeremyrubin> at large, not just in this context -- if we need to gate development of features which touch consensus to not use c++17 features in consensus
19:31:27 <jeremyrubin> because it will interfere with backports
19:31:28 <luke-jr> IMO we should just not allow C++17 for consensus code where it would make this problem
19:32:05 <jeremyrubin> So there's three "solutions"
19:32:08 <sipa> right, until those are outside of backport window
19:32:15 <jeremyrubin> 1) No c++17 stuff for a while because of backports
19:32:45 <jeremyrubin> 2) Allow linking in boost c++17 API fill-ins  that we already have until out of support window
19:32:55 <wumpus> I think we should either switch to C++17 for the entire codebase, or not at all
19:33:15 <luke-jr> jeremyrubin: we don't have in libconsensus
19:33:38 <sipa> i think that once master is c++17, we can switch backports to c++17 as well, actually
19:33:49 <wumpus> moderating which parts of the codebase C++17 features are allowed and where not makes things very complicated for reviewers and maintainers
19:33:51 <luke-jr> sipa: defeats one big point of backports?
19:33:55 <jeremyrubin> 3) upgrade backports to c++17 :)
19:34:00 <wumpus> so I'd prefer to not use C++17 at all then
19:34:08 <hebasto> Not switching from c++11 for the entire codebase could be a problem for Qt stuff on macOS
19:34:16 <luke-jr> IIRC backports only delays us an extra year, right?
19:34:17 <wumpus> it's not like we really need it
19:34:34 <sipa> wumpus: well that's a deadlock, because there will always be some backport to support
19:34:40 <wumpus> it'[s always 'it would be nice to use this new c++ standard'
19:34:46 <wumpus> which it would be, sure
19:34:51 <sipa> so independent of the urgency of c++17 or when it is introduced, it's a good question to address
19:35:41 <sipa> luke-jr: backports exist because introduced features introduce compatibility issues for people who want a safer upgrade path
19:35:42 <jeremyrubin> It's also the right time to start thinking about it IMO -- if 0.21 will be c++17 release
19:36:05 <jeremyrubin> Which I've seen wumpus say is feasible
19:36:09 <wumpus> yes, I just think using C++17 for only parts of the codebase is impractical
19:36:12 <luke-jr> sipa: one of those issues is C++ version compat
19:36:33 <sipa> but if there are a significant amount of people who wouldn't be able to upgrade to a new major version because of c++ language compatibility issues, we simply shouldn't update to c++17 (yet)
19:36:40 <luke-jr> how hard would it be to compile some of the codebase with C++17 enabled, and not others?
19:36:42 <sipa> wumpus: that's fair
19:36:52 <cfields> luke-jr: I really don't like that idea.
19:37:01 <wumpus> luke-jr: I'm sure it's *possible* but I fear all kinds of API conflicts
19:37:03 <jeremyrubin> luke-jr: would probably make binaries bigger
19:37:10 <luke-jr> hmm
19:37:11 <cfields> luke-jr: I think the threat of something like an uncaught exception is very real in that scenario.
19:37:13 <jeremyrubin> wumpus: yeah
19:37:19 <luke-jr> cfields: right, I see
19:37:35 <wumpus> cfields: yes, it's probably an unacceptable risk in our case
19:37:53 <jeremyrubin> How was this handled historically?
19:37:57 <wumpus> of course, if it's only for checking
19:38:04 <sipa> so maybe we can settle on: we don't update to c++17 until we're in a position where no new backport releases are expected
19:38:05 <wumpus> you could compile the consensus files with c++11 *as well*
19:38:06 <jeremyrubin> Or was the switch to 11 widely supported enough at the time
19:38:09 <wumpus> and throw away the result
19:38:41 <sipa> if then an emergency happens that causes us to revert on that - so be it
19:38:43 <jeremyrubin> Or we can turn on c++17 for 0.20, but compile both for checking as wumpus suggests
19:38:54 <jeremyrubin> That way we're "priming" our backport window
19:39:17 <wumpus> I'd still prefer not to do this (also because what is 'consensus code' is not very well isolated in our code base)
19:39:21 <jeremyrubin> I don't think c++17 has any *breaking* changes?
19:39:40 <jeremyrubin> wumpus: I'm saying the whole codebase not just consensus
19:40:10 <cfields> jeremyrubin: iirc ^^ is what we did for c++11.
19:40:12 <wumpus> compile the entire codebase with c++17 and c++11? that's a lot of cverhead
19:40:12 <jeremyrubin> We can upgrade to c++17 today but not accept code changes that break c++11 compat.
19:40:36 <sipa> wumpus: we did that for a while with c++11 i think, i actually?
19:40:48 <sipa> though i'm not sure this is the right time; our focus now is 0.20
19:40:51 <jeremyrubin> Releases etc can be c++17, but anyone can build for c++11. We probably need to do this for 1-2 releases
19:40:59 <jeremyrubin> sipa: it is the right time to start worrying about this
19:40:59 <sipa> and i think the discussion of when c++17 is a different one
19:41:06 <wumpus> sipa: oh you mean more like a travis run that compiles with c++17 instead of c++11?
19:41:12 <cfields> sipa: yea, that's what we did.
19:41:12 <sipa> wumpus: right
19:41:16 <jeremyrubin> Because if we do it for 0.20 it means 0.21 and 0.22 can use c++17
19:41:25 <cfields> sipa: we had a release that was "technically" c++11, but iirc we didn't force the flag on.
19:41:32 <cfields> Then we forced it on in master.
19:41:36 <sipa> cfields: right
19:41:52 <wumpus> makes sense to do that again then
19:41:54 <wumpus> for 0.21
19:41:57 <cfields> Don't remember for backports, I guess we were just kinda careful/reasonable about it?
19:42:09 <sipa> i guess if someone does the work for adding a c++17 travis build, and it passes with effectively no code changes... why not
19:42:39 <wumpus> I don't expect that to take a lot of changes
19:43:16 <sipa> i think so
19:44:01 <jeremyrubin> sounds like a plan to me?
19:44:11 <cfields> Also, we banned certain features when we started with c++11. thread_local is the obvious one that comes to mind, but IIRC there were others as well.
19:44:34 <dongcarl> if there's consensus, can someone summarize the game plan?
19:45:38 <jeremyrubin> I can try...
19:46:00 <jeremyrubin> 1) Make 0.20 c++17 and c++11 buildable if easily doable
19:46:05 <wumpus> dongcarl: the idea is to add a travis run to compile the current code with c++17, do the minimum changes to be compatible with c++17 as well as c++11, for 0.21
19:46:13 <sipa> trying a C++17 build now
19:46:24 <sipa> with gcc 9.3
19:46:37 <jeremyrubin> I thought we'd do it for 0.20 if it's a small patchset and we haven't branched it yet?
19:46:37 <wumpus> and delay the real switch to c++17 (like actually using new features) to 0.22
19:46:51 <wumpus> no, we're not going to do anything like that for 0.20
19:46:53 <jeremyrubin> Why not do it in 0.20 if it can be done?
19:47:05 <jeremyrubin> It has basically 0 functional changes?
19:47:08 <wumpus> 0.20.0rc1 should have been tagged yesterday
19:47:30 <wumpus> if anything is going in it's bugfixes
19:47:37 <cfields> fwiw, I'd like to do at least a small audit on the c++17/c++20 errata to see if there are any obvious implementation minefields to look out for.
19:47:41 <jeremyrubin> I mean it sort of doesn't really matter if it's in 0.20 or not as c++17 compat can be backported into a minor anyways...
19:48:29 <dongcarl> Okay, so what will gitian builds use? v0.20: c++11, v0.21: c++11, v0.22: c++17?
19:48:30 <cfields> (c++20 because presumably several c++17 issues were fixed there)
19:48:37 <dongcarl> cfields: Right, good call
19:49:01 <wumpus> dongcarl: gitian builds could use c++17 from 0.21 on
19:49:18 <wumpus> it just still has to be compatible with c++1
19:49:49 <sipa> wumpus: ah, so we can easily back out back to c++11 if issues are discovered?
19:49:51 <wumpus> so that things can be backported
19:49:57 <wumpus> sipa: that too
19:49:59 <sipa> right
19:50:05 <sipa> that seems reasonable
19:50:17 <cfields> Not sure about g++, but clang++ definitely has a switch for enforcing c++11 syntax/features for newer standards.
19:50:31 <cfields> So that'd be easy enough to lint/automate.
19:50:39 <sipa> oh the irony
19:50:46 <jeremyrubin> cfields: we only need one compiler to have that feature fortunately
19:50:55 <sipa> the only compile error i get with c++17 is Span related
19:51:00 <cfields> jeremyrubin: right
19:51:27 <dongcarl> Got it: v0.20: COMPAT with c++11 + c++17 only if easily doable, GITIAN with c++11; v0.21: COMPAT with c++11 + c++17, GITIAN with c++17; v0.22: COMPAT with c++17, GITIAN with c++17. Is this the plan?
19:51:51 <wumpus> sgtm
19:52:02 <sipa> dongcarl: ack
19:52:20 <dongcarl> Great. Will post on the c++17 issue
19:52:22 <jeremyrubin> Loks right. And backports from 0.22 only go to 0.21?
19:52:35 <jeremyrubin> Or to 0.20 too if we can get those builds up easily
19:52:57 <jeremyrubin> (depending on the feature using APIs that make backport non-trivial)
19:53:33 <wumpus> jeremyrubin: yes
19:53:40 <cfields> I think as long as we don't race to replace every line of code with a c++17ism we'll be fine, generally.
19:53:53 <wumpus> it's pretty rare in the first place to backport things two releases
19:54:01 <wumpus> cfields: also that
19:54:32 <jeremyrubin> cfields: noooo you can't just refactor the whole.... hahahah find and replace go refactorrrrrrrrrr
19:55:07 <sipa> refactorrr̅
19:55:08 <wumpus> hehe I think we had a rule for that for C++11 as well, don't C++11-ize things for the sake of doing so
19:55:34 <sipa> right
19:57:10 <wumpus> is someone going to post this into the c++17 issue?
19:57:35 <jeremyrubin> dongcarl:
19:57:46 <sipa> FWIW, with #18468 + updating ax_cxx_compile_stdcxx.m4, everything compiles in c++17
19:57:48 <gribble> https://github.com/bitcoin/bitcoin/issues/18468 | Span improvements by sipa · Pull Request #18468 · bitcoin/bitcoin · GitHub
19:58:05 <dongcarl> Yeah making a little table now, perhaps jeremyrubin can add on about the backporting plan, not sure I understand it fully yet
19:58:30 <dongcarl> (add on in the issue)
19:59:10 <cncr04s> don't make me have to upgrade my distro to support new c++ versions
19:59:27 <sipa> cncr04s: what distro might that be?
19:59:44 <sipa> (we don't generally update c++ versions when it interferes with building on common platforms)
19:59:51 <cncr04s> ubuntu 14.04
20:00:10 <jeremyrubin> I'm not sure upgrading core is your largest concern there
20:00:14 <cfields> cncr04s: also, runtime is not affected. Releases are linked statically against lib(std)c++.
20:00:34 <cfields> So this is 99% a development decision.
20:00:43 <wumpus> #endmeeting