< bitcoin-git> [bitcoin] bliotti opened pull request #19205: script: previous_release.sh rewritten in python (master...previous-release-py) https://github.com/bitcoin/bitcoin/pull/19205
< phantomcircuit> currently the ScanForWalletTransactions function handles keypool TopUp implicitly, since it's checking every output with the Solver, i'd like to have the wallet set a flag when the keypool has been topped up and just do the entire scan again until it's not topping up anymore
< phantomcircuit> for the current logic this wouldn't be good, but using block filter indexes with per node siphash parameters it wouldn't be that bad
< phantomcircuit> doing this would make handling having different sets of filters much easier, that way i can just as the chain implementation for blocks it thinks match against a list of script pubkeys
< phantomcircuit> instead of having all the chain walking logic in the wallet
< phantomcircuit> MarcoFalke, ^ any opinion?
< phantomcircuit> also i noticed that absolutely everything that calls ScanForWalletTransactions looks up the block hash by height just before calling it
< sipa> phantomcircuit: if it's through CChain, that's O(1) thankfully
< phantomcircuit> sipa, yeah just saying there's no reason for it to have the weird block_hash/block_height dual argument
< sipa> ah
< phantomcircuit> it also simplifies the handling of the reorg stuff if the first block can't be a reorg
< luke-jr> trying to think of a case where you'd want to use a non-main-chain block, but can't :/
< phantomcircuit> yeah neither can i
< bitcoin-git> [bitcoin] fanquake pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/b3091b2be7d1...807b9f811479
< bitcoin-git> bitcoin/master fa16e78 MarcoFalke: build: Add -Wshadow-field
< bitcoin-git> bitcoin/master fac6b9b MarcoFalke: test: Avoid overwriting the NodeContext member of the testing setup
< bitcoin-git> bitcoin/master 807b9f8 fanquake: Merge #19188: test: Avoid overwriting the NodeContext member of the testin...
< bitcoin-git> [bitcoin] fanquake merged pull request #19188: test: Avoid overwriting the NodeContext member of the testing setup [-Wshadow-field] (master...2006-testNoShadowField) https://github.com/bitcoin/bitcoin/pull/19188
< TheHoliestRoger> sipa can i pm you?
< promag> Can I request a gitian build for #18790?
< gribble> https://github.com/bitcoin/bitcoin/issues/18790 | gui: Improve thread naming by hebasto · Pull Request #18790 · bitcoin/bitcoin · GitHub
< fanquake> promag: done
< promag> ty!
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/807b9f811479...399a0d9dc7a1
< bitcoin-git> bitcoin/master 1a9ef1d Hennadii Stepanov: refactor: Replace RecursiveMutex with Mutex in Shutdown()
< bitcoin-git> bitcoin/master 399a0d9 MarcoFalke: Merge #19180: refactor: Replace RecursiveMutex with Mutex in Shutdown()
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19180: refactor: Replace RecursiveMutex with Mutex in Shutdown() (master...200605-shutdown) https://github.com/bitcoin/bitcoin/pull/19180
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/399a0d9dc7a1...8496dbeba687
< bitcoin-git> bitcoin/master 78c8f4f Hennadii Stepanov: refactor: Replace RecursiveMutex with Mutex in netbase.cpp
< bitcoin-git> bitcoin/master 8496dbe MarcoFalke: Merge #19190: refactor: Replace RecursiveMutex with Mutex in netbase.cpp
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19190: refactor: Replace RecursiveMutex with Mutex in netbase.cpp (master...200606-netbase) https://github.com/bitcoin/bitcoin/pull/19190
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/8496dbeba687...374fd6fc8b4c
< bitcoin-git> bitcoin/master c2410ce Hennadii Stepanov: refactor: Replace RecursiveMutex with Mutex in timedata.cpp
< bitcoin-git> bitcoin/master cc5c0d2 Hennadii Stepanov: refactor: Fix formatting of timedata.cpp
< bitcoin-git> bitcoin/master 374fd6f MarcoFalke: Merge #19189: refactor: Replace RecursiveMutex with Mutex in timedata.cpp
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19189: refactor: Replace RecursiveMutex with Mutex in timedata.cpp (master...200605-timedata) https://github.com/bitcoin/bitcoin/pull/19189
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/374fd6fc8b4c...41fb69404c03
< bitcoin-git> bitcoin/master fa2c2b5 MarcoFalke: doc: Extract net permissions doc
< bitcoin-git> bitcoin/master 41fb694 MarcoFalke: Merge #19192: doc: Extract net permissions doc
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19192: doc: Extract net permissions doc (master...2006-docNetPerm) https://github.com/bitcoin/bitcoin/pull/19192
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/41fb69404c03...9573d2b55b45
< bitcoin-git> bitcoin/master cb38b06 MIZUTA Takeshi: util: Don't reference errno when pthread fails.
< bitcoin-git> bitcoin/master 9573d2b fanquake: Merge #19194: util: Don't reference errno when pthread fails.
< bitcoin-git> [bitcoin] fanquake merged pull request #19194: util: Don't reference errno when pthread fails. (master...prototype) https://github.com/bitcoin/bitcoin/pull/19194
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #19206: test: Remove leftover comment in mining_basic (master...2006-testComment) https://github.com/bitcoin/bitcoin/pull/19206
< bitcoin-git> [bitcoin] jonatack opened pull request #19207: doc: -whitelist/-whitebind documentation improvements (master...whitelist-whitebind-doc-improvements) https://github.com/bitcoin/bitcoin/pull/19207
< bitcoin-git> [bitcoin] ycshao opened pull request #19208: test: move `sync_blocks` and `sync_mempool` functions to `test_framework.py` (master...issue-18930) https://github.com/bitcoin/bitcoin/pull/19208
< bitcoin-git> [bitcoin] MarcoFalke pushed 4 commits to master: https://github.com/bitcoin/bitcoin/compare/9573d2b55b45...b3ec1fe8114a
< bitcoin-git> bitcoin/master a9bd1f9 Danny Lee: test: warn if nodes not connected before disconnect_nodes
< bitcoin-git> bitcoin/master e6e7abd Danny Lee: test: remove redundant two-way disconnect_nodes calls
< bitcoin-git> bitcoin/master 34e641a Danny Lee: test: Remove unnecessary disconnect_nodes call in rpc_psbt.py
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #18890: test: disconnect_nodes should warn if nodes were already disconnected (master...disconnect-nodes) https://github.com/bitcoin/bitcoin/pull/18890
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/b3ec1fe8114a...3e58734e55ee
< bitcoin-git> bitcoin/master a9d28af Hennadii Stepanov: qt: Display warnings as rich text
< bitcoin-git> bitcoin/master 3e58734 MarcoFalke: Merge #18898: gui: Display warnings as rich text
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #18898: gui: Display warnings as rich text (master...200506-no-html) https://github.com/bitcoin/bitcoin/pull/18898
< bitcoin-git> [bitcoin] hebasto opened pull request #19210: qt: Get rid of cursor in out-of-focus labels (master...200608-cursor) https://github.com/bitcoin/bitcoin/pull/19210
< bitcoin-git> [bitcoin] hebasto closed pull request #14810: qt: Enable tabbing through labels (master...20181124-tab-through-labels) https://github.com/bitcoin/bitcoin/pull/14810
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/3e58734e55ee...297466b49ff3
< bitcoin-git> bitcoin/master 3e44210 Rod Vagg: Expose txinwitness for coinbase in JSON form
< bitcoin-git> bitcoin/master 34645c4 Rod Vagg: Test txinwitness is accessible on coinbase vin
< bitcoin-git> bitcoin/master 297466b MarcoFalke: Merge #18826: Expose txinwitness for coinbase in JSON form from RPC
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #18826: Expose txinwitness for coinbase in JSON form from RPC (master...rvagg/txinwitness-for-coinbase) https://github.com/bitcoin/bitcoin/pull/18826
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/297466b49ff3...a79bca2f1fb2
< bitcoin-git> bitcoin/master b00266f Sebastian Falbesoner: refactor: replace pointers by references within tx_verify.{h,cpp}
< bitcoin-git> bitcoin/master a79bca2 MarcoFalke: Merge #19069: refactor: replace pointers by references within tx_verify.{h...
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19069: refactor: replace pointers by references within tx_verify.{h,cpp} (master...20200526-refactor-replace-pointers-by-refs-in-tx_verify) https://github.com/bitcoin/bitcoin/pull/19069
< vasild> MarcoFalke: jonatack: the scripts to "filter" a code coverage report: https://github.com/vasild/filter_coverage
< vasild> I also generate an additional report with just the modified and not covered lines: https://people.freebsd.org/~vd/pr19031_coverage_of_modified_code/modified_and_not_covered.html to ease finding them (some source files may begin with 1000s of lines not touched by a change and scrolling to find "bright" lines is not covenient)
< jonatack> thanks vasild, will ping you if questions
< vasild> ok :)
< luke-jr> MarcoFalke: #12677 is on my todo list
< gribble> https://github.com/bitcoin/bitcoin/issues/12677 | RPC: Add ancestor{count,size,fees} to listunspent output by luke-jr · Pull Request #12677 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] hebasto opened pull request #19213: refactor: Replace RecursiveMutex with Mutex in Get{Data,Blocks}Dir() (master...200608-path-mx) https://github.com/bitcoin/bitcoin/pull/19213
< harding> wumpus: FYI, I've heard from someone that they can't verify the releases because they have "weak-digest sha1" in their gpg.conf (based on a recommendation they heard from petertodd) and the release signing key is self-signed using sha1. I explained the situation to them, but long-term I think that's a reasonable option for people to be setting and so maybe it's time to update the release key to use a stronger digest.
< bitcoin-git> [bitcoin] sipa opened pull request #19214: Auto-detect SHA256 implementation in benchmarks (master...202006_hw_sha_bench) https://github.com/bitcoin/bitcoin/pull/19214
< wumpus> harding: sure, do you perhaps know how to do this?
< harding> wumpus: I tried doing it several years ago, but I couldn't figure out how and ended up just creating a new key. :-(
< wumpus> hmm generating a new release keys seems much worse
< harding> wumpus: yeah, agreed. You can still sign the new release keys with the old release keys to create a clear trust path, but it's probably worth waiting to see if anyone reading this knows a way to just upgrade the existing key.
< wumpus> looks like it should just be "gpg --cert-digest-algo sha512 --expert --edit-key", then "sign", going to try...
< harding> wumpus: if that works and if you figured it out from just reading the manual page or doing a quick web search, I'm going to have hang my head in shame because I know I spent more than an hour researching this a couple years ago.
< phantomcircuit> sipa, no idea why the auto detect is actually slower
< phantomcircuit> definitely running a gcc old enough that the asm should beat the naive implementation significantly
< sipa> phantomcircuit: if you run bitcoind, what gets reported in debug.log?
< sipa> (it tells you which implementation is selected)
< sipa> in one of the first output lines
< sipa> 2020-03-15T20:46:14.743702Z Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation
< phantomcircuit> Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation
< sipa> any particular configure flags or CXXFLAGS?
< phantomcircuit> --with-incompatible-bdb --without-gui
< sipa> very surprising
< sipa> and the sse4 code is actually assembly, not intrinsics
< sipa> so a compiler can't be fucking that up
< phantomcircuit> hmm i wonder if im having build issues one sec
< phantomcircuit> it is actually `using sse4(1way),sse41(4way),avx2(8way)` in the benchmark too
< sipa> only the 1-way code matters for the benchmarks you're listing
< sipa> how does the merkle root benchmark compare?
< wumpus> harding: well I did that and pushed my re-signed key to the keyservers, hope it worked (no idea how to test)
< sipa> -filter=MerkleRoot
< phantomcircuit> it's slower too
< phantomcircuit> bench_master.txt:MerkleRoot, 5, 800, 4.61105, 0.00114375, 0.00117359, 0.00114912
< phantomcircuit> bench_sipa.txt:MerkleRoot, 5, 800, 4.71484, 0.00116899, 0.00119543, 0.00117554
< wumpus> harding: (well I can verify bitcoin core SHA256SUMS.asc using "weak-digest sha1", but haven't ever seen it fail)
< sipa> wth
< sipa> phantomcircuit: that's ridiculous
< sipa> it should be 5x faster
< harding> wumpus: It's failed for me with weak-digest sha1, so I'll try testing now.
< phantomcircuit> sipa, it's a laptop cpu possibly the sse4/avx2 instructions are just comically broken?
< sipa> phantomcircuit: but your numbers are fast
< sipa> unreasonably fast for the naive implementation
< sipa> MerkleRoot, 5, 800, 42.6441, 0.0106318, 0.0107271, 0.0106534
< sipa> ^ that's on my laptop with naive SHA256
< harding> wumpus: did you upload to keyserver.ubuntu.com? I still get a failure after running --refresh-keys from there.
< wumpus> "gpg --fingerprint --check-sigs 0x90C8019E36C2E964" does show two self-signatures
< wumpus> of course it doesn't show the algorithm used
< phantomcircuit> i mean i just did `git clean -fdx && git checkout master && ./autogen.sh && ./configure --with-incompatible-bdb --without-gui && make -j4 && ./src/bench/bench_bitcoin --filter='(SHA256|Merkle).*'` and got the same result for master
< phantomcircuit> which i see as b1b173994406158e5faa3c83b113da9d971ac104
< harding> wumpus: ah, now it works for me and my friend also says it works. Thanks!
< harding> (I had to request the specific key with --refresh-keys)
< phantomcircuit> i'll clear ccache as well lets try this agian...
< wumpus> "You can validate signatures are using non-SHA1 by using gpg --list-packets < SIGFILE, and checking that digest algo (aka hash algorithm) is not "2" (SHA1). For SHA512, you'd want "10" (see RFC4880 section 9.4 for details): " of course list-packets, of course
< sipa> haha
< harding> haha
< wumpus> harding: great! thanks for checking
< sipa> the gpg binary format is not too hard
< sipa> it's probably easier to learn to read a hexdump than to learn gpg's command line options
< wumpus> :-)
< sipa> i have edited gpg files using hd, head & tail
< sipa> phantomcircuit: i'm baffled
< phantomcircuit> it's gonna end up being some insane ccache issue except my original master benchmark is before i had even checked out the pr
< phantomcircuit> so uh
< * phantomcircuit> shrugs
< sipa> ok, i'm going to run this:
< sipa> $ ./autogen.sh && ./configure --with-incompatible-bdb --without-gui && make -j9 -C src bench/bench_bitcoin && ./src/bench/bench_bitcoin --filter='(SHA256|Merkle).*'
< sipa> with a git clean -dfx before it
< sipa> this is on a i7-7820HQ fwiw
< phantomcircuit> running the exact same command against master now
< phantomcircuit> sipa, i think your cpu should generally be faster
< wumpus> sipa: wow! I've never editited gpg files that low-level, did apparently mess around with gpgsplit at some point to turn an attached signature into a detached signature https://github.com/bitcoin-core/gitian.sigs/blob/master/scripts/extract-sig.py
< sipa> phantomcircuit: except your *naive* numbers are faster than my sse4 numbers!
< phantomcircuit> sipa, yeah im confused to
< sipa> and for the 1-way sse4 code, i'd believe it that an implementation may be sufficiently broken that it offers no gain
< sipa> but for the 8-way AVX2 ode to be effectively 8x slower than naive seems a severe stretch
< sipa> wumpus: i had to make the same edit to many different files, but it was the same byte range for all
< phantomcircuit> sipa, and now the numbers make sense, master is MerkleRoot, 5, 800, 26.4366, 0.00647519, 0.00678599, 0.00659338
< phantomcircuit> 19214 is MerkleRoot, 5, 800, 4.56617, 0.00113032, 0.00114694, 0.00114392
< phantomcircuit> so ??????
< phantomcircuit> i swear i didn't mix them up either
< phantomcircuit> i mean that's still kind of unreasonably fast for the naive implementation but at least they're in the right ratio?
< sipa> that looks more reasonable
< sipa> phantomcircuit: well whatever glitch happened on your CPU, if you figure it out you may want to inform intel that naive SHA256 code can beat a AVX2 parallel one :)
< phantomcircuit> the weird thing is that i did the master benchmark before i checked out the pr
< phantomcircuit> i checked my shell history to confirm that too
< phantomcircuit> sipa, oh do any of the other benchmarks maybe end up calling something that would call the auto detect?
< sipa> phantomcircuit: i don't think so
< phantomcircuit> i'd like to basically start over on ScanForWalletTransactions and split it into a call to a chain interface method that returns a list of candidate blocks and logic to (optionally, but by default) rescan if anything in the keypool changed
< phantomcircuit> as it stands the scanning logic can miss transactions if you use keypool keys out of order
< phantomcircuit> (i doubt that's ever actually happened to anybody though)
< sipa> phantomcircuit: how does that interact with work to make rescanning of all wallets happen simultaneously?
< phantomcircuit> is there a pr or an issue describing the strategy for that? (i don't see any reason it would be incompatible)
< sipa> no idea; just vaguely heard in mentioned
< luke-jr> wumpus: did you manage to re-self-sign without SHA1?
< luke-jr> wumpus: I did it a few years ago - maybe I can look up how I did it
< bitcoin-git> [bitcoin] achow101 opened pull request #19215: psbt: Include and allow both non_witness_utxo and witness_utxo for segwit inputs (master...psbt-segwit-fixes) https://github.com/bitcoin/bitcoin/pull/19215