< achow101> Is there any condition where core won't send a getdata when it receives an inv for a tx?
< achow101> assuming it doesn't already have the tx
< instagibbs> achow101, if it's in the recent rejects filter, which I guess already counts as "has"
< achow101> nvm. I think I found my problem. Shouldn't have MSG_WITNESS_TX in the inv.
< achow101> that didn't work.
< btcdrak> luke-jr: saying BIP1 is deprecated and now use BIP2 is the same as changing the text of BIP1, but more confusing. Let's just change BIP1
< luke-jr> btcdrak: ignoring the way it's supposed to work is far more confusing than following the process
< gmaxwell> I would generally agree for protocols that are deployed.
< gmaxwell> "X operators according to BIP10 not BIP11"
< luke-jr> it's trivial and clear to just put a large banner at the top of BIP 1 and throughout the document that it's obsolete, see BIP 2
< gmaxwell> but yes, thats okay too I guess.
< achow101> has the behavior of segwit peer services diverged from the bip?
< sipa> achow101: elaborate?
< achow101> has their been changes to the format of the getdata message? Or to when core responds to an inv?
< sipa> i don't think so
< achow101> ok
< achow101> I'm trying to debug sending txs with armory and the behavior from core is inconsistent
< achow101> sometimes it will send a getdata in response to the inv, and other times it won't
< sipa> while the node is fully synced?
< achow101> yes. testnet though, and testnet has been super weird lately
< achow101> it works if I unset the NODE_WITNESS service bit in Armory
< achow101> is getdata changed in any significant way in segwit?
< achow101> besides the inv types
< sipa> it doesn't disconnect, right?
< achow101> no
< sipa> just not getdata in response to a MSG_TX inv?
< achow101> sometimes it gets a response
< achow101> but when it does, armory either isn't getting it or doesn't understand it
< achow101> but it works fine without the witness service bit
< sipa> how can it not understand?
< sipa> it's just a getdata
< achow101> idk. works fine without segwit
< achow101> that's why I asked if getdata changed
< sipa> well it will ask for a MSG_WITNESS_TX in response, not an MSG_TX
< achow101> right. and I have that covered. but it's not even making it that far
< achow101> it looks like it isn't even receiving the getdata, but I can't possibly fathom why that would be the case
< sipa> it being armory?
< achow101> yes
< sipa> can you run with -debug=net, and create an excerpt from debug.log with the receival of the inv, and the few folpwong message
< sipa> following
< achow101> 2016-09-26 03:53:37 got inv: tx cf1ae8a9d9b93eaa281a853315a36f9f2ea256752bae0a72e2727522cb82bd1f new peer=1
< achow101> 2016-09-26 03:53:37 askfor witness-tx cf1ae8a9d9b93eaa281a853315a36f9f2ea256752bae0a72e2727522cb82bd1f 0 (00:00:00) peer=1
< achow101> 2016-09-26 03:53:37 Requesting witness-tx cf1ae8a9d9b93eaa281a853315a36f9f2ea256752bae0a72e2727522cb82bd1f peer=1
< achow101> 2016-09-26 03:53:37 sending: getdata (37 bytes) peer=1
< achow101> and that's it. no response with that txid
< sipa> the getdata does not contain a msg_witness_tx for that txid?
< achow101> it should. NODE_WITNESS is set in armory's services
< sipa> but it does not?
< sipa> what does it contain?
< sipa> bitcoin core is sending you a getdata
< achow101> i don't know what it contains. none of my breakpoints are being set off. I can wireshark it though
< sipa> ok
< sipa> it would be good to know where the problem lies
< achow101> this is the getdata from wireshark (well for another tx since I ran it again)
< achow101> 0000 00 00 00 00 00 00 00 00 00 00 00 00 08 00 45 00
< achow101> 0010 00 71 3e 73 40 00 40 06 fe 11 7f 00 00 01 7f 00
< achow101> 0020 00 01 47 9d aa 92 be 7f 69 f8 f7 cf 08 33 80 18
< achow101> 0030 01 5e fe 65 00 00 01 01 08 0a 00 81 8f 34 00 81
< achow101> 0040 8f 34 0b 11 09 07 67 65 74 64 61 74 61 00 00 00
< achow101> 0050 00 00 25 00 00 00 11 8c ea 6c 01 01 00 00 40 1f
< achow101> 0060 bd 82 cb 22 75 72 e2 72 0a ae 2b 75 56 a2 2e 9f
< achow101> 0070 6f a3 15 33 85 1a 28 aa 3e b9 d9 a9 e8 1a cf
< achow101> nvm. I found the problem. I forget the invtype in one place and that screwed the whole thing
< paveljanik> jonasschnelli, the new overlay when syncing/reindexing: is there any way to bring it back once hidden?
< luke-jr> paveljanik: click the icon
< paveljanik> which one? I already tried all of them ;-)
< paveljanik> ah, triangle with ! ;-)
< paveljanik> thank you!
< luke-jr> ☺
< * paveljanik> is a bad UI user 8)
< wumpus> hehe
< luke-jr> nah, I just have the weirdest memory. I remember the PR saying that :P
< jonasschnelli> Yes. Pressing on the warning icon is not really elegant UX
< wumpus> jonasschnelli: are you on MacOSX? can you please check if the libc function daemon() is available?
< jonasschnelli> wumpus: Yes. It's available
< jonasschnelli> daemon(int nochdir, int noclose);
< wumpus> thanks, yes that'sthe one
< jonasschnelli> I'm on OSX 10.10
< wumpus> let's extend this: can anyone with a UNIX-ish OS that is not Linux please check this? I've checked OpenBSD and it does, at least.
< jonasschnelli> daemon is a standard BSD function, BSD is the base-system of darwin (fork)
< wumpus> right
< wumpus> so I think we can just rely on that any OS that support daemonization in the first place and runs bitcoin core has that call
< wumpus> didn't BSD come up with deamons in the first place :)
< wumpus> this would make https://github.com/bitcoin/bitcoin/pull/8278 trivial
< wumpus> even better, we can remove the windows-specific path. Windows doesn't have daemon(), so it wouldn't support --daemonize
< wumpus> going to do a pull for this
< sipa> windows has background services, but their purpose seems a bit different, as they just avoid being tied to abuser session
< gmaxwell> what does-- say-- apache do on windows?
< luke-jr> I would be surprised if it didn't install as a system service
< wumpus> windows is out of scope here
< wumpus> we don't support that yet, and the point of this pull is not to support anything on wnidows
< wumpus> it's just to simplify and improve behavior on UNIX
< sipa> agree, just saying that the corresponding concept on windows has a different goal
< wumpus> windows services are a completely different animal, you can't just spawn them arbitrarily like UNIX daemons, they're more like /etc/init.d services installed as root
< wumpus> oh I agree with that
< sipa> maybe it makes sense to support that when we have done more use for wallet-less/wallet-split support
< wumpus> yes, it may be worthwhile to work on, but it won't share any code with -daemonize, it's more like the "how to install as a system service" guide that we have for some linux distros
< wumpus> I'm afraid it takes a lot of OS-specific code and registry wrangling
< luke-jr> looking over some old code I wrote for a Windows service, it's probably ~100 LOC
< luke-jr> maybe ~200
< wumpus> as well as needs to set up an account to run it under
< wumpus> you won't want to spawn it as ADMINISTRATOR
< luke-jr> >_<
< wumpus> (or "local services" which is pretty much admin-equiv)
< paveljanik> jonasschnelli, I have started testnet Qt with the current master from scratch, only bitcoin.conf remained. There is no overlay window showing it is synchronizing. Should it be displayed?
< paveljanik> Can't click on triangle with excl. mark...
< GitHub26> [bitcoin] laanwj opened pull request #8813: bitcoind: Daemonise using daemon(3) (master...2016_09_daemonize) https://github.com/bitcoin/bitcoin/pull/8813
< paveljanik> I understood that this is the primary use case where it should be shown.
< luke-jr> wumpus: well, Windows isn't going to be secure no matter what user Core runs as <.<
< GitHub11> [bitcoin] laanwj closed pull request #8278: Forking daemon (master...forking-daemon) https://github.com/bitcoin/bitcoin/pull/8278
< wumpus> luke-jr: sure, but if we do it, we need to support best practices
< wumpus> the help message for -daemon is pretty strange: "Run in the background as a daemon and accept commands". Run in the background, check. Accept commands? Don't we always?
< wumpus> (well strictly unless you set -server=0, but it has nothing to do with the daemon option)
< GitHub50> [bitcoin] laanwj closed pull request #8451: Get rid of the const field in CTransaction (master...noconsttx) https://github.com/bitcoin/bitcoin/pull/8451
< btcdrak> gmaxwell: Apache registers Windows services
< wumpus> MarcoFalke: btw https://github.com/jgarzik/univalue/pull/27 is passing now, it was just a temporary hiccum with MacOSX as you thought
< MarcoFalke> jup
< MarcoFalke> jgarzik already merged it
< wumpus> it still shows as open here
< MarcoFalke> oh, my osx fix
< MarcoFalke> I mean
< wumpus> oh okay
< paveljanik> jonasschnelli, please ignore it. It is shown correctly when you use correct tree/binary 8)
< wumpus> I didn't know you did an osx fix
< MarcoFalke> jonasschnelli: Does the sync overlay block the gui for you when you do reindex?
< MarcoFalke> I think we can do this without any lock to cs_main
< MarcoFalke> Will try to create a pull this week.
< MarcoFalke> (Or at least reduce the locking, but get rid of the fHeader "shortcut")
< wumpus> I wonder, is windows on 32 bit even still a thing these days?
< MarcoFalke> Windows XP 32 bit seems to be the most common os these days
< wumpus> it wouldn't make me terribly sad if we had to only support one architecture for windows
< wumpus> well the train for XP support has already failed
< wumpus> :p
< wumpus> I think android is the most common OS these days
< wumpus> but I don't think we should support that out of the box before there is some kind of SPV fallback for the wallet
< wumpus> in any case I think windows 32 bit is dead or dying as a platform. Linux 32-bit still makes some sense for VMs, I suppose.
< wumpus> I mean x86. ARM 32 makes obvious sense.
< GitHub61> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/37871f216e0d...4e1567acff4b
< GitHub61> bitcoin/master 9a75d29 Wladimir J. van der Laan: devtools: Check for high-entropy ASLR in 64-bit PE executables...
< GitHub61> bitcoin/master 62c2915 Wladimir J. van der Laan: build: supply `-Wl,--high-entropy-va`...
< GitHub61> bitcoin/master 4e1567a Wladimir J. van der Laan: Merge #8249: Enable (and check for) 64-bit ASLR on Windows...
< GitHub135> [bitcoin] laanwj closed pull request #8249: Enable (and check for) 64-bit ASLR on Windows (master...2016_06_windows64_security) https://github.com/bitcoin/bitcoin/pull/8249
< wumpus> huh isn't it *daemonize* instead of *daemonise*? confused
< sipa> british vs american?
< sipa> daemonize seems more common
< wumpus> what is the UNIX spelling?
< wumpus> yes, I thought so
< paveljanik> getting testnet IBD finished is a pain...
< wumpus> daemonize appears 2 times in the current source, daemonise 0 times, clear, changing the PR to keep sanity
< wumpus> paveljanik: why so?
< paveljanik> wumpus, doing it the second time here. Both stuck at block ~892320.
< wumpus> stuck in what way?
< paveljanik> 8 peers
< paveljanik> no progress in received blocks
< wumpus> any errors in the log?
< paveljanik> no
< paveljanik> many got inv, received inv
< paveljanik> debug console shows 0 txs in mempool
< paveljanik> it was a rm -rf testnet3 run. In both cases...
< paveljanik> 12 weeks ago in both cases.
< paveljanik> hmm.
< paveljanik> all nodes are 12.99+
< MarcoFalke> paveljanik: A stalling issue?
< paveljanik> yup
< MarcoFalke> ugh
< MarcoFalke> Does it disconnect peers?
< paveljanik> Syncyng headers
< wumpus> I haven't done a testnet sync from scratch in quite a while, maybe I should
< paveljanik> all peers at 947573
< MarcoFalke> Hmm, I saw some slow header syncs yesterday. I blamed my slow internet...
< wumpus> maybe I should try it in a win 32-bit VM for extra masochism points
< wumpus> nah
< sipa> paveljanik: all witness capable peers?
< paveljanik> all NETWORK & BLOOM & WITNESS
< sipa> did you reindex?
< paveljanik> in both cases rm -rf testnet3 full IBD
< paveljanik> from scratch
< wumpus> restarting (e.g., to get new peers) didn't solve it either?
< GitHub167> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/4e1567acff4b...ab0b411868e1
< GitHub167> bitcoin/master faef293 MarcoFalke: [wallet] Add high transaction fee warnings
< GitHub167> bitcoin/master ab0b411 Wladimir J. van der Laan: Merge #8486: [wallet] Add high transaction fee warnings...
< GitHub138> [bitcoin] laanwj closed pull request #8486: [wallet] Add high transaction fee warnings (master...Mf1607-walletHighFeeWarn) https://github.com/bitcoin/bitcoin/pull/8486
< MarcoFalke> Wondering if 8738 should be locked.
< paveljanik> wumpus, no.
< paveljanik> I even tried to rm peers.dat and restart
< MarcoFalke> paveljanik: So debug=net show nothing?
< wumpus> MarcoFalke: oh no, rebroad got involved too
< paveljanik> MarcoFalke, will try
< phantomcircuit_> wumpus: i apologize for how long it takes in advance
< wumpus> MarcoFalke: locked it
< wumpus> jonasschnelli: the comment nit was already solved in #8813, remember that github won't hide changes anymore.
< paveljanik> sent getheaders to all peas, received 947582
< paveljanik> Ignoring getheaders from peer=9 because node is in initial block download
< paveljanik> from all of them
< phantomcircuit_> wumpus: is there a particular pattern we're already using for RAII wrappers for db handles or things?
< phantomcircuit_> it's gonna need to be reference counted
< wumpus> a shared pointer?
< wumpus> std::shared_ptr is automagically reference counted
< paveljanik> looks like we are ignoring too much when n IBD
< phantomcircuit_> wumpus: yeah except none of the functions know whether they were the originally called method
< phantomcircuit_> there's public methods which call each other
< phantomcircuit_> so the first one creates the CWalletDB object and the rest use a private member
< phantomcircuit_> but doing that just with a shared pointer wont work cause the private member isn't destroyed
< wumpus> or create your own RAII wrapper, though I prefer going with existing c++11 features where possible
< wumpus> esp eith reference counting it's kind of easy to introduce off-by-one errors
< jonasschnelli> wumpus: Arg. Yes. I still find it confusing to see old code on the PR page. :) But probably good for clear documentation.
< wumpus> " except none of the functions know whether they were the originally called method" I'd suggest to fix that first
< wumpus> create explicit API methods and internal helper methods
< phantomcircuit_> hmm
< wumpus> this helps with locking too
< phantomcircuit_> yeah i guess just fixing the api first would be the way to go
< MarcoFalke> wumpus: https://travis-ci.org/bitcoin-core/univalue/settings. Is it enabled for pull requests?
< wumpus> and make the internal helper methods private so that external clients won't be tempted into calling them
< wumpus> MarcoFalke: yes
< wumpus> MarcoFalke: both for pushes and prs
< MarcoFalke> Hmm, didn't pick it up: https://github.com/bitcoin-core/univalue/pull/3
< paveljanik> peers send me: getheaders (which I ignore because of still in IBD), sendheaders, sendcmpct, pong, headers
< wumpus> MarcoFalke: bah, no travis buttons either
< jonasschnelli> paveljanik: Ignoring getheaders seems correct..
< jonasschnelli> During IBD
< wumpus> MarcoFalke: it doesnt look like travis is doing anything there
< wumpus> MarcoFalke: no builds at all yet
< MarcoFalke> Oh, maybe it needs at least one build at master...
< wumpus> just going to merge your pull, let's see if that will get travis to test
< wumpus> oh! it's starting
< MarcoFalke> heh
< wumpus> did anyone do anything?
< sipa> paveljanik: did you ever send a getheaders?
< paveljanik> yes, to all peers
< paveljanik> I'm now grepping though the old log, because I'm trying reindex
< paveljanik> and then received: headers (163 bytes) peer=1
< wumpus> can anyone please make a browser extension that hides github's big green 'merge' button? :-)
< jonasschnelli> heh... yes. Some local CSS injection.
< sipa> alternative: can we pay github to add a setting to remove it?
< achow101> why do you want that?
< wumpus> I've already requested that feature once, they actually have a per-repository option to remove it in some cases
< paveljanik> sipa, I sent this: initial getheaders (947583) to peer=1
< wumpus> but you can't disable it in all cases, and they don't intend to do that :(
< paveljanik> ie. I know the current height, but do not have blocks...
< wumpus> achow101: because I don't want to accidentally use that button instead of the script that we wrote for merging+signing
< wumpus> and the button seems to be explicitly designed to be big and green and easy to accidentally click
< wumpus> I guess in the next version it will follow the mouse cursor :p
< MarcoFalke> can it be disabled on events such as travis fails?
< wumpus> yes
< MarcoFalke> We could add another "CI" (maybe a linter) and have it return false all the time
< wumpus> I've thought about that, but I think the UI impact of that is even worse. No green checkmarks anymore
< GitHub193> [bitcoin] laanwj closed pull request #7857: Add fee option to fundrawtransaction (master...enhancement/add-fee-to-fundrawtransaction) https://github.com/bitcoin/bitcoin/pull/7857
< GitHub87> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/ab0b411868e1...bb843adc8d04
< GitHub87> bitcoin/master 381826d Wladimir J. van der Laan: bitcoin-cli: More detailed error reporting...
< GitHub87> bitcoin/master bb843ad Wladimir J. van der Laan: Merge #8722: bitcoin-cli: More detailed error reporting...
< GitHub154> [bitcoin] laanwj closed pull request #8722: bitcoin-cli: More detailed error reporting (master...2016_09_cli_http_error) https://github.com/bitcoin/bitcoin/pull/8722
< GitHub192> [bitcoin] MarcoFalke opened pull request #8814: [wallet, policy] ParameterInteraction: Don't allow 0 fee (master...Mf1607-walletHighFeeWarn) https://github.com/bitcoin/bitcoin/pull/8814
< GitHub105> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/bb843adc8d04...dd20ed1223b9
< GitHub105> bitcoin/master ddddaaf MarcoFalke: [rpc] Deprecate getinfo...
< GitHub105> bitcoin/master fa6e71b MarcoFalke: [qa] Add getinfo smoke tests and rework versionbits test
< GitHub105> bitcoin/master dd20ed1 Wladimir J. van der Laan: Merge #8780: [rpc] Deprecate getinfo...
< GitHub160> [bitcoin] laanwj closed pull request #8780: [rpc] Deprecate getinfo (master...Mf1609-getinfoDeprecate) https://github.com/bitcoin/bitcoin/pull/8780
< MarcoFalke> ^ for release notes, if someone feels like adding this
< GitHub103> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/dd20ed1223b9...8f1fbf36a769
< GitHub103> bitcoin/master c14ffd5 jonnynewbs: [trivial] fix mempool comment (outdated by BIP125)
< GitHub103> bitcoin/master 8f1fbf3 Wladimir J. van der Laan: Merge #8796: [trivial] fix mempool comment (outdated by BIP125)...
< GitHub85> [bitcoin] laanwj closed pull request #8796: [trivial] fix mempool comment (outdated by BIP125) (master...trivial_comment) https://github.com/bitcoin/bitcoin/pull/8796
< paveljanik> -reindex and it is slowly walking up - 892349 now
< paveljanik> happily requesting blocks, getdata...
< paveljanik> Strange. Have to leave now :-(
< GitHub173> [bitcoin] laanwj closed pull request #8772: [0.13] Backports (0.13...backports-0.13) https://github.com/bitcoin/bitcoin/pull/8772
< GitHub162> [bitcoin] laanwj pushed 55 new commits to 0.13: https://github.com/bitcoin/bitcoin/compare/8d9e8adc05f4...254e990ce5c3
< GitHub162> bitcoin/0.13 c6a6291 instagibbs: add witness address to address book...
< GitHub162> bitcoin/0.13 733760a BtcDrak: Update btcdrak signing key...
< GitHub162> bitcoin/0.13 3606b6b instagibbs: Update p2p-segwit.py to reflect correct AskFor behavior...
< MarcoFalke> wumpus: https://github.com/bitcoin/bitcoin/pull/8712 is a trivial cherry-pick, which was missed in ^
< wumpus> yes, I'm working on a new pull for the remaining ones
< wumpus> isn't #8418 needed in 0.13.1 as well?
< wumpus> otherwise some recent pulls become harder to backport as they all make changes in those tests
< sipa> i don't think it ever hurts to backport tests
< sipa> it just may mean more work
< wumpus> well it hurts if the functionality tested doesn't exist
< wumpus> but compactblocks does, right?
< sipa> compact blocks is in 0.13
< wumpus> right
< wumpus> seems #8418 applies without any changes to 0.13 (though haven't actually run the tests yet)
< wumpus> another thing is, #8739 was tagged for 0.13.1, that makes no sense at all without #8418
< sdaftuar> wumpus: it should! i actually thought it was already merged in 0.13
< sdaftuar> sorry about that confusion with 8739
< wumpus> no problem, fixing it now
< GitHub120> [bitcoin] laanwj opened pull request #8815: Backports for 0.13.1 (0.13...2016_09_backports_0_13_1) https://github.com/bitcoin/bitcoin/pull/8815
< paveljanik> the third sync on testnet was OK. Strange.
< paveljanik> slow, but ok
< GitHub176> [bitcoin] czzarr opened pull request #8816: print P2WSH redeemScript in getrawtransaction if it s not a pubkey (master...print-p2wsh-redeemscript-in-getrawtransaction) https://github.com/bitcoin/bitcoin/pull/8816
< GitHub154> [bitcoin] czzarr closed pull request #8816: print P2WSH redeemScript in getrawtransaction if it s not a pubkey (master...print-p2wsh-redeemscript-in-getrawtransaction) https://github.com/bitcoin/bitcoin/pull/8816
< paveljanik> MarcoFalke, can you please check #8808 for the same binaries now? I reverted one hunk (trivial one though 8) which cause me problems here and now travis is OK.
< MarcoFalke> I think I did 20 minutes ago and it didn't match
< GitHub18> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/8f1fbf36a769...2f71490d2179
< GitHub18> bitcoin/master c9ce17b Derek Miller: Trivial: Grammar and capitalization
< GitHub18> bitcoin/master 2f71490 MarcoFalke: Merge #8805: Trivial: Grammar and capitalization...
< GitHub129> [bitcoin] MarcoFalke closed pull request #8805: Trivial: Grammar and capitalization (master...master) https://github.com/bitcoin/bitcoin/pull/8805
< GitHub106> [bitcoin] jnewbery opened pull request #8817: update bitcoin-tx to output witness data (master...bitcoin-tx-witness) https://github.com/bitcoin/bitcoin/pull/8817
< achow101> is there a way to disable segwit activation on regtest?
< btcdrak> another openssl advisory https://www.openssl.org/news/secadv/20160926.txt
< gmaxwell> achow101: change the dates in the chainparams.
< achow101> I guess that's one way to do it..
< sipa> achow101: use a miner that doesn't support segwit :)