< kallewoof> I'm a little confused about coinbase transactions and 'is all from me'. Currently, CWallet::IsAllFromMe will say no to a coinbase transaction, even (it appears) if it's the miner's wallet. The check itself tries to find the prevout in the wallet, but prevout is 0000..., so it obviously won't be there. This causes issues with the IsAllFromMe PR (#12508), which I patched by explicitly saying 'true'
< kallewoof> for coinbase txs in CWallet::IsAllFromMe, but I wonder if this will cause everyone to think they own all coinbase transactions...
< gribble> https://github.com/bitcoin/bitcoin/issues/12508 | IsAllFromMe by kallewoof · Pull Request #12508 · bitcoin/bitcoin · GitHub
< kallewoof> Then again, if it's a coinbase tx, it will have 100+ confirmations, so in all cases where IsAllFromMe matters (mostly unconfirmed, or low confirm count), it's irrelevant.
< wumpus> hmm interesting
< kallewoof> Or is there a case where a 100+ confirm input being mine or not mine makes a difference? I guess in spending it someone could double spend, so ^ is probably not right..
< wumpus> for 100+ confirms there is no difference. The reason for the 100 depth is to make sure it's deep enough to avoid any realistic reorgs causing trouble.
< kallewoof> wumpus: Right. But I realized someone could spend a coinbase tx sending 12.5 btc to me, my node would think it was 'from me' and act accordingly, and then they would double spend it and I would risk losing 12.5 btc.
< luke-jr> huh? I don't understand what you're saying at all
< luke-jr> generated coins are from noone..
< kallewoof> luke-jr: Me neither. I think I'm very confused.
< sipa> from me means that the inputs were coins treated as yours
< kallewoof> Right. So a coinbase input would not be considered yours even if you created the block?
< wumpus> it's impossible to double-spend a coinbase, by definition the coinbase can only be in the block where the reward is taken
< luke-jr> kallewoof: the coinbase isn't really an input at all, just dummy data
< kallewoof> A coinbase transaction has a coinbase input and an output. I was talking about double spending the output.
< kallewoof> luke-jr: The question is, should IsAllFromMe(a coinbase tx) say true or false, for the miner mining it?
< luke-jr> false obviously?
< luke-jr> it's TO the miner, not FROM him
< kallewoof> luke-jr: All right. That makes things a bit tricky, I guess, but I'll dig.
< kallewoof> luke-jr: It doesn't seem like morcos's initial idea in #9167 is compatible with your interpretation, though: "Created a new wallet and walletTx function IsAllFromMe which correctly computes whether all the inputs to a transaction match the requested IsMine filter.
< gribble> https://github.com/bitcoin/bitcoin/issues/9167 | IsAllFromMe by morcos · Pull Request #9167 · bitcoin/bitcoin · GitHub
< sipa> kallewoof: i don't understand why you're concerned about coinbase txn at all
< sipa> coinbases are by definition never from anything
< kallewoof> sipa: IsMine says true and IsAllFromMe says false for coinbase txs
< sipa> yes, as it should?
< kallewoof> Sorry, I meant, IsFromMe and IsAllFromMe say true/false for coinbase
< sipa> yes, why is that a problem?
< kallewoof> Wait, are coinbase transactions from me or not from me? If IsFromMe says true and there's only a coinbase input why would IsAllFromMe say false? Shouldn't both say the same in this case?
< kallewoof> luke-jr says false is correct. In which case IsFromMe is wrong, no?
< sipa> wait
< sipa> oh, IsFromMe is true?
< kallewoof> Yes
< sipa> that i don't understand!
< sipa> maybe it's a special rule for foinbase tzn
< sipa> coinbase tzn
< kallewoof> All it does is check GetDebit(tx, ISMINE_ALL) > 0
< sipa> txn
< kallewoof> Right -- I'm saying I need to add that to CWallet::IsAllFromMe as well.
< kallewoof> that=the special rule for coinbase txs
< sipa> what is IsAllFromMe even used for?
< kallewoof> That's a great question. It seems to be used in feebumper, and in wallet.cpp in some places.
< kallewoof> Actually no it's only used in feebumper on master.
< sipa> yay
< kallewoof> So it seems completely fine to allow the special case.
< sipa> well, you can't feebump a coinbase!
< * kallewoof> nods
< bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/27278dffe877...4781813b5617
< bitcoin-git> bitcoin/master 08b17de Karl-Johan Alm: [arith_uint256] Do not destroy *this content if passed-in operator may reference it
< bitcoin-git> bitcoin/master b120f7b Karl-Johan Alm: [test] Add tests for self usage in arith_uint256
< bitcoin-git> bitcoin/master 4781813 Wladimir J. van der Laan: Merge #12537: [arith_uint256] Make it safe to use "self" in operators...
< bitcoin-git> [bitcoin] laanwj closed pull request #12537: [arith_uint256] Make it safe to use "self" in operators (master...uint-safe-self-op) https://github.com/bitcoin/bitcoin/pull/12537
< bitcoin-git> [bitcoin] VizXu opened pull request #12919: no message (0.8...master) https://github.com/bitcoin/bitcoin/pull/12919
< bitcoin-git> [bitcoin] fanquake closed pull request #12919: no message (0.8...master) https://github.com/bitcoin/bitcoin/pull/12919
< bitcoin-git> [bitcoin] kallewoof opened pull request #12920: test: Fix sign for expected values (master...test-signs) https://github.com/bitcoin/bitcoin/pull/12920
< bitcoin-git> [bitcoin] Empact opened pull request #12921: Make use of cpuid.h bit definitions (master...cpuid-bits) https://github.com/bitcoin/bitcoin/pull/12921
< wumpus> does anyone happen to have an arm32 system with crc32 feature in /proc/cpuinfo?
< wumpus> I want to port https://github.com/laanwj/crcbench to arm32 (with the goal of integrating that into leveldb) however I currently have no wway to test. Well I could check if it compiles, I guess.
< wumpus> oh right, I guess 64-bit ARM can run 32-bit executables
< MarcoFalke> PSA: We now have 15 parallel jobs on travis for the next year :3
< wumpus> awesome, thank you MarcoFalke :)
< MarcoFalke> Thanks to ChainCode for sponsoring that!
< wumpus> yes!
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/4781813b5617...6fc5a050f216
< bitcoin-git> bitcoin/master bf08fc5 Karl-Johan Alm: test: Assert on correct variable
< bitcoin-git> bitcoin/master 6fc5a05 MarcoFalke: Merge #12918: test: Assert on correct variable...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #12918: test: Assert on correct variable (master...test-typo-rpcrawtx) https://github.com/bitcoin/bitcoin/pull/12918
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/6fc5a050f216...a04440feb9c9
< bitcoin-git> bitcoin/master 280023f practicalswift: Remove duplicate includes
< bitcoin-git> bitcoin/master c36b720 practicalswift: Add Travis check for duplicate includes...
< bitcoin-git> bitcoin/master a04440f MarcoFalke: Merge #11878: Add Travis check for duplicate includes...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #11878: Add Travis check for duplicate includes (master...lint-includes) https://github.com/bitcoin/bitcoin/pull/11878
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/a04440feb9c9...cd8e45b4e75a
< bitcoin-git> bitcoin/master c198dc0 Jan Čapek: [Doc] Clarify the meaning of fee delta not being a fee rate in prioritisetransaction RPC
< bitcoin-git> bitcoin/master cd8e45b MarcoFalke: Merge #12007: [Doc] Clarify the meaning of fee delta not being a fee rate in prioritisetransaction RPC...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #12007: [Doc] Clarify the meaning of fee delta not being a fee rate in prioritisetransaction RPC (master...master) https://github.com/bitcoin/bitcoin/pull/12007
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/cd8e45b4e75a...603975b96a15
< bitcoin-git> bitcoin/master 9142dfe practicalswift: Use explicit casting in cuckoocache's compute_hashes(...) to clarify integer conversion
< bitcoin-git> bitcoin/master 603975b MarcoFalke: Merge #12770: Use explicit casting in cuckoocache's compute_hashes(...) to clarify integer conversion...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #12770: Use explicit casting in cuckoocache's compute_hashes(...) to clarify integer conversion (master...cuckoocache) https://github.com/bitcoin/bitcoin/pull/12770
< aj> MarcoFalke: wow!
< wumpus> so apparently the crc32 extension only exists on armv8, but I didn't know 32-bit-only armv8 existed but apparently it does (Cortex-A32)
< wumpus> anyhow getauxv trick to detect cpu features works; if (getauxval(AT_HWCAP) & HWCAP_CRC32) { on aarch64 and if (getauxval(AT_HWCAP2) & HWCAP2_CRC32) { on arm32
< MarcoFalke> Anyone else seeing this: #12915?
< gribble> https://github.com/bitcoin/bitcoin/issues/12915 | Segmentation fault in util: ScheduleBatchPriority · Issue #12915 · bitcoin/bitcoin · GitHub
< aj> hmm, i thought designator initialisers (param{.sched_priority=0}) weren't ok for c++11 (http://en.cppreference.com/w/cpp/language/aggregate_initialization says they're c++20 fwiw)
< wumpus> MarcoFalke: nope. it crashes inside pthread_setschedparam? that's curious
< wumpus> the only way I see that can happen if is a null pointer is passed, but as it is passing a pointer to a static variable you'd say that's not possible
< wumpus> does this only happen with the sanitize ron?
< aj> err, should it be pthread_self() not 0 as the first argument?
< wumpus> uhmm
< wumpus> the man page doesn't say anything about 0 being a valid value, at least
< wumpus> MarcoFalke: can you try that ^^
< MarcoFalke> happens also with sanitizer off
< MarcoFalke> I've seen segfaults in the unit test a couple of days ago, but they went away after a reboot. Really sketchy
< wumpus> should definitely be pthread_self() - non-determinism in thread ids might explain why it sometimes crashes and sometimes not?
< MarcoFalke> For me it always faults
< MarcoFalke> The unit test thing was a different topic. Just related because it was also a segfault
< MarcoFalke> Making it "const static sched_param param{0};" didn't help
< bitcoin-git> [bitcoin] laanwj opened pull request #12923: util: Pass pthread_self() to pthread_setschedparam instead of 0 (master...2018_04_pthread_self) https://github.com/bitcoin/bitcoin/pull/12923
< MarcoFalke> will try that ^
< MarcoFalke> aj: wumpus: Thx. Work for me now
< * MarcoFalke> subway
< wumpus> I can make the initializer change in the same PR, but as it compiles in c++11 mode, I'm not sure .
< aj> https://stackoverflow.com/questions/43471743/pthread-self-on-linux -- suggests that without linking pthreads, pthread_self() will sometimes/often/?? return 0
< wumpus> that explains why it works for some
< MarcoFalke> aj: I added the cout and it is definetly not 0 for me
< wumpus> it depends on the specifics of the pthread implementation, after all posix threads is an interface, not a specific implementation
< luke-jr> aj: if you don't link pthreads, pthread_self won't resolve at all, and calling it would be a segfault..
< luke-jr> I would expect
< wumpus> so one should not make any assumptions about the value that it returns
< aj> luke-jr: works fine for me without linking libpthread, value is very non-zero either way though. (i don't get a segfault with marcofalke's test case)
< luke-jr> aj: well, glibc no longer has libpthread anymore
< luke-jr> (threading is part of the libc now, and linking libpthread explicitly actually can have harmful side effects)
< wumpus> about time that threading moved into libc, even little embedded SoCs are mulitcore these days, having threading as something special/optional is just absurd
< bitcoin-git> [bitcoin] jamesob closed pull request #12873: [ci] Run functional tests using bitcoin-qt in one Travis job (master...2018-04-03-travis-func-qt) https://github.com/bitcoin/bitcoin/pull/12873
< luke-jr> wumpus: would be nice if they didn't have bugs doing it though :p
< luke-jr> (with glibc, if you link libpthread, and then call vfork early on, it will just return a pointer to the vfork function rather than actually forking)
< aj> jamesob: maybe just open a new PR instead? :(
< jamesob> aj: yeah, probably a good idea. Wonder why/how that PR is screwing travis up.
< jamesob> sorry for the spam
< jonasschnelli> wumpus: curious: whats the reason for implementing NI crc for arm 32bit?
< wumpus> jonasschnelli: last time I checked, quite a lot of time is spent crcing (for leveldb checksums), while verifying the chain
< jonasschnelli> wumpus: Good to know. I just started to play with the ODROID HC2 (Cortex-A15)
< wumpus> so using those extensions on hardware that support them probably helps, though I agree the 32-bit case is unlikley to be hit
< wumpus> (as 32-bit armv8 are very rare)
< wumpus> ah, nice
< jonasschnelli> I guess Odroids XU4 and HC2 are 32 bit armv7?
< jonasschnelli> Not sure if the have NI crc32
< jonasschnelli> Don't have access until Thursday to the machine... so no /proc/cpuinto right now
< wumpus> armv7 never has crc32
< jonasschnelli> Just read that up... v8 is min, right.
< jonasschnelli> v8.1-A AFAIK
< bitcoin-git> [bitcoin] jnewbery opened pull request #12924: Fix hdmaster-key / seed-key confusion (scripted diff) (master...master_key_to_seed) https://github.com/bitcoin/bitcoin/pull/12924
< bitcoin-git> [bitcoin] jonasschnelli closed pull request #12094: Fix hdmaster-key / seed-key confusion (master...2018/01/hdseed) https://github.com/bitcoin/bitcoin/pull/12094
< bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/603975b96a15...a7cbe38ae2dd
< bitcoin-git> bitcoin/master cff66e6 Wladimir J. van der Laan: util: Pass pthread_self() to pthread_setschedparam instead of 0...
< bitcoin-git> bitcoin/master b86730a Wladimir J. van der Laan: util: Remove designator initializer from ScheduleBatchPriority...
< bitcoin-git> bitcoin/master a7cbe38 Wladimir J. van der Laan: Merge #12923: util: Pass pthread_self() to pthread_setschedparam instead of 0...
< bitcoin-git> [bitcoin] laanwj closed pull request #12923: util: Pass pthread_self() to pthread_setschedparam instead of 0 (master...2018_04_pthread_self) https://github.com/bitcoin/bitcoin/pull/12923
< bitcoin-git> [bitcoin] Empact closed pull request #12921: Make use of cpuid.h bit definitions (master...cpuid-bits) https://github.com/bitcoin/bitcoin/pull/12921
< promag> it it me or CWalletTx::GetRequestCount() is not used?
< instagibbs> promag, src/interface/wallet.cpp:95: result.request_count = wtx.GetRequestCount();
< promag> rigth, but request_count is not used
< promag> oh sorry, it is
< promag> my bad :/
< instagibbs> np
< bitcoin-git> [bitcoin] jonasschnelli opened pull request #12925: [Trivial] Logprint the start of a rescan (master...2018/04/rescan) https://github.com/bitcoin/bitcoin/pull/12925
< jamesob> now that we're not supporting Python2, can we start adding function (type) annotations to functional test code? I think that'll help make the test framework a bit easier to use
< bitcoin-git> [bitcoin] laanwj pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/a7cbe38ae2dd...0700b6f778d9
< bitcoin-git> bitcoin/master ea23945 Russell Yanofsky: scripted-diff: Rename wallet database classes...
< bitcoin-git> bitcoin/master 398c6f0 Russell Yanofsky: Update walletdb comment after renaming....
< bitcoin-git> bitcoin/master 9b0f0c5 Russell Yanofsky: Add m_ prefix to WalletBatch::m_batch
< bitcoin-git> [bitcoin] laanwj closed pull request #11851: scripted-diff: Rename wallet database classes (master...pr/wren) https://github.com/bitcoin/bitcoin/pull/11851
< wumpus> jamesob: I guess so
< jamesob> I'm thinking particularly for arguments in util functions like "pubkey" and "blockhash" which are pretty ambiguous
< wumpus> I've never used type annotations in python so I don't know how useful they are
< jamesob> the use is pretty limited out of the box; they're like more succinct docstrings. there are third-party tools that do verification, though I've never used any of them
< wumpus> I'd say make an example PR where you change a few functions, then see how the review goes
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/0700b6f778d9...cf8073f8d1d1
< bitcoin-git> bitcoin/master fab9095 MarcoFalke: qa: Windows fixups for functional tests
< bitcoin-git> bitcoin/master cf8073f MarcoFalke: Merge #12917: qa: Windows fixups for functional tests...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #12917: qa: Windows fixups for functional tests (master...Mf1804-qaWinFixups) https://github.com/bitcoin/bitcoin/pull/12917
< jamesob> wumpus: sounds good
< bitcoin-git> [bitcoin] sipa opened pull request #12926: Run unit tests in parallel (master...201804_parunit) https://github.com/bitcoin/bitcoin/pull/12926
< bitcoin-git> [bitcoin] trulex opened pull request #12927: Docs: fixed link, replaced QT with Qt (master...patch-1) https://github.com/bitcoin/bitcoin/pull/12927
< jnewbery> wumpus: I'm adding #12892 as high priority for review. Please go ahead and remove it if you don't think it merits that.
< gribble> https://github.com/bitcoin/bitcoin/issues/12892 | [wallet] [rpc] introduce label API for wallet by jnewbery · Pull Request #12892 · bitcoin/bitcoin · GitHub
< jnewbery> oh, looks like I'm not allowed to. It needs triage into one of the columns
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/cf8073f8d1d1...7ee6fc58f87b
< bitcoin-git> bitcoin/master 23abfb7 Steve Lee: added logging line back that was accidentally removed with #10762
< bitcoin-git> bitcoin/master 7ee6fc5 MarcoFalke: Merge #12845: Trivial: Add logging line in init.cpp that was accidentally removed with #10762...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #12845: Trivial: Add logging line in init.cpp that was accidentally removed with #10762 (master...add_logging_line_to_newer_branch) https://github.com/bitcoin/bitcoin/pull/12845
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/7ee6fc58f87b...06ead15050f5
< bitcoin-git> bitcoin/master 7039319 Darko Janković: Docs: fixed link, replaced QT with Qt
< bitcoin-git> bitcoin/master 06ead15 MarcoFalke: Merge #12927: Docs: fixed link, replaced QT with Qt...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #12927: Docs: fixed link, replaced QT with Qt (master...patch-1) https://github.com/bitcoin/bitcoin/pull/12927
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/06ead15050f5...727175a08dff
< bitcoin-git> bitcoin/master 75d0e4c Suhas Daftuar: [qa] Delete cookie file before starting node...
< bitcoin-git> bitcoin/master 727175a MarcoFalke: Merge #12902: [qa] Handle potential cookie race when starting node...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #12902: [qa] Handle potential cookie race when starting node (master...2018-04-improve-dbcrash-restarts) https://github.com/bitcoin/bitcoin/pull/12902
< bitcoin-git> [bitcoin] practicalswift opened pull request #12928: qt: Initialize variables previously neither defined where defined nor in constructor (master...qt-constructors) https://github.com/bitcoin/bitcoin/pull/12928