< gmaxwell> streams.h:407:44: warning: declaration of ‘data’ shadows a member of 'this' [-Wshadow]
< gmaxwell> I boggle at shadow warnings being so compiler specific.
< gmaxwell> This isn't the case for plain C.
< wumpus> yes for C it seems much better defined
< wumpus> though it may be historical coincidence that clang and gcc match up so well with most warnings. MSVC on the other hand...
< * luke-jr> wonders what Core would look like with -Weverything
< sipa> it hurts.
< wumpus> that said, it will probably hurt for any moderately-sized C++ project
< wumpus> except for the compilers themselves maybe; I'd (naively) expect those to compile cleanly in themselves
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/01b7cda91684...56ab672b59da
< bitcoin-git> bitcoin/master 343ba8f practicalswift: [wallet] Remove redundant initialization...
< bitcoin-git> bitcoin/master 56ab672 Wladimir J. van der Laan: Merge #9576: [wallet] Remove redundant initialization...
< bitcoin-git> [bitcoin] laanwj closed pull request #9576: [wallet] Remove redundant initialization (master...remove-redundant-initialization-ii) https://github.com/bitcoin/bitcoin/pull/9576
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/56ab672b59da...5a6af3172254
< bitcoin-git> bitcoin/master e57a1fd Russell Yanofsky: Define 7200 second timestamp window constant
< bitcoin-git> bitcoin/master 5a6af31 Wladimir J. van der Laan: Merge #9908: Define 7200 second timestamp window constant...
< bitcoin-git> [bitcoin] laanwj closed pull request #9908: Define 7200 second timestamp window constant (master...pr/timewin) https://github.com/bitcoin/bitcoin/pull/9908
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/5a6af3172254...48c3429c50fb
< bitcoin-git> bitcoin/master 025dec0 NicolasDorier: [qa] assert_start_raises_init_error
< bitcoin-git> bitcoin/master 48c3429 Wladimir J. van der Laan: Merge #9832: [qa] assert_start_raises_init_error...
< bitcoin-git> [bitcoin] laanwj closed pull request #9832: [qa] assert_start_raises_init_error (master...assert_start_raises_init_error) https://github.com/bitcoin/bitcoin/pull/9832
< bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/48c3429c50fb...9d5fcbfb0889
< bitcoin-git> bitcoin/master 99c0e81 John Newbery: Fix BIP68 activation test
< bitcoin-git> bitcoin/master f5aba8a John Newbery: Move tx version 2 standardness check to after bip68 activation
< bitcoin-git> bitcoin/master 9d5fcbf Wladimir J. van der Laan: Merge #9739: Fix BIP68 activation test...
< bitcoin-git> [bitcoin] laanwj closed pull request #9739: Fix BIP68 activation test (master...fixbip68testing) https://github.com/bitcoin/bitcoin/pull/9739
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/9d5fcbfb0889...d32581cc29d1
< bitcoin-git> bitcoin/master db07f91 practicalswift: Assert that what might look like a possible division by zero is actually unreachable
< bitcoin-git> bitcoin/master d32581c Wladimir J. van der Laan: Merge #9547: bench: Assert that division by zero is unreachable...
< bitcoin-git> [bitcoin] laanwj closed pull request #9547: bench: Assert that division by zero is unreachable (master...avoid-potential-division-by-zero-in-benchmark-state-keeprunning) https://github.com/bitcoin/bitcoin/pull/9547
< bitcoin-git> [bitcoin] laanwj pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/d32581cc29d1...fa625b078b01
< bitcoin-git> bitcoin/master 654e044 Russell Yanofsky: [trivial] Add comment documenting CWalletTx::mapValue
< bitcoin-git> bitcoin/master a1fe944 Russell Yanofsky: Remove reference to nonexistent "version" wallet transaction mapvalue field...
< bitcoin-git> bitcoin/master 87ed396 Russell Yanofsky: [trivial] Add comment documenting bumpfee mapValues
< bitcoin-git> [bitcoin] laanwj closed pull request #9333: Document CWalletTx::mapValue entries and remove erase of nonexistent "version" entry. (master...pr/comment-mapvalue) https://github.com/bitcoin/bitcoin/pull/9333
< bitcoin-git> [bitcoin] kobake opened pull request #9926: Rename argument name 'data' to prevent shadowing. (master...fix-shadow-warning) https://github.com/bitcoin/bitcoin/pull/9926
< bitcoin-git> [bitcoin] kobake closed pull request #9926: Rename argument name 'data' to prevent shadowing. (master...fix-shadow-warning) https://github.com/bitcoin/bitcoin/pull/9926
< bitcoin-git> [bitcoin] jonasschnelli closed pull request #9256: Fix more CWallet/CWalletDB layer violations (master...2016/12/ref_walletdb) https://github.com/bitcoin/bitcoin/pull/9256
< wumpus> jonasschnelli: too bade about 9256 :( might pick it up at some point, need to be able to replace the database backend for the cloudabi port
< jonasschnelli> wumpus: Oh. Okay.. I'm currently working on #8574 (which is also a BDB abstraction).
< gribble> https://github.com/bitcoin/bitcoin/issues/8574 | [Wallet] refactor CWallet/CWalletDB/CDB by jonasschnelli · Pull Request #8574 · bitcoin/bitcoin · GitHub
< jonasschnelli> 9256 can be done afterwards..
< wumpus> I've noticed that there is a lot of overlap between dbwrapper.h and walletdb.h
< wumpus> yeah
< jonasschnelli> Yes. We need clean layering first to support multiple db backends
< wumpus> I really like the dbwrapper.h. With that I've mangaed to replace the db backend multiple times. Implementing e.g. a walletdb.h for leveldb seemsto be more work
< wumpus> (as it doesn't wrap some of the objects)
< * jonasschnelli> looking closer at dbwrapper.h/cpp
< wumpus> then again - a pure key/value store may not be what we eventually want for the wallet at all, maybe something like sqlite with indexes is better. But on the short term it'd be useful to switch database backends and just consider key/value stores as a given, abstracted.
< jonasschnelli> Yes. Something like that for the wallet db would be great. I first though CDBEnv follows that approach... but not really
< wumpus> I think they evolved from the same abstraction, but dbwrapper was updated time and time again to be a better abstraction, while walletdb did not :)
< jonasschnelli> Sqlite could be useful. I though about it.... but not sure if we want private keys in there.. also not sure about the compatibility between different sqlite implementations..
< jonasschnelli> I still like sipa logdb approach (full in-mem mapping, no on-disk database queries, simple append only log with on-going hash)
< wumpus> sqlite actually encourages usage as an applicatiion data format in their faq
< wumpus> so I *think* that means they make a commitment to on-disk compatibility. Also their databases are single files, unlike leveldb.
< wumpus> yes sure I like that too
< wumpus> in any case, for everything we need to improve the abstractions involved
< jonasschnelli> For pure transactions and pubkey, I would probably prefere sqlite
< wumpus> right - and with HD wallets we need a lot less key storage
< jonasschnelli> Yes. Lets focus on abstraction and discuss the new persistence layer later.
< wumpus> +private
< sipa> the abstractions are complicated by the fact that BDB needs some things (flushing, locking, environment, ...) that nothing else needs
< jonasschnelli> Yes. The abstraction needs to support that (at least for the next years)
< sipa> no, only until we get rid of BDB
< sipa> i hope that's less than a few years
< MarcoFalke> Anyone want to review #9880?
< gribble> https://github.com/bitcoin/bitcoin/issues/9880 | Verify Tree-SHA512s in merge commits, enforce sigs are not SHA1 by TheBlueMatt · Pull Request #9880 · bitcoin/bitcoin · GitHub
< jonasschnelli> hmm.. if we offer a good migration option,... but the migration option needs BDB support... :)
< MarcoFalke> Currently only wumpus can merge on master
< sipa> jonasschnelli: that can just be a trivial "iterate the keys, convert" - not a full db abstraction
< jonasschnelli> sipa: Indeed
< jonasschnelli> wumpus, sipa: if you care for wallet db abstraction, this (#8574) could be reviewed:
< gribble> https://github.com/bitcoin/bitcoin/issues/8574 | [Wallet] refactor CWallet/CWalletDB/CDB by jonasschnelli · Pull Request #8574 · bitcoin/bitcoin · GitHub
< jonasschnelli> MarcoFalke: Oh. Yes. My merged broke verify-commits (and therefor travis on master): https://travis-ci.org/bitcoin/bitcoin/jobs/206917518#L513
< wumpus> jonasschnelli: I'll take a look. On the short run I just need something that can replace berkeleydb with a different k/v store, no need for any backward compatiblity or migration
< jonasschnelli> Thanks. Yes. That's a different path then...
< wumpus> jonasschnelli: not using a database or a custom database would be fine too, but that seems like a lot more work :)
< wumpus> and would require a huge patch set to maintain on top
< wumpus> which would need to be updated for every upstream wallet change
< wumpus> so yeah, switching the k/v store is probably the best option
< Victorsueca> I just had a question this morning while at was at shower... Is it safe to defrag while bitcoin core is running?
< wumpus> defrag? is that still a thing?
< BirneGetreide_> Just don't defrag, ever. It is a waste of time
< jonasschnelli> Oh... verify-commits.sh gave a "Segmentation fault: 11"! hah
< jonasschnelli> I hope it's not coming from gnupg
< wumpus> but no, we can't guarantee that that is safe. Moving filesystem blocks in the background could be safe or unsafe depending on whether the OS keeps track of things properly.
< Victorsueca> it's windows so I doubt it does lol
< wumpus> right, you didn't even need to ask
< jonasschnelli> Hmm... I think the verify-commit.sh segfault 11 im getting is from /bin/sh
< jonasschnelli> It happens when the script calls IS_SIGNED() (recursive call)
< jonasschnelli> Happens on master was well...
< wumpus> ugh :/
< jonasschnelli> Can't even attach lldb to /bin/sh due to OSX's "integrity protection"... copied out sh and running now in lldb
< sipa> too many recursion levels in the shell?
< jonasschnelli> sipa: I think so.
< jonasschnelli> Stack trace: http://bitcoin.jonasschnelli.ch/st.txt
< jonasschnelli> now running in bash
< jonasschnelli> same
< jonasschnelli> I think some Ubuntu verions are also affected. http://lists.gnu.org/archive/html/bug-bash/2003-12/msg00007.html
< jonasschnelli> Would it make sense to port verify-commits.sh to python? We already rely on python for the rest, right? ping BlueMatt
< jonasschnelli> BlueMatt: it seems that the current amount of recursive calls on IS_SIGNED() seems to break some shell implementations (OSX 10.11 and I think also Ubuntu 14.04)
< wumpus> can we somehow make verify-commits.sh simpler, at least temporary?
< wumpus> e.g. do the most important check that the HEAD commit is signed, but no more, that shouldn't take any recursion
< wumpus> then after the script is fixed we can go back to the more comphrehensive one
< jonasschnelli> wumpus: Yes. That's probably a good idea... maybe we add an arg and by default, it just checks the head
< bitcoin-git> [bitcoin] jonasschnelli opened pull request #9928: Allow verify-commit.sh to just verify the HEAD commit (Use non-recursive verification by default) (master...2017/03/vc_simple) https://github.com/bitcoin/bitcoin/pull/9928
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/fa625b078b01...8a3b07529d9c
< bitcoin-git> bitcoin/master 7184e25 Jonas Schnelli: [Wallet] refactor CWallet/CWalletDB/CDB...
< bitcoin-git> bitcoin/master 8a3b075 Wladimir J. van der Laan: Merge #8574: [Wallet] refactor CWallet/CWalletDB/CDB...
< bitcoin-git> [bitcoin] laanwj closed pull request #8574: [Wallet] refactor CWallet/CWalletDB/CDB (master...2016/08/bdb_abstraction_2) https://github.com/bitcoin/bitcoin/pull/8574
< wumpus> going to have a stab at making the RPC tests run (optionally) through UNIX sockets
< wumpus> don't think much is needed besides abstracting 'url' into 'RPC connect info'
< jonasschnelli> wumpus: haven't thought about the RPC test... great idea.
< wumpus> jonasschnelli: yep it provides an example on how to use RPC over UNIX sockets in python at the same time
< bitcoin-git> [bitcoin] laanwj opened pull request #9929: tests: Delete unused function _rpchost_to_args (master...2017_03_tests_unused) https://github.com/bitcoin/bitcoin/pull/9929
< BlueMatt> jonasschnelli: if you want to feel free, but I wont be able to maintain it anymore :P
< wumpus> many of us have the same problem with shellscript :p
< wumpus> but I don't care what language it is in, it just needs to work
< wumpus> nesting recursion too deep is a problem in any language
< BlueMatt> yea, i should have removed the recursion when i made it not ever look at the second parent
< BlueMatt> should be trivial to do
< bitcoin-git> [bitcoin] laanwj closed pull request #9924: [UI Styling] Left-Align Tab Bar - optionsdialog.ui (master...patch-1) https://github.com/bitcoin/bitcoin/pull/9924
< bitcoin-git> [bitcoin] laanwj closed pull request #9925: [UI Styling] Left-Align Tab Bar - debugwindow.ui (master...patch-2) https://github.com/bitcoin/bitcoin/pull/9925
< bitcoin-git> [bitcoin] laanwj pushed 6 new commits to master: https://github.com/bitcoin/bitcoin/compare/8a3b07529d9c...4df8213b98d3
< bitcoin-git> bitcoin/master be908a6 Matt Corallo: Fail merge if there are any symlinks
< bitcoin-git> bitcoin/master d9c450f Matt Corallo: Verify Tree-SHA512s in merge commits, enforce sigs are not SHA1
< bitcoin-git> bitcoin/master eddc77a Peter Todd: Add comment re: why SHA1 is disabled
< bitcoin-git> [bitcoin] laanwj closed pull request #9880: Verify Tree-SHA512s in merge commits, enforce sigs are not SHA1 (master...2017-02-validate-sha512) https://github.com/bitcoin/bitcoin/pull/9880
< BlueMatt> god damn it, how did it fail on head
< BlueMatt> grrrr it doesnt fail locally
< bitcoin-git> [bitcoin] matthias-g opened pull request #9930: Trivial: Correct indentation (master...fix-indent) https://github.com/bitcoin/bitcoin/pull/9930
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/4df8213b98d3...c78adbf450be
< bitcoin-git> bitcoin/master b23dcd2 John Newbery: Fix segwit getblocktemplate test.
< bitcoin-git> bitcoin/master c78adbf Wladimir J. van der Laan: Merge #9843: Fix segwit getblocktemplate test...
< bitcoin-git> [bitcoin] laanwj closed pull request #9843: Fix segwit getblocktemplate test (master...fixsegwitgetblocktemplate) https://github.com/bitcoin/bitcoin/pull/9843
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/c78adbf450be...d5ce14e22338
< bitcoin-git> bitcoin/master 99fecf8 Wladimir J. van der Laan: tests: Delete unused function _rpchost_to_args...
< bitcoin-git> bitcoin/master d5ce14e Wladimir J. van der Laan: Merge #9929: tests: Delete unused function _rpchost_to_args...
< bitcoin-git> [bitcoin] laanwj closed pull request #9929: tests: Delete unused function _rpchost_to_args (master...2017_03_tests_unused) https://github.com/bitcoin/bitcoin/pull/9929
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/d5ce14e22338...72fb5158b1c8
< bitcoin-git> bitcoin/master 188f89c Gregory Sanders: Disallow copy of CReserveKeys
< bitcoin-git> bitcoin/master 72fb515 Wladimir J. van der Laan: Merge #9906: Disallow copy constructor CReserveKeys...
< bitcoin-git> [bitcoin] laanwj closed pull request #9906: Disallow copy constructor CReserveKeys (master...noreservecopy) https://github.com/bitcoin/bitcoin/pull/9906
< morcos> Call for another set of eyes on #9602.. I think it has enough technical review now, but couldn't hurt to have some more people review the open questions in the PR message and make sure they are ok with the changes made.
< gribble> https://github.com/bitcoin/bitcoin/issues/9602 | Remove coin age priority and free transactions - implementation by morcos · Pull Request #9602 · bitcoin/bitcoin · GitHub
< morcos> And then lets get it merged please
< gmaxwell> wumpus: for the unix domain socket bitcoin-cli, could you have the client behind an actual feature test? so that when the user's libevent is upgraded it starts working?
< wumpus> gmaxwell: once my patch lands in mainline libevent, it can be enabled based on the libevent version
< wumpus> currently there is no way to detect whether libevent can do it or not
< wumpus> if it takes a long time to be included in libevent we could add the patch to depends so that the released binaries will have it
< wumpus> can always decide on that when the 0.15 release gets close and it's still uncertain
< gmaxwell> has the libevent people given any feedback suggesting if they're likely to accept the change or not?
< wumpus> nope
< gmaxwell> might be good to get that, if they wont we might want a different approach. :(
< gmaxwell> (than potentially carrying a patch with release binaries forever)
< wumpus> well it's still useful even without the client change
< wumpus> and there is no other approach to support it in bitcoin-cli anyhow
< BlueMatt> anyone able to reproduce the travis master fail?
< BlueMatt> verify-commits is failing again but not locally anymore
< wumpus> not that I know of, at least, maybe libevent people could suggest something, butI haven't had feedback on it at all
< gmaxwell> BlueMatt: verify commits is failing for me on the 0.14 branch, I haven't checked master today.
< wumpus> gmaxwell: wow that's depressing :/
< wumpus> they've had, what, two days including a sunday to respond, I wouldn't worry about "carrying a patch forever" yet
< BlueMatt> oh wut?
< wumpus> on 0.14 it should pass now
< BlueMatt> well it should pass on master too
< BlueMatt> did it succeed on 0.14 on travis?
< wumpus> yes I think so
< BlueMatt> yes, it did
< BlueMatt> hmmm, MarcoFalke i cant reproduce your Tree-SHA512 from https://github.com/bitcoin/bitcoin/commit/f7ec7cfd38b543ba81ac7bed5b77f9a19739460b
< BlueMatt> i didnt try the python code, but the bash-based snippet doesnt work here
< wumpus> verify-commits.sh passes on master here, locally
< BlueMatt> found the issue...needs gpg 2.1 to pass --weak-digest
< BlueMatt> also the above tree-sha512 issue
< wumpus> can't reproduce that one either,tree_sha512sum() gives dc5cf61d226a16a77f32fab8abc18fd469dfe8bda59a2cb8f9eca3a60e474f180dec4fa59bb3fee86efa0e123ba8c198c7efe76bfdaa2518f4169ab0849f6694
< BlueMatt> yea, that looks like what i (and verify-commits) gets
< wumpus> it's not causing verify-commits.sh to fail here though
< BlueMatt> you have to run verify-commits with --tree-checks
< BlueMatt> (because its slowwwww)
< BlueMatt> next pr will fix all above issues, and also I think i should do --tree-checks but only for top commit
< BlueMatt> so that travis will catch this
< BlueMatt> but not for more because otherwise travis would take hours by the time we get to 0.15
< wumpus> I agree, we should avoid making the check slower
< wumpus> checking just the top commit or top two commits thoroughly should be enough, travis runs often enough
< BlueMatt> yes
< BlueMatt> it should run every commit, no?
< BlueMatt> we might see it fail hours later, but at least it runs for each one
< wumpus> I don't think that's guaranteed. It runs every time it sees a new push
< BlueMatt> ahh, yes, race
< wumpus> which usually means it runs for every top-level commit as the merge script pushes them one at a time, but still
< wumpus> what is it with all those people reporting Ekiga build problems in our issue tracker
< sipa> ekiga?
< wumpus> apparently some VOIP program
< sipa> and what are they reporting? i haven't seen that
< wumpus> I first didn't notice they were talking about a different application
< gwillen> well, one person happened to get a match on the text of the error message
< sipa> hah
< BlueMatt> wtf
< wumpus> in any case, configure arguments suggestions port very badly between different applications
< gwillen> then google indexed the word 'ekiga' when they said it, and now everyone's getting linked to this bug when they google 'ekiga' and the error message :-)
< wumpus> lol that must be it
< gwillen> indeed, I just checked, that bug is the first hit for the error, and the second hit for 'ekiga' plus the error text.
< BlueMatt> wumpus: i cant verify your latest sha512 either
< wumpus> sigh :/
< wumpus> is it possible that the order is not deterministic?
< BlueMatt> hmmm, now I'm confused
< BlueMatt> i doubt it?
< wumpus> is it hashing any files which are not part of git, but linger around in the directory?
< BlueMatt> hmm, maybe I'm wrong, maybe your hash is right
< BlueMatt> cant tell from terminal history anymore :(
< wumpus> I'll check in a minute, after I push my tree
< wumpus> for me it matches
< wumpus> (using the python code, output is e55ce10bf7f2dc91de9797e60ab7767fb51f25255995d62ddf358c52b7aaa23c26fbfb522e1610ff950b86804ddbc38dc0d7708bfab2c4d33ad99a275d8c77db, which matches what is in the merge commit)
< wumpus> "git ls-tree --full-tree -r --name-only HEAD | LANG=C sort | xargs -n 1 sha512sum | sha512sum" gives the same output
< BlueMatt> yea, heisenbug
< BlueMatt> travis is failing on it but i cant find it
< BlueMatt> wumpus: yes, sipa meant to say LC_ALL when he said LANG (according to the sort man page)
< sipa> i thought i meant to say C, but maybe i thought wrong about what i meant
< BlueMatt> sipa: no, you have to set LC_ALL=C, not LANG=C
< bitcoin-git> [bitcoin] TheBlueMatt opened pull request #9932: Fix verify-commits on travis and always check top commit's tree (master...2017-03-fix-verify-commits) https://github.com/bitcoin/bitcoin/pull/9932
< sipa> BlueMatt: ahh!
< wumpus> whoops, yes, stupid locales :/
< BlueMatt> why travis has a different locale is beside me
< wumpus> the python version in github-merge.py shouldn't be affected
< wumpus> no idea what locale it sets, but it helped find this issue
< wumpus> which I suppose is good
< BlueMatt> fair
< bitcoin-git> [bitcoin] jtimon closed pull request #9882: RPC: Introduce -rpcamountdecimals for the RPC to use other units than BTC (master...0.14.99-rpc-amounts) https://github.com/bitcoin/bitcoin/pull/9882
< jtimon> ping #9279
< gribble> https://github.com/bitcoin/bitcoin/issues/9279 | Consensus: Move CFeeRate out of libconsensus by jtimon · Pull Request #9279 · bitcoin/bitcoin · GitHub