< sipa> achow101: i overlooked it before; i've locked the comments on the first commit
< achow101> yay
< achow101> less spam
< gmaxwell> oh thats why achow was asking. :P
< jtimon> review beg https://github.com/bitcoin/bitcoin/pull/8994 now there's no exception for testing segwit.py and I left the loading of the parameters from aseparated file for later
< achow101> I think I am close to figuring out what happened with #11171, and it is not related to the off screen thing
< achow101> well while trying to debug that thing, I found a different bug :/
< sipa> achow101: you're on a roll
< achow101> sipa: can you help? This is as far as I got: https://github.com/bitcoin/bitcoin/issues/11171#issuecomment-326457725
< achow101> I'm not sure if I've gone down the wrong rabbit hole or if I am just not seeing something
< sipa> achow101: calling a virtual method on a null pointer will cause a segfault, but this is not true for normal methods (though it's undefined behaviour, in practice the method just gets called with a this == nullptr)
< sipa> of course, if the called method tries to access any class fields, a segfault will occur
< achow101> is there a virtual method here?
< sipa> i don't know - i know almost nothing about the GUI code
< kallewoof> Ugh... I didn't realize the mempool propagated quite as slowly as it does. I have 3 public nodes (one has been up for months, one for a week or two, and one was started up yesterday). The mempools on each is quite different. 4827 entries for the longest running one, 2848 for the second and 802 for the third.
< sipa> that's not necessarily due to propagation
< kallewoof> What else could be causing it?
< sipa> if the differences are in transactions that (directly or indirectly) depend on unconfirmed transactions that were in the older running nodes but not in the newer ones, that's expected
< sipa> there is no mempool reconciliation
< kallewoof> So most of the extra cruft would be for low fee txs that are not making it into blocks for a long time?
< kallewoof> Oh wait, I see what you're saying
< kallewoof> Node gets a tx with an input that is unconfirmed and unknown and the node discards it. Or wait, it puts it in orphanTx vector, right? Maybe I can use that to get a better picture..
< kallewoof> sipa: Weird. It looks like a node will ask for parent txs if it runs into a tx with unknown inputs, presuming it hasn't rejected any of its parents before. Doesn't that address what you said?
< kallewoof> for loop over vin that does `if (!AlreadyHave(_inv)) pfrom->AskFor(_inv);` (net_processing.cpp somewhere around line 1900 or so)
< sipa> kallewoof: perhaps they already have a conflict for the parent?
< kallewoof> I could see that be the case for a couple of transactions (having a double spent parent I guess? where one node sees one and the other sees the other), but the old node has 4x the # of txs compared to the 1 day old one. seems high.
< kallewoof> Connection counts are 63, 17, 14 which might mean that some txs simply don't get very far (need lots of peers to see them), but maybe there's another reason.
< kallewoof> s/which might mean/which might indicate/
< meshcollider> Since #11171 is being used to discuss the GUI bug could someone please reopen it :)
< meshcollider> Thanks sipa
< sipa> yw!
< gmaxwell> kallewoof: it will fetch parents but its pretty limited in doing that. It won't fetch things that it just wasn't online to see.
< gmaxwell> unless a child shows up.
< gmaxwell> in my expirence it normally takes about 24 hours of operation before compact blocks reaches its full hitrate.
< gmaxwell> and right now with a flood of very low fee txn you won't pick those up.
< jimpo> kallewoof: Do you mind dumping the mempools and sharing if you have RPC enabled? I'd be interested in taking a look.
< gmaxwell> kallewoof: fwiw, it's a couple line patch to use the mempool rpc to cause a peer to inv you their whole mempool, it's pretty abusive so you should probably only do it to your own nodes. (e.g. make a patch that adds an rpc command to send a mempool message to a particular node id then use it)
< gmaxwell> (or at least use it sparingly against anyone else.)
< kallewoof> jimpo: lemme see what they look like now after the rush of 8k txs first..
< kallewoof> gmaxwell: ahh okay. well, i kind of want these nodes to behave as a normal average joe node so i don't want them to "cheat"
< gmaxwell> kallewoof: sure, depends on what you're testing.
< kallewoof> jimpo: they seem to be a bit closer to what you'd expect now. {11022, 9101, 8525} txs for {69, 13, 17} connection count. still interested?
< gmaxwell> connection count won't matter.
< kallewoof> I assumed connection count would give you better coverage
< gmaxwell> just having been online to see the whole backlog matters mostly.
< gmaxwell> no, except with very few good peers or something everything gets flooded to every link.
< gmaxwell> but old transactions are not sent.
< jimpo> kallewoof: Yeah, I'm still interested. I'll see if I can replicate too, but your nodes have been up a lot longer.
< kallewoof> one has only been up ~24h
< kallewoof> would getrawmempool output be enough or do you want more details?
< jimpo> getrawmempool is great
< gmaxwell> so you don't see things that were broadcast before you came online, except where they get pulled in by children. or by an occasional piece of 'helpful' custom software that connects to you and vomits a bunch of old txn at you for inexplicable reasons.
< jimpo> Yeah, that's probably why
< kallewoof> jimpo: https://bc-2.jp/grm.zip
< kallewoof> gmaxwell: I thought txs were relayed randomly to enhance privacy
< kallewoof> I must've gotten that mixezd up with something else.
< kallewoof> Hum. I don't think that's my issue, but if someone sent a tx with a fee so low it wouldn't go into mempoo, and someone else CPFP'd that, it wouldn't go into the mempool because parent was rejected, I think. That might not be ideal.
< gmaxwell> kallewoof: you can't use CPFP to evade mempool minimums but they are very low.
< gmaxwell> kallewoof: they are relayed with randomized delays, in sorted order, but all are relayed.
< kallewoof> gmaxwell: OK, so that wouldn't explain the difference unless there had been a ton of new txs recently (which wasn't the case at the time).
< gmaxwell> kallewoof: also peers (well, later than 0.12) will not relay you txn which are too low to go into your mempool.
< * kallewoof> nods
< gmaxwell> kallewoof: you can look at the recieved times on all the txn in your mempools, I expect that what you will find that all the txn missing from your lower uptime nodes have recieve times before it started.
< kallewoof> gmaxwell: That's a good idea, yeah.
< meshcollider> I've noticed src/test/testutil.cpp|h seem to basically provide nothing, is there anything wrong with removing those files and calling fs::temp_directory_path() directly instead of GetTempPath() ?
< bitcoin-git> [bitcoin] MeshCollider opened pull request #11210: Stop test_bitcoin-qt touching ~/.bitcoin (master...201708_qt_test_dir) https://github.com/bitcoin/bitcoin/pull/11210
< bitcoin-git> [bitcoin] jjz opened pull request #11212: Change the type of BlockStatus to unsigned int (master...master) https://github.com/bitcoin/bitcoin/pull/11212
< bitcoin-git> [bitcoin] jnewbery opened pull request #11213: Fix getrawtransaction help for per-txout chainstate db (master...getrawtransactionhelp) https://github.com/bitcoin/bitcoin/pull/11213
< bitcoin-git> [bitcoin] jnewbery closed pull request #11213: [trivial] [rpc] Fix getrawtransaction help for per-txout chainstate db (master...getrawtransactionhelp) https://github.com/bitcoin/bitcoin/pull/11213
< bitcoin-git> [bitcoin] MarcoFalke pushed 6 new commits to master: https://github.com/bitcoin/bitcoin/compare/d81dccf191a4...28f788e47e58
< bitcoin-git> bitcoin/master be2a2ab John Newbery: [tests] fix - use rpc_timeout as rpc timeout
< bitcoin-git> bitcoin/master 36b6268 John Newbery: [tests] TestNode: separate add_node from start_node...
< bitcoin-git> bitcoin/master 6cf094a John Newbery: [tests] Avoid passing around member variables in test_framework
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #11121: TestNode tidyups (master...test_node_tidyups) https://github.com/bitcoin/bitcoin/pull/11121
< bitcoin-git> [bitcoin] practicalswift closed pull request #11190: [tests] Remove unused imports (script.OP_0 and script.sha256) (master...remove-unused-import-op_0) https://github.com/bitcoin/bitcoin/pull/11190
< GAit> jnewbery: thanks for the more feedback, updated the PR let me know if/when you want me to squash
< paveljanik> Please close #11202
< jnewbery> MarcoFalke : silent merge conflict for #11121 :( I'll open a fixup PR
< jnewbery> (bitcoin_cli.py didn't have the set_test_params() override)
< bitcoin-git> [bitcoin] jnewbery opened pull request #11215: [tests] fixups from set_test_params() (master...bitcoin_cli_set_test_params) https://github.com/bitcoin/bitcoin/pull/11215
< jnewbery> #11215 fixes it. Master won't be able to pass travis until that's merged.
< jnewbery> wumpus sipa ^^ 2 line change in test file - can review/merge please?
< bitcoin-git> [bitcoin] sipa pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/28f788e47e58...ec20f01ba094
< bitcoin-git> bitcoin/master 3918d93 John Newbery: [tests] fixups from set_test_params()
< bitcoin-git> bitcoin/master ec20f01 Pieter Wuille: Merge #11215: [tests] fixups from set_test_params()...
< bitcoin-git> [bitcoin] sipa closed pull request #11215: [tests] fixups from set_test_params() (master...bitcoin_cli_set_test_params) https://github.com/bitcoin/bitcoin/pull/11215