19:02:35 #startmeeting 19:02:35 Meeting started Thu Oct 8 19:02:35 2020 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot. 19:02:35 Useful Commands: #action #agreed #help #info #idea #link #topic. 19:02:37 hi 19:02:40 hi 19:02:52 hiiii 19:02:55 #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 19:02:57 amiti fjahr jeremyrubin lightlike emilengler jonatack hebasto jb55 elichai2 19:03:04 hi 19:03:13 ciao 19:03:27 hi 19:04:15 FWIW: the 0.21 feature freeze is in a week, it probably makes more sense to discuss what is tagged for the 0.21 milestone as high priority for review at this point 19:04:25 this is https://github.com/bitcoin/bitcoin/milestone/45 19:06:05 that's a long list 19:06:12 it looks like no other topics have been proposed for this week 19:06:15 How is the merge back of the GUI repository handled? MarcoFalke? 19:06:19 yes, it's also likely outdated 19:06:22 (Regarding freeze) 19:06:37 which makes it good to go over it I suppose 19:06:55 hi 19:07:30 and at least two of the issues are simply 'follow release process' 19:07:38 i think #19954 is pretty much done 19:07:41 https://github.com/bitcoin/bitcoin/issues/19954 | tor: complete the TORv3 implementation by vasild · Pull Request #19954 · bitcoin/bitcoin · GitHub 19:07:56 yes 19:08:08 #18077 and #18710 could be moved to 0.22 19:08:10 https://github.com/bitcoin/bitcoin/issues/18077 | net: Add NAT-PMP port forwarding support by hebasto · Pull Request #18077 · bitcoin/bitcoin · GitHub 19:08:13 https://github.com/bitcoin/bitcoin/issues/18710 | Add local thread pool to CCheckQueue by hebasto · Pull Request #18710 · bitcoin/bitcoin · GitHub 19:08:28 hebasto: ok, thanks 19:08:38 #19543 doesn't have an associated PR yet? 19:08:40 https://github.com/bitcoin/bitcoin/issues/19543 | Normalize fee units for RPC ("BTC/kB" and "sat/B) · Issue #19543 · bitcoin/bitcoin · GitHub 19:08:42 5 19:09:00 (typo) 19:09:04 i've been focusing on #19953 the past day or so as it seems to be the highest priority along with tor v3 19:09:07 https://github.com/bitcoin/bitcoin/issues/19953 | Implement BIP 340-342 validation (Schnorr/taproot/tapscript) by sipa · Pull Request #19953 · bitcoin/bitcoin · GitHub 19:09:39 aj: i planned to do #19543 after FF as it's a bugfix 19:09:40 https://github.com/bitcoin/bitcoin/issues/19543 | Normalize fee units for RPC ("BTC/kB" and "sat/B) · Issue #19543 · bitcoin/bitcoin · GitHub 19:09:51 two drafts also could be moved? 19:09:53 all todos are done for taproot, including the json tests in the qa-assets repo 19:09:59 jonatack: so that one needs 0.21 milestone? 19:10:16 aj: seems like it, but also seems not urgent for 0.21? 19:11:04 aj: oh "This needs to happen before the next major release. Otherwise, it will be a breaking change." 19:11:09 wumpus: yeah 19:11:12 ping MarcoFalke 19:11:28 wumpus: i'd very much like to get taproot in 0.21, as the alternative (i expect) is that we'll want it early in 0.22 anyway, which will just complicate backports 19:11:51 (the code, not activation obviously) 19:12:03 hebasto: done 19:12:14 sipa: agree! 19:12:48 +1 19:13:59 I feel like we should really push for #19077 if possible just to have it in the same version as descriptor wallets like we discussed 19:14:02 https://github.com/bitcoin/bitcoin/issues/19077 | wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets by achow101 · Pull Request #19077 · bitcoin/bitcoin · GitHub 19:14:09 But it's a big PR 19:14:28 i haven't paid attention to the PR itself, but it seems it's been making lots of progress lately, including review 19:14:45 Yeah it's had a decent amount of review already so it's not infeasible 19:15:05 i know wumpus had reservations about having it in 0.21 19:15:24 looks like we're starting to run into issues building bdb 4.8 on some platforms at least: #19411 19:15:26 https://github.com/bitcoin/bitcoin/issues/19411 | Unable to build BDB 4.8 on macOS Big Sur beta or Xcode 12.0 · Issue #19411 · bitcoin/bitcoin · GitHub 19:16:01 sipa: yes, it seems fairly risky to introduce a new wallet database format (which is used by default) in something last minute 19:16:03 --with-incompatible-bdb lalala 19:16:23 wumpus: it's only used by descriptor wallets which we already have marked as "experimental" 19:16:35 achow101: they're not created by default for new users? 19:16:39 no 19:16:41 in that case I"m oay with it 19:17:02 ah, descriptor wallets are not default for new wallets? 19:17:07 not yet 19:17:25 that's perhaps the best of both worlds... get descriptor wallets and sqlite in 0.21, but neither is default 19:17:37 if it's opt-in I see no problem 19:18:27 if descriptor wallets were the default, a lot of tests would be failing :p 19:20:46 wrt #20005 I'm not convinced we should do anything for it, the particular issue doesn't affect anything in our project and clearly in the longer run it's better left solved upstream 19:20:47 https://github.com/bitcoin/bitcoin/issues/20005 | memcmp with constants that contain zero bytes are broken in GCC · Issue #20005 · bitcoin/bitcoin · GitHub 19:21:54 agree 19:22:11 it's good to continue discussing the options, but i don't think there is urgency 19:22:22 sure, but removing the milestone then 19:22:22 we're not using gcc 9 or gcc 10 for releases either, i believe? 19:23:17 no, 8 at most 19:23:41 7.4 for linux x86_64 19:25:24 yes, correct 19:25:42 otherwise there's agreement what is on the milestone right now? 19:26:48 not everything will make it, but sure 19:27:11 i'm also hoping for 19988 19:27:23 given the amount of review it's had the past days 19:27:58 I think 19988 is ready. It has three or four recent ACKs now 19:28:22 #19988 19:28:25 https://github.com/bitcoin/bitcoin/issues/19988 | Overhaul transaction request logic by sipa · Pull Request #19988 · bitcoin/bitcoin · GitHub 19:29:03 50% test code :) 19:29:20 added to milestone anyway 19:29:25 thanks! 19:30:44 unrelated: i've noticed that sometimes clicking "show resolved" on github expands everything, and sometimes not 19:30:57 does anyone else have this, or know how it's caused? 19:30:58 #16463 could be removed from the milestone I guess 19:31:01 https://github.com/bitcoin/bitcoin/issues/16463 | [BIP 174] Implement serialization support for GLOBAL_XPUB field. by achow101 · Pull Request #16463 · bitcoin/bitcoin · GitHub 19:31:31 ok done 19:32:41 any other topics for today? 19:33:03 I have a question about c++17 19:33:42 wumpus: you pinged about moving https://github.com/users/ajtowns/projects/1 under bitcoin/ ; happy to, might be nice to move the automation into drahtbot at the same time 19:34:04 we're planning to move to c++17 (remove c++11 compat) in v0.21: https://github.com/bitcoin/bitcoin/issues/16684 19:34:18 no, in 0.22 right? 19:34:33 0.21 introduces c++17 compilation by default 19:34:36 oops sorry, yes v0.22 19:34:47 0.22 is c++17 only 19:34:49 okay 19:35:04 so after v0.21 branch we're theoretically free to use c++17 features throughout the codebase 19:35:11 yes 19:35:29 My question was whether it makes sense to try to keep c++11 compatability for consensus code for longer 19:35:40 for new code definitely, as for c++11, please don't file PRs to convert all old code at once 19:35:49 for the sake of doing so 19:36:09 there's no *need* for any compatibility with c++11 anymore 19:36:12 ie use c++17 features in net processing/wallet/etc, but try to keep consensus code on c++11 for longer 19:36:59 given that apparently compilers have bugs it might make sense to be more conservative with upgrading to the latest features in consensus 19:37:06 I think that will make things really complicated, even defining what is 'consensus code' is pretty hard right now 19:37:12 but it's just a half-baked thought 19:37:41 wumpus: right. It'd be nice if that separation were clearer 19:37:46 or do you only mean 'what is part of libconsensus' 19:38:01 dongcarl's libbitcoinkernel is a nice idea but it's only an idea right now 19:38:50 libbitcoinkernel? no hits on google 19:38:51 yes, it'd be nice but definitely not around the corner, I don't think it's in any shape to make C++ version depend on it 19:39:12 agree with regard to bugs though 19:39:16 almost all of C++17 is in GCC 7, and all of it is in GCC 8 19:39:18 everything is broken let' go shopping 19:39:50 so i'd agree as far as being conservative w.r.t. C++ features in consensus-critical code, but i don't think that's necessarily a new idea 19:39:56 we always want to be conservative there 19:40:15 i don't think we need a "this subset of the code must remain c++11 compatible" rule 19:40:29 whether enforced or not 19:40:44 I really thin kwe should make one decision for the entire codebase 19:40:53 agree 19:41:07 if the recent discovery means that new versions of C++ are unsafe, that means, let's just give up on C++17 for now 19:41:17 i don't think so 19:41:24 I don't particularly want the P2P code to be unsafe either 19:41:31 it's a bug in a C89 feature FFS 19:41:34 sipa: does it make sense to codify what you mean by 'conservative w.r.t c++ features in consensus-critical code'? 19:42:00 we don't have suffiient separateion of consensus-critical code from anything 19:42:03 we need to have that, sure 19:42:32 but it's more than you'd expect if you include util and compat and other indirect dependencies 19:42:58 jnewbery: yeah, i'm trying to think what that would mean 19:43:01 it's just a thought 19:43:26 honestly I think this is completely seperate from the c++17 question 19:43:32 yes, agree 19:43:36 yes, it would be good to isolate the consensus code 19:43:54 then again this was a project since, 2012 or so... 19:44:25 also: leveldb is part of consensus 19:44:29 i think the question for C++17 or not just depends on whether we expect build environments that people will want to use for 0.22 support it sufficiently or not 19:45:00 it's not like you can build the consensus code separately anyway 19:45:16 indeed 19:45:38 we *could* say that libconsensus needs to remain C++11 buildable - but i don't think there is much of a reason for that 19:46:05 I don't think so either 19:46:33 c++17 is already three years old anyway and most c++ compilers implemented it, or features from it, before that 19:46:46 it's not that we're super fast in adopting new c++ standards 19:46:48 so let's discuss this after 0.21 branch off, whether we think requiring a C++17 build environment will be a problem by the time 0.22 gets released 19:47:07 indeed 19:47:08 sipa: +1 19:47:32 to summarize, if I may: before feature freeze in one week, please review 19953 (BIPs 340-342), 19954 (tor v3), 19988 (tx relay logic), and 19077 (sqlite wallet) 19:47:40 seems deviating from our plan here on last minute does need a very good reason though 19:47:52 I am interested in how we judge 'conservative w.r.t c++ features', but not necessarily now 19:48:33 wumpus: yeah 19:48:34 I think we need to be conservative in making changes to the consensus code, not so much specifically regarding c++ features 19:48:39 jnewbery: i think that was just a subset of "conservative in general" 19:48:54 right 19:49:15 which was also my first reply, don't change code for c++17 for the sake of using c++17 19:49:21 and perhaps avoid features that reviewers may not be very familiar with - which is correlated but not the same as recently-introduced language features 19:49:49 any other topics? 19:50:51 jonatack: +1 19:52:11 honestly I'd feel a lot better if we had focused on isolating the consensus-critical code a long time ago, seems like something people like to talk about but it never really panned out yet 19:52:30 wumpus: it's... hard 19:52:52 sipa: yes :/ 19:52:56 jonatack: agree! 19:54:09 wumpus: we're moving in that direction ... slowly. #20049 and #20050 are next 19:54:09 sipa: it's hard but maybe one of the things remaining that's really worth doing 19:54:10 https://github.com/bitcoin/bitcoin/issues/20049 | De-globalizing ChainstateManager · Issue #20049 · bitcoin/bitcoin · GitHub 19:54:12 https://github.com/bitcoin/bitcoin/issues/20050 | validation: Prune (in)direct g_chainman usage related to ::LookupBlockIndex (bundle 1) by dongcarl · Pull Request #20050 · bitcoin/bitcoin · GitHub 19:54:50 jnewbery: I guess the main problem is that isolating the consensus code means changes to the consensus code which is a risk in itself so hard to do 19:55:44 jnewbery: but good to know! 19:55:48 wumpus: also that we want to switch between non-consensus and consensus code efficiently in lots of places 19:56:34 yes, and given previous attempts at refactoring out consensus code ended us more than once is a worse half-baked state, it's harder to get reviewer enthousiasm for such changes 19:56:50 aj: yes defining an interface that makes sens in itself but doesn't make things a lot slower is another thing 19:57:03 Definitely non-zero risks, which is why the focus should be on doing it incrementally, and testing it continuously :-) 19:57:36 definitely more interestedi n it than discussions about the icon color though :-) 19:57:45 Hehe 19:57:47 haha 19:57:50 hehe 19:57:54 it is as BIP42 predicted 19:58:09 "Prominent among them is the discussion on what to call 1 billion Bitcoin, which symbol color to use for it, and when wallet clients should switch to it by default. " 19:58:33 heh! 19:59:20 #endmeeting