< sipa> BlueMatt: i'm not
< bitcoin-git> [bitcoin] practicalswift opened pull request #10839: Don't use pass by reference to const for cheaply-copied types (bool, char, etc.). (master...dont-pass-by-reference-for-cheaply-copied-types) https://github.com/bitcoin/bitcoin/pull/10839
< fanquake> sipa same issue as before. Assertion failed: (consensus.hashGenesisBlock etc
< sipa> fanquake: ok, thank you
< bitcoin-git> [bitcoin] practicalswift opened pull request #10840: Remove duplicate include (master...remove-duplicate-include) https://github.com/bitcoin/bitcoin/pull/10840
< AsadSalman> Hi
< AsadSalman> I'm working in the bitcoin core, trying to get the amount of a transaction sent into the mempool
< AsadSalman> I'm logging all entries (working on regtestnet, btw)
< AsadSalman> I am able to get a value out (as you can see in the screenshot)
< AsadSalman> but that is a sum of both CTxOut
< AsadSalman> I only want the amount I spent (which was 10BTC, i.e. 1,000,000,000 satoshi)
< AsadSalman> I don't know what the other CTxOut that is getting summed in is, and how do I get rid of it in the final sum?
< sipa> it's change
< AsadSalman> change? I'm not sure what you mean, could you elaborate?
< sipa> not here, try bitcoin.stackexchange.com
< sipa> or #bitcoin
< AsadSalman> okay! (sorry, I thought this would be an appropriate place to ask about core)
< sipa> your question is about how bitcoin transactions work
< gmaxwell> BlueMatt: on 10831 please review the rebase carefully, I'm sick and wouldn't even have done the rebase today but for merge window urgency.
< cfields> sipa: still crashes :(
< cfields> sipa: i managed to get lldb to step into the asm, but it doesn't show me where I am when i single-step instructions, only that I'm in the asm block. So i have to step 1 instruction at a time and dump registers
< cfields> the explicit assignments will help though, since now I'll know where I'm looking
< sipa> cfields: so you do reach the asm block?
< sipa> can you count how many instructions inside you execute?
< cfields> sipa: yea, and past the zero-size check
< cfields> sipa: yea, i can do that. the recent changes should make it much easier, i'll poke some more
< sipa> cfields: thanks for all the time you're spending on thid
< cfields> sipa: np. it's a big boost.
< sipa> cfields: for IBD it seems a 200-300 second reduction to block 450k
< sipa> at the tip the impact is probably larger, but harder to benchmark
< cfields> wow
< cfields> net-side will benefit at the tip too, though probably not enough to be relevant
< cfields> sipa: gcc docs says the value of inputs isn't allowed to change. i suppose values of pointers passed in are ok though?
< sipa> cfields: the only inputs are constants
< sipa> everything else is marked as an output
< cfields> oh right, nm
< gmaxwell> sha-ni is a bigger boost.
< sipa> yes, far larger
< sipa> close to 1000s reduction
< sipa> more like 800s
< sipa> (compared to master)
< gmaxwell> cfields: I expect even just sse4 to make a very measurable improvement in compact block relay... that is mostly bottlenecked on sha2...
< cfields> whoa
< cfields> have you benchmarked any benefit to adding a dedicated sha2d?
< sipa> cfields: can you show another disasm?
< gmaxwell> haven't though if we want further benefit I think what we really want is a hash tree that hashes multiple things at once, parallel sse4 sha2 is significantly faster than one at a time... and it even looks like for sha-ni for max throughput you want to do at least two at a time.
< cfields> gmaxwell: yea, i was reading that paper (one of many?) yesterday
< cfields> stepping through it now
< cfields> sipa: note that i haven't hand-modified any flags for that one, so the stack protector is in place where it hasn't been in other dumps
< sipa> i can see
< sipa> that
< sipa> cfields: during the entire execution, the rdi register should contain a pointer to the SHA256 state
< sipa> and it should never change
< sipa> there is not a single instruction in your disasm that modifies that register, so far so good
< sipa> so the question is... is rdi actually pointing to the state?
< gmaxwell> sipa: you could try taking cfields disasm and monkey patching it into a binary to try to reproduce he failure.
< sipa> gmaxwell: nontrivial... linking information would be lost
< cfields> sipa: actually, hold on. linker stuck a bunch of nops in the final binary...
< cfields> let met give you a dump of that rather than a dump of just the object
< sipa> cfields: ah, yes!
< gmaxwell> linker replaced the object with foldgers crystals, turns out we noticed.
< cfields> uhmm, wtf
< sipa> the linker replaced the object with foldgers crystals?
< cfields> ok, i don't trust myself enough here to be a proxy debugger...
< cfields> sec
< cfields> I'm just going to send you the whole dasm. Got anywhere convenient for a 58mb text dump?
< sipa> sure
< sipa> mail?
< midnightmagic> lol
< sipa> gmaxwell: your foldgers crystals guess is surprisingly accurate
< cfields> bingo!
< cfields> got it.
< sipa> oh?
< cfields> now to figure out why :)
< cfields> -Wl,-dead_strip
< cfields> it's weird that if it's going to strip it, it doesn't strip the whole thing though
< cfields> (to be clear, we use -Wl,-dead_strip by default. With that removed, it runs as expected)
< sipa> cfields: give local labels a name that starts with L
< sipa> that way the linker treats them as local labels rather than separate functions
< cfields> sipa: works!
< sipa> i found thst
< cfields> hah! well that was some easy googling for once :)
< cfields> sipa: do you see that documented anywhere?
< sipa> nope
< bitcoin-git> [bitcoin] jnewbery opened pull request #10841: [rpc] Give users one final warning before depracating getinfo (master...deprecate_getinfo) https://github.com/bitcoin/bitcoin/pull/10841
< sipa> ah
< cfields> sipa: maybe safer to use digits instead, then? Reading the docs like a pedant, I assume some future linker could break win32 the same way.
< cfields> nope, that's crashy too
< sipa> cfields: so, latest PR version works?
< cfields> sipa: i don't think so, i think that matches what i tried
< sipa> ?
< cfields> sipa: i misread the docs, look at their example
< cfields> "So for example, the first 1: may be named .L1C-B1, and the 44th 3: may be named .L3C-B44."
< sipa> A local symbol is any symbol beginning with certain local label prefixes. By default, the local label prefix is `.L' for ELF systems or `L' for traditional a.out systems, but each target may have its own set of local label prefixes.
< cfields> apparently the numbers aren't a replacement for the L prefix
< sipa> you're looking at automatically generated names
< sipa> we don't need that; the asm syntax's %= guaranrees uniqueness already
< cfields> sipa: sorry, misread the diff
< sipa> it won't work, i forgot to rename the jumps
< cfields> yep, with those renamed, all good
< phantomcircuit> gmaxwell, btw when i did the walletdb stuff i was doing CWalletDB *pwalletdb=NULL and then initializing pwalletdb only if it was null instead of creating a new function
< bitcoin-git> [bitcoin] practicalswift opened pull request #10842: Fix incorrect Doxygen tag (@ince → @since) (master...doxygen-since) https://github.com/bitcoin/bitcoin/pull/10842
< bitcoin-git> [bitcoin] practicalswift opened pull request #10843: Add attribute [[noreturn]] (C++11) to functions that will not return (master...noreturn) https://github.com/bitcoin/bitcoin/pull/10843
< terjegun> Hello! I was wondering, is there any reward for contributing translations to the project?
< Lauda> No.
< instagibbs> terjegun, satisfaction in knowing you helped others?
< bitcoin-git> [bitcoin] ReneNyffenegger opened pull request #10844: Use range based for loop (master...dbwrapper_tests-for) https://github.com/bitcoin/bitcoin/pull/10844
< bitcoin-git> [bitcoin] TheBlueMatt closed pull request #10838: (finally) remove the longest-ever-deprecated RPC call getinfo (master...2017-07-seriously-fuck-getinfo) https://github.com/bitcoin/bitcoin/pull/10838
< bitcoin-git> [bitcoin] practicalswift opened pull request #10845: Remove unreachable code (master...remove-unreachable-code) https://github.com/bitcoin/bitcoin/pull/10845
< bitcoin-git> [bitcoin] TheBlueMatt opened pull request #10846: Add stdlib include in qt/test/test_main (for setenv) (master...2017-07-env-include) https://github.com/bitcoin/bitcoin/pull/10846
< bitcoin-git> [bitcoin] TheBlueMatt closed pull request #10846: Add stdlib include in qt/test/test_main (for setenv) (master...2017-07-env-include) https://github.com/bitcoin/bitcoin/pull/10846
< bitcoin-git> [bitcoin] sipa pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/5cfdda2503c9...ef37f2033c4a
< bitcoin-git> bitcoin/master a9e82f6 Pieter Wuille: Use cpuid intrinsics instead of asm code
< bitcoin-git> bitcoin/master 674848f Pieter Wuille: Clarify entropy source
< bitcoin-git> bitcoin/master ef37f20 Pieter Wuille: Merge #10820: Use cpuid intrinsics instead of asm code...
< bitcoin-git> [bitcoin] sipa closed pull request #10820: Use cpuid intrinsics instead of asm code (master...20170717_cpuid) https://github.com/bitcoin/bitcoin/pull/10820
< bitcoin-git> [bitcoin] sipa pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/ef37f2033c4a...b4d03be3cac0
< bitcoin-git> bitcoin/master 912da1d René Nyffenegger: Use AC_ARG_VAR to set ARFLAGS....
< bitcoin-git> bitcoin/master b4d03be Pieter Wuille: Merge #10766: Building Environment: Set ARFLAGS to cr...
< bitcoin-git> [bitcoin] sipa closed pull request #10766: Building Environment: Set ARFLAGS to cr (master...ARFLAGS) https://github.com/bitcoin/bitcoin/pull/10766
< bitcoin-git> [bitcoin] sipa pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/b4d03be3cac0...99c7db8731cc
< bitcoin-git> bitcoin/master c53369c practicalswift: Remove duplicate include
< bitcoin-git> bitcoin/master 99c7db8 Pieter Wuille: Merge #10840: Remove duplicate include...
< bitcoin-git> [bitcoin] sipa closed pull request #10840: Remove duplicate include (master...remove-duplicate-include) https://github.com/bitcoin/bitcoin/pull/10840
< bitcoin-git> [bitcoin] sipa pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/99c7db8731cc...ad6fce67b9bb
< bitcoin-git> bitcoin/master d0413c6 René Nyffenegger: Use range based for loop...
< bitcoin-git> bitcoin/master ad6fce6 Pieter Wuille: Merge #10844: Use range based for loop...
< bitcoin-git> [bitcoin] sipa closed pull request #10844: Use range based for loop (master...dbwrapper_tests-for) https://github.com/bitcoin/bitcoin/pull/10844
< bitcoin-git> [bitcoin] sipa pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/ad6fce67b9bb...565494619d80
< bitcoin-git> bitcoin/master 6835cb0 practicalswift: Avoid static analyzer warnings regarding uninitialized arguments...
< bitcoin-git> bitcoin/master 5654946 Pieter Wuille: Merge #10735: Avoid static analyzer warnings regarding uninitialized arguments...
< bitcoin-git> [bitcoin] sipa closed pull request #10735: Avoid static analyzer warnings regarding uninitialized arguments (master...std-array) https://github.com/bitcoin/bitcoin/pull/10735
< BlueMatt> that seems to indicate it doesnt support "seg -e"???
< BlueMatt> that sounds like an ubuntu-on-windows issue
< sipa> I vaguely remember that there were known issues when building on Ubuntu-in-Windows, but I don't remember whether it was this.
< BlueMatt> not this, no
< BlueMatt> well, afair
< cfields> IIRC the last round of problems was caused by an update that borked the default path
< sipa> cfields: what is the option to make gcc emit every function in its own section?
< cfields> sipa: ffunction-section
< cfields> er, there's another s in there somewhere though
< cfields> -ffunction-sections
< sipa> do we enable -Wl,--gc-sections by default?
< cfields> sipa: and you want to link with -gc-sections
< cfields> no
< sipa> any reason not to?
< cfields> sipa: i think default-hidden-visibility accomplishes the same thing without the section bloat?
< sipa> cfields: is that always set for us?
< sipa> cfields: a build with CFLAGS/CXXFLAGS/LDFLAGS all set to "-O2 -g0", results in a 6MB stripped bitcoind
< sipa> cfields: a build with CFLAGS/CXXFLAGS/LDFLAGS all set to "-O2 -g0 -ffunction-sections -fdata-sections -Wl,--gc-sections", results in a 5.4MB stripped bitcoind
< cfields> sipa: it's not set auto, --enable-reduced-exports
< cfields> mind comparing with that?
< gmaxwell> why don't we do that by default
< cfields> gmaxwell: there's a visibility issue with some boost versions
< gmaxwell> ah
< sipa> cfields: that option makes no difference; 6.0MB again
< cfields> huh
< sipa> make clean && ./configure --with-incompatible-bdb --without-gui --enable-reduced-exports && make -j9 src/bitcoind && strip src/bitcoind && wc -c src/bitcoind
< sipa> 6008256 src/bitcoind
< cfields> sipa: sounds promising then, though I'm kinda confused as to why
< sipa> make clean && COMMON="-O2" && ./configure --with-incompatible-bdb --without-gui CFLAGS="$COMMON" CXXFLAGS="$COMMON" LDFLAGS="$COMMON" && make -j9 src/bitcoind && strip src/bitcoind && wc -c src/bitcoind
< sipa> 6008256 src/bitcoind
< sipa> ^ exactly the same
< sipa> make clean && COMMON="-O2 -ffunction-sections -fdata-sections -Wl,--gc-sections" && ./configure --with-incompatible-bdb --without-gui CFLAGS="$COMMON" CXXFLAGS="$COMMON" LDFLAGS="$COMMON" && make -j9 src/bitcoind && strip src/bitcoind && wc -c src/bitcoind
< sipa> 5397952 src/bitcoind
< cfields> sipa: i wonder if there are any downsides wrt locality?
< sipa> cfields: i can run a reindex benchmark
< sipa> cfields: should --enable-reduced-exports affect the CXXFLAGS/LDFLAGS that get printed in any way?
< cfields> sipa: sigh, probably not
< cfields> that needs to be cleaned up. There's lots of stuff not reported
< gmaxwell> sipa: you mean when doing a V=1 build so you can see the arguments it actually uses
< cfields> sipa: aurgh!
< cfields> wait, hmm
< cfields> sipa: sorry, i lied to you :(
< cfields> --enable-reduce-exports
< sipa> shouldn't it complain if i make a typo?
< cfields> sipa: it used to, but we can't anymore because config options get forwarded to subconfigs
< sipa> heh, i always wondered how it dealt with that...
< sipa> -rwxr-xr-x 1 pw pw 6008256 Jul 16 13:15 src/bitcoind-shasse-O2
< sipa> -rwxr-xr-x 1 pw pw 5979584 Jul 16 13:21 src/bitcoind-shasse-O2reduce
< sipa> -rwxr-xr-x 1 pw pw 5397952 Jul 16 13:19 src/bitcoind-shasse-O2sections
< sipa> the middle one is with CXXFLAGS="-O2" --enable-reduce-exports
< sipa> benchmarking those 3
< cfields> sipa: may as well try with -fdata-sections too
< sipa> cfields: yeah, that's with both
< gmaxwell> sipa: why would you expect a performance difference?
< cfields> ah ok
< sipa> gmaxwell: cfields asked if there could be a locality impact
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/565494619d80...1fc783fc08bc
< bitcoin-git> bitcoin/master 5618b7d Pavel Janík: Do not shadow upper local variable `state`.
< bitcoin-git> bitcoin/master 1fc783f MarcoFalke: Merge #10739: test: Move variable `state` down where it is used...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #10739: test: Move variable `state` down where it is used (master...20170704_Wshadow_txvalidationcache_tests) https://github.com/bitcoin/bitcoin/pull/10739
< cfields> sipa: if we're using ffunction-sections, we can use -Wl,--icf=safe too
< cfields> shaves off another ~70k for me
< sipa> cfields: i don't see that in man ld
< cfields> sipa: man ld.gold
< sipa> ah
< bincap> given two gitian.signs asserts (from two people) is there a quick command to compare if their out_manifest is identical?
< bitcoin-git> [bitcoin] practicalswift opened pull request #10847: Enable devirtualization opportunities by using the final specifier (C++11) (master...devirtualization) https://github.com/bitcoin/bitcoin/pull/10847
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/1fc783fc08bc...3895e25a7736
< bitcoin-git> bitcoin/master 2c2e90d practicalswift: Fix incorrect Doxygen tag (@ince → @since). Make Doxygen parameter names match actual parameter names.
< bitcoin-git> bitcoin/master 3895e25 MarcoFalke: Merge #10842: Fix incorrect Doxygen tag (@ince → @since). Doxygen parameter name matching....
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #10842: Fix incorrect Doxygen tag (@ince → @since). Doxygen parameter name matching. (master...doxygen-since) https://github.com/bitcoin/bitcoin/pull/10842
< Chris_Stewart_5> In the test framework, using node.generate() will propogate the block to another peer right?
< sipa> it'll propagate to whatever other peer it is connected to
< sipa> who will do the same
< Chris_Stewart_5> Do I need to use some sort of sleep mechanism if I need it to propogate before runnign the next line?
< sipa> no, you need to use wait_for_block
< sipa> or something similar
< Chris_Stewart_5> oh, cool. I'll check that out. Thanks.
< BlueMatt> cfields: gonna update 10809?
< sipa> BlueMatt: care to investigate #10758?
< gribble> https://github.com/bitcoin/bitcoin/issues/10758 | Fix some chainstate-init-order bugs. by TheBlueMatt · Pull Request #10758 · bitcoin/bitcoin · GitHub
< sipa> (it seems to forget progress when continuing a reindex)
< BlueMatt> oh, yes, thats next on my list
< sipa> actually, i may be an idiot there
< BlueMatt> hmm, did that not used to happen? I have no idea
< sipa> it is supposed to start over from scratch, but go faster through the parts it has already done
< sipa> i guess the thing to make sure is that it does not wipe the block index when contonuing a reindex
< BlueMatt> it shouldnt, though
< BlueMatt> hmm
< BlueMatt> (thats the section where you commented on the comment you found confusing
< BlueMatt> (it says explicitly that we dont want to clear the db even if we're setting fReindex unless its a new reindex...)
< sipa> well i failed to understand what it was trying to say :)
< BlueMatt> sipa: try now?
< BlueMatt> (comment, that is)
< BlueMatt> didnt change the code
< sipa> ok, will have a look soon
< BlueMatt> so, wait, do you think there is a bug that i should go hunt or do you think you mis-read?
< sipa> i thought there was a bug, but i'm less sure now
< sipa> i'll verify
< BlueMatt> hmmm...this client-side getinfo thing scares me
< BlueMatt> our rpcs are always atomic elements, but if you start calling getinfo, then getwalletinfo, then getmininginfo separately, they could return results based on different views of the current chain
< BlueMatt> which would be astoundingly confusing for users
< sipa> i'm totally fine with deprecating and then deleting
< BlueMatt> well I'm curious what the previously-mentioned "IRC discussion" was that resulted in the pr getting reopened
< BlueMatt> i dont want to show up late to the party and shit on it and say it shouldnt happen, but I'm really uneasy about the idea
< BlueMatt> sipa: do you want the asm sha in for 15?
< BlueMatt> should i bother reviewing it?
< BlueMatt> (yet)
< sipa> BlueMatt: i think it's a great and easy performance win, and i think it's ready
< sipa> whether we want it in 0.15 depends on reviewer interest
< BlueMatt> k
< BlueMatt> i'll try to get to it
< sipa> awesome
< cfields> BlueMatt: yea, will do in a bit. I'm frantically trying to bump qt atm.
< cfields> +1 for sha asm in 0.15
< cfields> sipa: any theories as to what slows down the lto build?
< sipa> cfields: over-eager inlining across objects?
< cfields> makes sense
< gmaxwell> sipa: wumpus had a comment about where the detection was being triggered for the asm.
< cfields> i could see how it might make a mess of our serialization
< sipa> gmaxwell: i've long since addressed that and commented about it
< gmaxwell> I only haven't acked it because I didn't think the patch was in its final state yet... I've tried it on a few hosts, works as expected.
< gmaxwell> sipa: oh
< gmaxwell> oh I see there are more commits now
< sipa> gmaxwell: where 'long' means 15 hours ago
< sipa> it also prints which implementation is being used
< sipa> the only unaddressed comment is BlueMatt's concern about printing copyright information for binary-only distributions
< BlueMatt> we do it for openssl for qt - we have it in the about dialog or smth iirc
< gmaxwell> I don't believe matt is correct in any case; we don't have a "binary only distribution" -- sure you don't have to download the source, but thats up to you.
< sipa> easy enough to add something there, if needed
< gmaxwell> BlueMatt: openssl is 4-clause BSD
< cfields> \o/ a self-test
< sipa> cfields: actually, you should test whether reintroducing the foldgers crystals bug in the sse asm causes it to select the basic sha256 implementation at run time
< sipa> it should
< gmaxwell> sipa: I was thinking of suggesting logging that failure, when cpu id and selftest disagree...
< cfields> sipa: hmm, i wonder if we should still fail in that case though, and require a --software-sha256 flag or something
< sipa> gmaxwell: the crypto code doesn't have access to logging
< cfields> if that's happening in the wild, we really want to know about it
< sipa> we could assert in case the selftest doesn't go as expected
< sipa> i agree that this may hide bugs
< sipa> though in a pretty safe way... i don't think it's easy to screw up SHA256 in a way that wouldn't be detectable with a single test
< gmaxwell> sipa: one thing the selftest doesn't extensively test is sensitivity to alignent.
< sipa> ok, i can fix that
< sipa> i can also extend it to test 0 and/or 2 blocks rather than just a single block
< BlueMatt> gmaxwell: hmm? bitcoin.org/bin has binary distributions
< BlueMatt> we ship binary-only stuff, at least on windows
< BlueMatt> iirc
< sipa> BlueMatt: i think gmaxwell's point is that the source code is still available for those
< sipa> i have no idea whether that matters, IANAL
< BlueMatt> ah, ok
< BlueMatt> wellt hen I dont care
< BlueMatt> as long as his point isnt "we dont ship binary-only" and is instead "but source is freely available", then he would know :)
< gmaxwell> BlueMatt: if it meant what you thought, it wouldn't be GPL compatible. You're interperting the license in the same way that 4-clause bsd is written.
< BlueMatt> k :)
< gmaxwell> BlueMatt: my point was we always make the source available.
< bitcoin-git> [bitcoin] TheBlueMatt reopened pull request #10838: (finally) remove the longest-ever-deprecated RPC call getinfo (master...2017-07-seriously-fuck-getinfo) https://github.com/bitcoin/bitcoin/pull/10838
< BlueMatt> gmaxwell: we dont always *ship* the source along with the binary
< BlueMatt> was my point
< BlueMatt> as long as I'm misreading the liscense, I withdraw my concern :)
< BlueMatt> we just make it available
< gmaxwell> sipa: not something to fix now, but I see that the multiblock code doesn't really make a heroic effort to transform multiple blocks at a time. This may disavantage the avx2-rorx code, which really wants 2 blocks at a time (as it vector processes the initial expander in both blocks)
< sipa> gmaxwell: that's not too hard to fix
< gmaxwell> ya, I know. e.g. change the buffering to buffer two blocks.
< sipa> yeah
< gmaxwell> similar, the 8m rorx code is setup for 8 blocks.