< bitcoin-git>
[bitcoin] sanket1729 opened pull request #20100: split policy/error consensus codes for CLEANSTACK, MINAMALIF (master...minimal_if) https://github.com/bitcoin/bitcoin/pull/20100
< bitcoin-git>
[bitcoin] achow101 opened pull request #20101: rpc: change no wallet loaded message to be clearer (master...better-no-wallet-error) https://github.com/bitcoin/bitcoin/pull/20101
< achow101>
gwillen: ^
< wumpus>
the release notes should not be considered documentation, and it's good if the program is self-contained, I'd greatly prefer linking to some actual documentation instead-like the help text of options or other RPC calls
< bitcoin-git>
bitcoin/master 9dd4de2 MarcoFalke: Merge #20027: Use mockable time everywhere in net_processing
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #20027: Use mockable time everywhere in net_processing (master...202009_mockable_netproc) https://github.com/bitcoin/bitcoin/pull/20027
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #20103: test: Enable mocktime RPC for all test chains (master...2010-testMockAllChains) https://github.com/bitcoin/bitcoin/pull/20103
< wumpus>
but no, there is no precedent for help options that are not RPC, and I'm not sure that's a good idea to introduce, it feels to me a kind of cluttery to use RPC help for that, it does reveal our lack of a more general user documentation. As a workaround isn't there some existing RPC where is would be appropriate?
< wumpus>
to be honest I think that error message is OK
< wumpus>
it's long but it actually tells something useful
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #20106: cirrus: Use kvm to avoid spurious CI failures in the default virtualization cluster (master...2010-ciOtherVirt) https://github.com/bitcoin/bitcoin/pull/20106
< dongcarl>
vasild, sipa: I might be misremembering, but I recall having a discussion about the versionspace of serializers/unserializers and how we should separate them for different types?
< dongcarl>
Oh I see that's still being worked on in #19503
< wumpus>
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
< sipa>
all todos are done for taproot, including the json tests in the qa-assets repo
< wumpus>
jonatack: so that one needs 0.21 milestone?
< wumpus>
aj: seems like it, but also seems not urgent for 0.21?
< wumpus>
aj: oh "This needs to happen before the next major release. Otherwise, it will be a breaking change."
< aj>
wumpus: yeah
< wumpus>
ping MarcoFalke
< sipa>
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
< sipa>
(the code, not activation obviously)
< wumpus>
hebasto: done
< wumpus>
sipa: agree!
< jonatack>
+1
< meshcollider>
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
< gribble>
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
< meshcollider>
But it's a big PR
< sipa>
i haven't paid attention to the PR itself, but it seems it's been making lots of progress lately, including review
< meshcollider>
Yeah it's had a decent amount of review already so it's not infeasible
< sipa>
i know wumpus had reservations about having it in 0.21
< wumpus>
looks like we're starting to run into issues building bdb 4.8 on some platforms at least: #19411
< wumpus>
sipa: yes, it seems fairly risky to introduce a new wallet database format (which is used by default) in something last minute
< sipa>
--with-incompatible-bdb lalala
< achow101>
wumpus: it's only used by descriptor wallets which we already have marked as "experimental"
< wumpus>
achow101: they're not created by default for new users?
< achow101>
no
< wumpus>
in that case I"m oay with it
< sipa>
ah, descriptor wallets are not default for new wallets?
< achow101>
not yet
< sipa>
that's perhaps the best of both worlds... get descriptor wallets and sqlite in 0.21, but neither is default
< wumpus>
if it's opt-in I see no problem
< achow101>
if descriptor wallets were the default, a lot of tests would be failing :p
< wumpus>
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
< aj>
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
< wumpus>
0.21 introduces c++17 compilation by default
< jnewbery>
oops sorry, yes v0.22
< wumpus>
0.22 is c++17 only
< wumpus>
okay
< jnewbery>
so after v0.21 branch we're theoretically free to use c++17 features throughout the codebase
< wumpus>
yes
< jnewbery>
My question was whether it makes sense to try to keep c++11 compatability for consensus code for longer
< wumpus>
for new code definitely, as for c++11, please don't file PRs to convert all old code at once
< wumpus>
for the sake of doing so
< wumpus>
there's no *need* for any compatibility with c++11 anymore
< jnewbery>
ie use c++17 features in net processing/wallet/etc, but try to keep consensus code on c++11 for longer
< jnewbery>
given that apparently compilers have bugs it might make sense to be more conservative with upgrading to the latest features in consensus
< wumpus>
I think that will make things really complicated, even defining what is 'consensus code' is pretty hard right now
< jnewbery>
but it's just a half-baked thought
< jnewbery>
wumpus: right. It'd be nice if that separation were clearer
< wumpus>
or do you only mean 'what is part of libconsensus'
< wumpus>
dongcarl's libbitcoinkernel is a nice idea but it's only an idea right now
< aj>
libbitcoinkernel? no hits on google
< wumpus>
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
< wumpus>
agree with regard to bugs though
< sipa>
almost all of C++17 is in GCC 7, and all of it is in GCC 8
< wumpus>
everything is broken let' go shopping
< sipa>
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
< sipa>
we always want to be conservative there
< sipa>
i don't think we need a "this subset of the code must remain c++11 compatible" rule
< sipa>
whether enforced or not
< wumpus>
I really thin kwe should make one decision for the entire codebase
< sipa>
agree
< wumpus>
if the recent discovery means that new versions of C++ are unsafe, that means, let's just give up on C++17 for now
< sipa>
i don't think so
< wumpus>
I don't particularly want the P2P code to be unsafe either
< sipa>
it's a bug in a C89 feature FFS
< jnewbery>
sipa: does it make sense to codify what you mean by 'conservative w.r.t c++ features in consensus-critical code'?
< wumpus>
we don't have suffiient separateion of consensus-critical code from anything
< wumpus>
we need to have that, sure
< wumpus>
but it's more than you'd expect if you include util and compat and other indirect dependencies
< sipa>
jnewbery: yeah, i'm trying to think what that would mean
< sipa>
it's just a thought
< wumpus>
honestly I think this is completely seperate from the c++17 question
< sipa>
yes, agree
< wumpus>
yes, it would be good to isolate the consensus code
< wumpus>
then again this was a project since, 2012 or so...
< wumpus>
also: leveldb is part of consensus
< sipa>
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
< wumpus>
it's not like you can build the consensus code separately anyway
< sipa>
indeed
< sipa>
we *could* say that libconsensus needs to remain C++11 buildable - but i don't think there is much of a reason for that
< wumpus>
I don't think so either
< wumpus>
c++17 is already three years old anyway and most c++ compilers implemented it, or features from it, before that
< wumpus>
it's not that we're super fast in adopting new c++ standards
< sipa>
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
< sipa>
indeed
< jnewbery>
sipa: +1
< jonatack>
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)
< wumpus>
seems deviating from our plan here on last minute does need a very good reason though
< jnewbery>
I am interested in how we judge 'conservative w.r.t c++ features', but not necessarily now
< sipa>
wumpus: yeah
< wumpus>
I think we need to be conservative in making changes to the consensus code, not so much specifically regarding c++ features
< aj>
jnewbery: i think that was just a subset of "conservative in general"
< wumpus>
right
< wumpus>
which was also my first reply, don't change code for c++17 for the sake of using c++17
< sipa>
and perhaps avoid features that reviewers may not be very familiar with - which is correlated but not the same as recently-introduced language features
< wumpus>
any other topics?
< sipa>
jonatack: +1
< wumpus>
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
< sipa>
wumpus: it's... hard
< wumpus>
sipa: yes :/
< wumpus>
jonatack: agree!
< jnewbery>
wumpus: we're moving in that direction ... slowly. #20049 and #20050 are next
< wumpus>
sipa: it's hard but maybe one of the things remaining that's really worth doing
< gribble>
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
< wumpus>
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
< wumpus>
jnewbery: but good to know!
< aj>
wumpus: also that we want to switch between non-consensus and consensus code efficiently in lots of places
< sipa>
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
< wumpus>
aj: yes defining an interface that makes sens in itself but doesn't make things a lot slower is another thing
< dongcarl>
Definitely non-zero risks, which is why the focus should be on doing it incrementally, and testing it continuously :-)
< wumpus>
definitely more interestedi n it than discussions about the icon color though :-)
< dongcarl>
Hehe
< sipa>
haha
< jonatack>
hehe
< sipa>
it is as BIP42 predicted
< sipa>
"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. "
< wumpus>
heh!
< wumpus>
#endmeeting
< lightningbot>
Meeting ended Thu Oct 8 19:59:20 2020 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)