< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/edec7f7c2542...a4a279b4f368
< bitcoin-git> bitcoin/master e60ef21 fanquake: doc: Clang 8 or later is required with FORCE_USE_SYSTEM_CLANG
< bitcoin-git> bitcoin/master a4a279b fanquake: Merge #19617: doc: Clang 8 or later is required with FORCE_USE_SYSTEM_CLAN...
< bitcoin-git> [bitcoin] fanquake merged pull request #19617: doc: Clang 8 or later is required with FORCE_USE_SYSTEM_CLANG (master...note_that_clang_8_is_required_with_force_system_clang) https://github.com/bitcoin/bitcoin/pull/19617
< bitcoin-git> [bitcoin] Empact opened pull request #19632: test: Catch decimal.InvalidOperation from TestNodeCLI#send_cli (master...2020-07-send_cli-invalid-op) https://github.com/bitcoin/bitcoin/pull/19632
< bitcoin-git> [bitcoin] kallewoof closed pull request #14582: wallet: always do avoid partial spends if fees are within a specified range (master...181026-try-avoidpartialspends) https://github.com/bitcoin/bitcoin/pull/14582
< bitcoin-git> [bitcoin] kallewoof closed pull request #16653: script: add simple signature support (checker/creator) (master...2019-08-genpursigs) https://github.com/bitcoin/bitcoin/pull/16653
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/a4a279b4f368...a63a26f04213
< bitcoin-git> bitcoin/master ae4958b Chris L: rpc: RPCResult Type of MempoolEntryDescription should be OBJ. If multiple ...
< bitcoin-git> bitcoin/master a63a26f MarcoFalke: Merge #19585: rpc: RPCResult Type of MempoolEntryDescription should be OBJ...
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19585: rpc: RPCResult Type of MempoolEntryDescription should be OBJ. (master...19579) https://github.com/bitcoin/bitcoin/pull/19585
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #19098: test: Remove duplicate NodeContext hacks (master...pr/qtx) https://github.com/bitcoin/bitcoin/pull/19098
< bitcoin-git> [bitcoin] MarcoFalke reopened pull request #19098: test: Remove duplicate NodeContext hacks (master...pr/qtx) https://github.com/bitcoin/bitcoin/pull/19098
< MarcoFalke> The meeting could toggle between two times, so everyone could at least attend every other meeting
< jonatack> i'm interested in these meetings (core dev weekly, wallet, p2p, review club) but they are currently during sleep hours, which made for grouchy participation and low productivity the next day... i concluded it's best to abstain ;)
< jonatack> I like MarcoFalke's suggestion to toggle between two times
< jonatack> at any rate, I read the logs to catch up the next day :)
< bitcoin-git> [bitcoin] fanquake pushed 8 commits to master: https://github.com/bitcoin/bitcoin/compare/a63a26f04213...f7c73b03d975
< bitcoin-git> bitcoin/master d362f19 Pieter Wuille: doc: list support for BIP 339 in doc/bips.md
< bitcoin-git> bitcoin/master 9efd86a Pieter Wuille: refactor: add GenTxid (=txid or wtxid) type and use it for tx request logic
< bitcoin-git> bitcoin/master 900d7f6 Pieter Wuille: p2p: enable fetching of orphans from wtxid peers
< bitcoin-git> [bitcoin] fanquake merged pull request #19569: Enable fetching of orphan parents from wtxid peers (master...202007_wtxid_followup) https://github.com/bitcoin/bitcoin/pull/19569
< jnewbery> I'm pretty against having an alternating meeting time for a couple of reasons:
< jnewbery> 1) it's confusing. People will inevitably show up at the wrong time on the wrong day or miss meetings and get annoyed.
< jnewbery> 2) I'd need to find a host for the opposite time meeting. In my experience from PR review club, people are keen to host one or two and then lose interest.
< bitcoin-git> [bitcoin] laanwj pushed tag v0.20.1: https://github.com/bitcoin/bitcoin/compare/v0.20.1
< hebasto> \o/
< bitcoin-git> [bitcoin] laanwj pushed 1 commit to 0.20: https://github.com/bitcoin/bitcoin/compare/6acb21e724e3...7ff64311bee5
< bitcoin-git> bitcoin/0.20 7ff6431 Wladimir J. van der Laan: build: Bump version to 0.20.1-final
< wumpus> yes, I think that's also why the main meeting was never changed to alternating times, even though it was considered a few times
< wumpus> btw, we don't even mention the wallet meeting yet on https://bitcoincore.org/en/meetings/, would make sense to do so, or at least have a page that mentions all the meetings in one place
< wumpus> could also link to the PR review meetings
< wumpus> and the 2018- meeting summaries could move to something like 'historical meeting summaries'
< aj> jnewbery: "people will .. miss meetings and get annoyed" oh, look, there's the box i'm in
< jonatack> jnewbery: lately there's almost been a surplus of review club hosts? slots backed up, etc, not including others who might be interested in hosting at a different time
< jonatack> anyway... in general (not just the review club) it may be worthwhile to try and see if people are confused by alternating slots.
< jonatack> the benefit might be a greater diversity of input at the meetings. geographic diversity is interesting. but i won't go on further about it.
< * troygiorshev> proposes to randomize the time each week so that people are forced to check when it is
< jonatack> :D
< provoostenator> hebasto beat me to it by 17 minutes for Gitian signatures. I should automate the PR part...
< hebasto> still using LXC :)
< provoostenator> Oh it was done much earlier; I just didn't watch the terminal window
< hebasto> provoostenator: have you already automate tagging part?
< jonatack> If anyone wants to build gitian sigs for the first time for 0.20.1, I just updated this guide: https://github.com/jonatack/bitcoin-development/blob/master/gitian-building.md
< emzy> provoostenator: I had to delete gitian-builder/cache to get the hash of MacOSX10.14.sdk.tar.gz right and compile again. But you are faster anyway.
< provoostenator> hebasto: no automation, but I highlight "pushed tag" on IRC (ht achow101)
< achow101> automation is hard
< provoostenator> Also probably not ideal to automate, in case someone kidnaps me just before an evil release
< hebasto> provoostenator :D
< hebasto> agree, automated gitian builds as well as automated client updates are not for bitcoin
< hebasto> anyway kidnapping has no point as an adversary has no ability to *sign* your gitian builds
< jnewbery> jonatack: there hasn't been a surplus of review club hosts. I'm always looking for people to volunteer to host.
< jnewbery> I think maybe you underestimate the effort required to make sure this thing runs regularly.
< jnewbery> I'm very open to other people hosting review club meetings for different time zones. My experience has been that people don't stick with it for very long.
< jonatack> jnewbery: indeed, i reckoned i was underestimating that. it may have been a one-off backlog.
< jnewbery> It's not a backlog. I schedule hosts in advance to make sure that we always have something lined up
< jonatack> jnewbery: i'm not convinced that second meetings are very exciting for people. i don't think alternate times for the main meeting have been tried.
< jnewbery> there are many people who join every week at 17:00 UTC on weds because that's part of their schedule. I'm not going to stop holding meetings at that time. I'm very open to you or anyone else holding a meeting at a time that's more suitable for other time zones though.
< jonatack> that said, i don't want to be overbearing about mono-meeting times and i'm happy with reading the logs the next day and learning from them.
< jnewbery> I think we're offtopic here. If you want to discuss more, let's use #bitcoin-core-pr-reviews
< jonatack> my point was about the diversity of input at meetings in general, not only review club.
< jnewbery> ok. I just wanted to make sure that people in this room didn't think that we have a surplus of hosts. I'm always looking for hosts, and I think everyone who's done it has found it a worthwhile use of their time.
< jonatack> +1
< jonatack> if anyone is curious, here are the people who have hosted review clubs so far: https://bitcoincore.reviews/meetings-hosts
< pinheadmz> thats cool jonatack !
< achow101> provoostenator: osx mismatch, did you clear your gitian cache?
< achow101> - 61a385c6836240224dafa06f19c0f5b9bea1215852c318fd6bd1e2f7baeb856e bitcoin-0.20.1-osx-unsigned.tar.gz
< achow101> + fdffff0e6b956ffa51b0a01f04b9e9cff2a658589116a911c322a94855feeeac bitcoin-0.20.1-osx-unsigned.tar.gz
< provoostenator> I didn't, which incantation?
< achow101> rm -r gitian-builder/cache
< provoostenator> And did my 0.20.1rc1 build also mismatch?
< achow101> yes, mismatch for rc1 too
< provoostenator> Doing a new build, will make another PR with that.
< provoostenator> Did we backport the SDK upgrade?
< provoostenator> Or is this an unrelated problem?
< achow101> unrelated
< jonatack> achow101: when is that spell needed? have never cleared the cache
< achow101> jonatack: usually it isn't needed as the cache should clear/not be used when a dependency updates
< achow101> but somtimes a dependency's binary will change because some build tooling changes and that makes cached builds vs non-cached builds differ
< achow101> so in those cases (like now) it's just easier to ask everyone to clear their cache before building so we all arrive at the same result
< jonatack> ok thanks
< provoostenator> Oh I guess I didn't have to rebuild Linux and Windows...
< bitcoin-git> [bitcoin] justinmoon opened pull request #19634: rpc: Document getwalletinfo's unlocked_until field as optional (master...getwalletinfo) https://github.com/bitcoin/bitcoin/pull/19634
< jeremyrubin> hebasto: for #18710 do you have a plan to get cross platform testing? It might help to ask some specific people to run benches. But also if you read the article that I shared on boost:: vs std:: it's unclear that our current bench would even capture it...
< gribble> https://github.com/bitcoin/bitcoin/issues/18710 | Add local thread pool to CCheckQueue by hebasto · Pull Request #18710 · bitcoin/bitcoin · GitHub
< jeremyrubin> hebasto: I think what you might want to add is a new bench which is just testing different threading primitives out, and show there's no regression there?
< jeremyrubin> I can also send the article to martinus and ask if there are any features in nanobench that can help diagnose such a regression
< hebasto> jeremyrubin: which threading primitives do you mean for benchmarking?
< provoostenator> OTOH wiping the cache changed some linux details too
< hebasto> yes, martinus' help would be valuable
< provoostenator> (but not the end result)
< sipa> figure out how to turn of cpu frequency scaling if you benchmark
< sipa> on my laptop's i7 cpu that means booting with intel_pstate=disable on the kernel command line
< hebasto> some tools are mentioned here https://wiki.archlinux.org/index.php/CPU_frequency_scaling
< jeremyrubin> from martinus:
< jeremyrubin> Hi, I think a good way to expose variance is to reduce the number of iterations. Usually nanobench tries to find a reasonable number of iterations depending on clock accuracy, but running many iterations per measurement means that it will become smoothed. So I'd do something like this:
< jeremyrubin> ankerl::nanobench::Bench().epochIterations(1).epochs(10000).run(
< jeremyrubin> "expose variance", [&] { /* whatever */ });
< jeremyrubin> that forces only a single iteration per measurement(epoch), so this maximizes variance. The disadvantage is that time measurements will be less accurate.
< jeremyrubin> To visualize the fluctiation you can generate boxplots:
< jeremyrubin> std::ofstream html("out.html");
< jeremyrubin> bench.render(ankerl::nanobench::templates::htmlBoxplot(), html);
< jeremyrubin> its probably best to play a bit with epochIterations() and epochs() argument to get reasonable visualization and result
< jeremyrubin> ^^^ back to non martinus
< jeremyrubin> this shows that std's condvar is defective
< jeremyrubin> so what im asking to test is that whichever primitive were using is not defective
< jeremyrubin> idk if it's an issue in that particular environment
< jeremyrubin> or an issue with c++11 generally
< jeremyrubin> but the checkqueue would be prett sensitive to something like this, and is perf critical
< hebasto> this article did not mention c++ library version
< sipa> "Visual Studio 2012 Express"
< jeremyrubin> so i'd prefer not leaving unless we have confidence the defect doesnt exist in our supported platforms
< sipa> i suspect is the c++ library version :)
< jeremyrubin> sipa: correct
< jeremyrubin> and the issue isnt avg perf, it's the variance
< jeremyrubin> which is why current bench doesnt inform much
< hebasto> so the goal is to compare boost:: and std:: condvar performance variances?
< jeremyrubin> fwiw there could be a legit reason to prefer the slower one -- maybe it delegates to the OS more and it's a thread priority issue
< jeremyrubin> but idk, and i'd be nervous to make a change without being sure
< jeremyrubin> hebasto: yes, in addition to actual performance
< hebasto> yes, we must be sure before making changes
< dburkett> If a block is part of the active chain, or we have the data in blk*.dat, in what cases would we not have tx data downloaded?
< jeremyrubin> hebasto: this one is just particularly annoying :/ i suspect that this issue has been fixed or doesnt exist on non windows but who knows.
< jeremyrubin> if it were only a regression on windows i'd still favor the patch
< sipa> dburkett: when it's pruned, i think
< jeremyrubin> but with condvars its both a hardware and system issue
< sipa> dburkett: ah, no
< jeremyrubin> so you should engage e.g. wumpus on what's a reasonable set to test
< sipa> dburkett: HaveTxDownloaded() means that a block and _all its parents_ have been downloaded
< dburkett> Ah, that makes sense! I knew it wasn't the case of pruned, because the docs specifically state that returning true doesn't mean txs haven't been pruned.
< dburkett> I forgot nChainTx is the number of txs in the entire chain up to that point. Makes sense now. Thanks sipa
< sipa> HAVE_BLOCK_DATA is unset when a block is pruned
< sipa> but nTx/nChainTx remain
< dburkett> Got it. There's a lot of possible states these indices can be in. It's sometimes hard for me to keep track of all of them :)
< achow101> wallet meeting?
< provoostenator> I'm around
< achow101> #startmeeting
< lightningbot> Meeting started Fri Jul 31 19:01:14 2020 UTC. The chair is achow101. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< provoostenator> hi
< achow101> #bitcoin-core-dev Wallet 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 ariard digi_james amiti fjahr
< provoostenator> Review nag for #15382 which is hopefully pretty close
< gribble> https://github.com/bitcoin/bitcoin/issues/15382 | util: add RunCommandParseJSON by Sjors · Pull Request #15382 · bitcoin/bitcoin · GitHub
< achow101> that's on my review queue
< achow101> the main sqlite wallet pr is now ready for review
< provoostenator> Nice, will move on to that after I'm done with a Taproot excursion
< achow101> I was also wondering whether we wanted to do #19619 before it though
< gribble> https://github.com/bitcoin/bitcoin/issues/19619 | Remove wallet.dat path handling from wallet.cpp, rpcwallet.cpp by ryanofsky · Pull Request #19619 · bitcoin/bitcoin · GitHub
< achow101> no topics today?
< provoostenator> Not much news from my end
< provoostenator> 0.20.1 has the PSBT change, that's all we needed there?
< achow101> yes
< provoostenator> sipa: when miniscript?
< achow101> also #19602 is legacy to descriptor wallet migration
< gribble> https://github.com/bitcoin/bitcoin/issues/19602 | wallet: Migrate legacy wallets to descriptor wallets by achow101 · Pull Request #19602 · bitcoin/bitcoin · GitHub
< provoostenator> Shouldn't that go in the wallet tool?
< provoostenator> At least until we really insist on an upgrade?
< achow101> it could
< achow101> i made it a separate RPC, but it could also be rolled into upgradewallet
< provoostenator> And even if we insist on an upgrade, only the GUI would need access to this code, client users can use wallettool
< achow101> sure
< achow101> however it is useful to not have to unload and reload a wallet in order to migrate it
< achow101> #endmeeting
< lightningbot> Meeting ended Fri Jul 31 19:12:31 2020 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< bitcoin-git> [bitcoin] Saibato opened pull request #19635: param: add bool parameter -ephemeraltoronion to generate ephemeral tor addreses (master...ephemeral-tor-onion) https://github.com/bitcoin/bitcoin/pull/19635
< jonasschnelli> 0.20.1 detached signatures are up and tagged
< achow101> gitian builders ^
< meshcollider> Oh no totally forgot the meeting this morning! Thanks achow101
< meshcollider> Looks like a short one anyway
< meshcollider> 12:57 AM <aj> jnewbery: "people will .. miss meetings and get annoyed" oh, look, there's the box i'm in
< meshcollider> :)
< jeremyrubin> sipa: is this proven: not exists tx tx2. txid(tx) == wtxid(tx2) && tx != tx2
< sipa> jeremyrubin: yes, assuming double-sha256 collision resistance that statement is equivalent to ser_without_witness(tx) == ser_with_witness(tx2)
< sipa> either both tx and tx2 don't have a witness, and the statement is obviously false
< sipa> or tx2 has a witness, in which case the serialization definitely differs, due to the flag byte
< jeremyrubin> but it's not possible in tx2 that the flag + marker can be read as the length field for the txins?
< jeremyrubin> because a leading 0 byte means it can't be a length right?
< jeremyrubin> in which case it would be marker and not flag that guarantees uniqueness?
< sipa> a valid transactions cannot have 0 inputs
< sipa> the witness serialization is intended to be unambiguously different from the old serialization
< sipa> (except that we later discovered that people do pass around transactions with 0 inputs, where things break... but that's not relevant for valid network transactions)(
< jeremyrubin> gotcha. and the unambiguity is because it's a compactsize field preceding the vIn
< jeremyrubin> so 0 is always read as a 1 byte number in any context
< sipa> indeed
< harding> wumpus: FYI, 0.20.1 website PR: https://github.com/bitcoin-core/bitcoincore.org/pull/711
< bitcoin-git> [bitcoin] achow101 opened pull request #19636: doc: Update 0.20.1 release notes with psbt changes (0.20...0.20.1-psbt-relnotes) https://github.com/bitcoin/bitcoin/pull/19636
< luke-jr> oops? too late?