< bitcoin-git> [bitcoin] jimmysong opened pull request #10219: Tests: Order Python Tests By Duration (master...order_tests_by_duration) https://github.com/bitcoin/bitcoin/pull/10219
<@wumpus> don't think this will need a long testing time, given the minor changes relative to rc1
<@wumpus> is it just me or is test_runner.py really slow now on master?
<@wumpus> huh only happens on one vm, must be me
<@wumpus> what I don't understand is that this is a much faster machine just much slower at running the tests
< sipa> slow i/9?
< sipa> i/o?
< MarcoFalke> we should really collect statistics about the tests such as run time and then plot them over time.
< MarcoFalke> I think we had a regression in the wallet once that made the wallet test run really slow...
<@wumpus> fast/old computer: 3.98user 1.26system 0:33.90elapsed 15%CPU (0avgtext+0avgdata 57976maxresident)k 8inputs+42776outputs (0major+107146minor)pagefaults 0swaps
<@wumpus> slow/new computer: 7.29user 2.48system 1:42.07elapsed 9%CPU (0avgtext+0avgdata 57904maxresident)k 0inputs+45776outputs (0major+108208minor)pagefaults 0swaps
<@wumpus> (both for fundrawtransaction test)
<@wumpus> to me it looks like it's just spending more time waiting
< MarcoFalke> tearing down the nodes takes ages for tiny tests compared to the test's actual run time
< gmaxwell> unfortunately with the test running on shared vm infrastructure timings are probably not all that useful.
<@wumpus> it probably doesn't help rpc performance from python that authproxy calls log.debug for all data that comes in and goes out, pretty-printing everything even though usually it's discarded
<@wumpus> (not likely the cause of my slowdown, just noticed)
< MarcoFalke> Indeed, no wall clock, but maybe cpu_time, memory_peak and io could help.
<@wumpus> I'm on to something maybe, a getnewaddress call takes 0.013565 on the one system, 0.168683 (more than ten times as much) on the other. Could be slow i/o, but that slow?
<@wumpus> this does not seem to extend to most other RPC calls (though havne't looked at them all)
< gmaxwell> well getnewaddress is syncing the wallet... so fsync time?
< sipa> we do a db sync operatiom after every new address
< sipa> jinx
<@wumpus> if so we need a flag to disable that for the tests
<@wumpus> fsync slow makes sense, I've noticed that before, I think it tries to sync the entire partition image
< gmaxwell> there is that eatmydata thing that could be used with tests.
<@wumpus> cool, didn't know about that one
< sipa> s/partition/filesystem
< sipa> i think? or is literally the disk block cache?
<@wumpus> yes, filesystem
<@wumpus> or not sure really
<@wumpus> it might as well be trying to sync the entire virtual disk to disk
< sipa> but syncing of a filesystem needs dependency information between sectors, or you may end up with an inonsistent state
<@wumpus> I suspect it's something like that at least: fsync() inside the VM has file granularity, but qemu calling fsync() has complete file system granularity
< sipa> so even if it's a disk level cache, it needs information from the filesystem to order the wrotes
<@wumpus> so not only the wallet is fsynced, but also all the other things the tests do such as writing tons of log files
<@wumpus> anyhow I'll try with the eatmydata and see if it resolves the slowdown
<@wumpus> yes!
<@wumpus> old computer: 3.73user 1.04system 0:39.09elapsed 12%CPU (0avgtext+0avgdata 61112maxresident)k 8inputs+9064outputs (0major+45873minor)pagefaults 0swaps
<@wumpus> new computer: 3.24user 0.78system 0:33.70elapsed 11%CPU (0avgtext+0avgdata 57832maxresident)k 0inputs+10472outputs (0major+108842minor)pagefaults 0swaps
<@wumpus> more than three times as fast as before
<@wumpus> wonder if this will help travis too
< bitcoin-git> [bitcoin] laanwj opened pull request #10220: Experiment: test: Disable fsync in travis tests (master...2017_04_tests_eatmydata) https://github.com/bitcoin/bitcoin/pull/10220
<@wumpus> so apparently on travis, eatmydata gives a 2x gain (thanks for testing MarcoFalke), not as good as in my VM (I may have some misconfiguration) but still nice
< sipa> nice indeed!
< SopaXorzTaker> offtopic PSA
< SopaXorzTaker> wumpus, sipa, the large bitcoin collider client script is untrustworthy
< SopaXorzTaker> refrain from running it until the author gives explanations
< sipa> don't worry, i had no intention of running it
< SopaXorzTaker> just FYI
< bitcoin-git> [bitcoin] sipa pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/f4db00f9a548...c5e9e428a919
< bitcoin-git> bitcoin/master 45f0961 Gregory Maxwell: Prevent integer overflow in ReadVarInt....
< bitcoin-git> bitcoin/master c5e9e42 Pieter Wuille: Merge #9693: Prevent integer overflow in ReadVarInt....
< bitcoin-git> [bitcoin] sipa closed pull request #9693: Prevent integer overflow in ReadVarInt. (master...varint_maxsize) https://github.com/bitcoin/bitcoin/pull/9693
< bitcoin-git> [bitcoin] sipa pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/c5e9e428a919...a077a90da88f
< bitcoin-git> bitcoin/master b2c9254 Matt Corallo: Check interruptNet during dnsseed lookups
< bitcoin-git> bitcoin/master a077a90 Pieter Wuille: Merge #10215: Check interruptNet during dnsseed lookups...
< bitcoin-git> [bitcoin] sipa closed pull request #10215: Check interruptNet during dnsseed lookups (master...2017-04-dnsseed-break) https://github.com/bitcoin/bitcoin/pull/10215
< bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/a077a90da88f...2584925077f9
< bitcoin-git> bitcoin/master d89f8ad Matt Corallo: Make DisconnectBlock and ConnectBlock static in validation.cpp
< bitcoin-git> bitcoin/master 9fececb Matt Corallo: Remove CValidationInterface::UpdatedTransaction...
< bitcoin-git> bitcoin/master 2584925 Wladimir J. van der Laan: Merge #10178: Remove CValidationInterface::UpdatedTransaction...
< bitcoin-git> [bitcoin] laanwj closed pull request #10178: Remove CValidationInterface::UpdatedTransaction (master...2017-01-wallet-cache-inmempool-2) https://github.com/bitcoin/bitcoin/pull/10178
<@wumpus> SopaXorzTaker: good sleuthing, but no, you don't have to be afraid I run random scripts from the internet on anything important, let alone bitcoin-related ones
< SopaXorzTaker> wumpus, yeah
< SopaXorzTaker> but this script actually does remote code execution
<@wumpus> the whole premise is a bit scammy; it reminds me of the trojans in the 90's whose control component had a trojan too. So everyone using the trojan to grief other people got owned themselves too...
< luke-jr> what is it even supposed to do?
<@wumpus> people running this script try to steal coins by generating random private keys. This is incredibly unlikely, and if it worked it'd be wrong in various ways
< luke-jr> lol
< luke-jr> bruteforcing privkeys is just ridiculous to attempt, but to do it with *Perl code*? lololol
< luke-jr> at face value, it's obvious the only purpose is to be a backdoor
< SopaXorzTaker> luke-jr, well
< SopaXorzTaker> it's actually done with an inner C program which is natively compiled by the script
< SopaXorzTaker> actually, the bruteforcing has some results
< sipa> and it seems to have OpenCL code too
< SopaXorzTaker> there were some addresses deliberately generated with weak PRNGs
<@wumpus> yes, if it mimimcs specific bad PRNGs (or bad brainwallets) instead of simply randomly generating keys it can certainly turn up something
< sipa> including things put there by the script's author :)
< SopaXorzTaker> wumpus, yes
< SopaXorzTaker> there is a so-called puzzle transaction
< SopaXorzTaker> with 32 BTC
< SopaXorzTaker> look it up
< SopaXorzTaker> each address uses a key one bit stronger than the previous one
< SopaXorzTaker> (there's 256 addresses)
< SopaXorzTaker> eg. 0000..0001
< SopaXorzTaker> 0000..0011
< SopaXorzTaker> 0000..0101
< SopaXorzTaker> 0000..1011
< SopaXorzTaker> and so on
<@wumpus> yes, for the author it could be very profitable, and no need to bruteforce at all, just stealing all the wallets of people running this
<@wumpus> in a way it's the classical con, make people believe something that's too good to be true
< bitcoin-git> [bitcoin] TheBlueMatt opened pull request #10221: Stop treating coinbase outputs differently in GUI: show them at 1conf (master...2017-04-no-coinbase-display-lag) https://github.com/bitcoin/bitcoin/pull/10221
< morcos> I'm happy enough to just exclude in all my bitcoin.conf files, but just want to see if everyone else is ok with the leveldb log spam... it prints a lot of useless messages right at startup (where everythign else you care about is printed)
< morcos> I'm not sure exactly what information we're expecting from the leveldb logging, so maybe there is a better solution that concentrates on that infomration or at least cleanly aggregating at startup.. it's not really clear to me how to make much use of what it does log
< sipa> only when you enable the relevant debug category?
< morcos> sipa: i assume, but my habit is to always enable all
<@wumpus> the more debugging is added, the less useful it becomes to run with debug=all
<@wumpus> though I'm fine with a debug=alllowvolume or such if you want to add that, which excludes at least leveldb and libevent
<@wumpus> but usually the recommedation is to add debug categories only when troubleshooting a certain subsystem; this became even easier with the RPC call to turn on/off individual debug flags
< jtimon> is there any advantage to Q_FOREACH over c++11 foreach?
<@wumpus> no
<@wumpus> fairly sure c++11 foreach will work with qt objects too
< jtimon> wumpus: it seems performance can be worse in some cases: https://www.dvratil.cz/2015/06/qt-containers-and-c11-range-based-loops/
< jtimon> I mean, not in a way that it can't be solved
< sipa> hmm, seems to be about some specifics with Qt containers
< jtimon> the reason I ask is because I was trying to remove PAIRTYPE and it seems Q_FOREACH requires it too, I'm not completely sure though, I'm compiling removing Q_FOREACH first and then I'll try again without removing Q_FOREACH in case we prefer to keep it, but only -j4 since I'm on the laptop...
< bitcoin-git> [bitcoin] jnewbery opened pull request #10222: [tests] test_runner - check unicode (master...test_runner_check_unicode) https://github.com/bitcoin/bitcoin/pull/10222
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/2584925077f9...6ce733747e16
< bitcoin-git> bitcoin/master a97ed80 John Newbery: [tests] test_runner - check unicode
< bitcoin-git> bitcoin/master 6ce7337 MarcoFalke: Merge #10222: [tests] test_runner - check unicode...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #10222: [tests] test_runner - check unicode (master...test_runner_check_unicode) https://github.com/bitcoin/bitcoin/pull/10222
< BlueMatt> sipa: hey
< BlueMatt> re: #10148's Clear() in ApplyTxInUndo: hmm, you may be right, anyway, dear god this is not trivial to reason about :/
< gribble> https://github.com/bitcoin/bitcoin/issues/10148 | Use non-atomic flushing with block replay by sipa · Pull Request #10148 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/6ce733747e16...50a1cc0f0aef
< bitcoin-git> bitcoin/master c9e31c3 Warren Togami: Clarify importprivkey help text with example of blank label without rescan...
< bitcoin-git> bitcoin/master 50a1cc0 MarcoFalke: Merge #10207: Clarify importprivkey help text ... example of blank label without rescan...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #10207: Clarify importprivkey help text ... example of blank label without rescan (master...importprivkey) https://github.com/bitcoin/bitcoin/pull/10207
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/50a1cc0f0aef...d86bb075bf6d
< bitcoin-git> bitcoin/master c85b080 John Newbery: [test] add warnings to test_runner
< bitcoin-git> bitcoin/master 08e51c1 John Newbery: [tests] Remove cache directory by default when running test_runner
< bitcoin-git> bitcoin/master d86bb07 MarcoFalke: Merge #10197: [tests] Functional test warnings...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #10197: [tests] Functional test warnings (master...functional_test_warnings) https://github.com/bitcoin/bitcoin/pull/10197
< sipa> BlueMatt: i'm going to add a WIP to the title, i'm not confortable with merging until there are more substantial tests
< BlueMatt> yea, I was starting to feel the same way....I mean alternatively we could drop the multi-head support and only support single-action (ie a series of connects/disconnects) between full flushes, which would simplify things and go back to a bit more how it was until you rewrote a bunch
< BlueMatt> sipa: ^
< sipa> BlueMatt: i think it is fine to only test the single-head case for now
< sipa> at worst, the result is not backward compatible omce we need muktihead
< sipa> *multi once
< BlueMatt> sipa: well my point was just the implementation of the multi-head-handling case is complicated enough that it adds a ton of review burden
< BlueMatt> esp pre-utxo-db-format-change
< BlueMatt> may be easier to just do it single-head-only, then do utxo-db, then change to multi-head...if we break compat there its ok
< sipa> the multihead code and pertxout are orthogonal, i think
< BlueMatt> i havent dug too much into pertxout yet, but shouldnt it simplify things, or is there still a concept of per-tx CCoins everywhere above the db?
< BlueMatt> my assumption was the review for this would be much simpler if you dont have to think about making sure entire transaction objects are correct, instead of there just being add/remove-outputs
< BlueMatt> at least i found it much easier to review prior to the latest changes, even ignoring the handle-disconnect stuff
< gmaxwell> I think they turn out to be pretty much orthorgonal.
< gmaxwell> okay thats a point.
< sipa> BlueMatt: the Clean call is indeed a possible violation of that orthogonality...
< BlueMatt> that was my primary example, indeed
< sipa> i don't think there are any others, but it is a fair point that pertxout is breaking backward xcompatibility already, so perhaps attempting to already support multihead isn't actually worth it
< sipa> but i think the complexity is mostly in testing
< sipa> the difference in implementation between multihead and single head is just that loop and building of a set
< BlueMatt> ok, i found the building of a set hard to reason about :p
< sipa> fair enough, but you can reason about the cases that are relevant for single head?
< BlueMatt> probably? dunno, i was more tired today than previous days, so there may also be a skew there :p
< sipa> well, maybe it is best to explain the full algorithm and reasoning why it is correct in text in comments