< cfields> sdaftuar: https://github.com/theuni/bitcoin/tree/move-addrdb . It builds on top of #11457, mostly as a cheap review beg :)
< gribble> https://github.com/bitcoin/bitcoin/issues/11457 | Introduce BanMan by theuni · Pull Request #11457 · bitcoin/bitcoin · GitHub
< cfields> It still needs a few things. At minimum, it needs an interface class (similar to NetEventsInterface) to be useful for your tests. Then a few things to tidy up in CConnman, as it was just kinda shoved in there as-is.
< cfields> I'll try to get it finished up tomorrow. But I think that gets you mostly what you were after.
< cfields> oh, and loading the addrdb is very shoe-horned in there. Need to figure out where to stick that. IMO moving it into CAddrMan makes sense, as other implementations (tests) will likely be memory-backed anyway.
< ossifrage> This may be me seeing things, of the 87 nodes connected, the ones with the longest ping times are all 0.15.0
< kallewoof> ossifrage: not seeing that on my end
< bitcoin-git> [bitcoin] sipa pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/808c84f89d0e...26fee4f6bd9a
< bitcoin-git> bitcoin/master 258d33b Karl-Johan Alm: [mempool] Mark unaccepted txs present in mempool as 'already there'....
< bitcoin-git> bitcoin/master 26fee4f Pieter Wuille: Merge #11062: [mempool] Mark mempool import fails that were found in mempool as 'already there'...
< sipa> kallewoof: ^ i was confused to not see a 'already there' when starting up a master node, and then realized that wasn't merged yet
< bitcoin-git> [bitcoin] sipa closed pull request #11062: [mempool] Mark mempool import fails that were found in mempool as 'already there' (master...mempool-alreadythere) https://github.com/bitcoin/bitcoin/pull/11062
< gmaxwell> sipa: kallewoof: thanks.
< * kallewoof> cheers :)
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/26fee4f6bd9a...ffa5159cefb8
< bitcoin-git> bitcoin/master 207408b Jonas Schnelli: Fix crash via division by zero assertion
< bitcoin-git> bitcoin/master ffa5159 Wladimir J. van der Laan: Merge #11508: Fix crash via division by zero assertion...
< bitcoin-git> [bitcoin] laanwj closed pull request #11508: Fix crash via division by zero assertion (master...2017/10/qt_cc_crash_zero) https://github.com/bitcoin/bitcoin/pull/11508
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/ffa5159cefb8...b645f368f299
< bitcoin-git> bitcoin/master 7104de8 João Barbosa: [wallet] Fix leak in CDB constructor...
< bitcoin-git> bitcoin/master b645f36 Wladimir J. van der Laan: Merge #11492: [wallet] Fix leak in CDB constructor...
< promag> doesn't make sense to "return false" here: https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L941 ?
< bitcoin-git> [bitcoin] laanwj closed pull request #11492: [wallet] Fix leak in CDB constructor (master...2017-10-cdb-constructor-leak) https://github.com/bitcoin/bitcoin/pull/11492
< promag> if fTxIndex and !ReadTxIndex why continue?
< sipa> promag: makes sense
< promag> PR on the way
< promag> btw
< promag> sometimes why on earth AssertLockHeld fails on some travis jobs?
< promag> any insight?
< sipa> that would indicate a bug..
< promag> right, I've added the AssertLockHeld, but only some builds fail
< promag> s/builds/jobs
< sipa> not all are built with -DDEBUG_LOCKS
< wumpus> there is significant overhead to lock checking so it is only enabled for debug builds
< promag> you mean debug_lockorder?
< wumpus> yes
< promag> I understand, I'll check that out
< sipa> we have an ad-hoc lock contention/correctness checking, which is compiled in when you build with -DDEBUG_LOCKORDER
< Banzai10> promag, do you have the logs?
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/b645f368f299...2ca518deff25
< bitcoin-git> bitcoin/master c5dfa90 Cristian Mircea Messel: [tests] Add uacomment tests...
< bitcoin-git> bitcoin/master 2ca518d Wladimir J. van der Laan: Merge #11486: [tests] Add uacomment tests...
< bitcoin-git> [bitcoin] laanwj closed pull request #11486: [tests] Add uacomment tests (master...test_uacomment) https://github.com/bitcoin/bitcoin/pull/11486
< promag> forever alone #11514 :P
< gribble> https://github.com/bitcoin/bitcoin/issues/11514 | Iterate and remove nodes without container copy by promag · Pull Request #11514 · bitcoin/bitcoin · GitHub
< promag> wumpus: now that 1 line pr is done, take this with 2 lines https://github.com/bitcoin/bitcoin/pull/11006/files
< wumpus> ah yes that one looks good to me now
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/2ca518deff25...a1d78b59fc03
< bitcoin-git> bitcoin/master 793667a João Barbosa: Improve shutdown process
< bitcoin-git> bitcoin/master a1d78b5 Wladimir J. van der Laan: Merge #11006: Improve shutdown process...
< promag> :confetti:
< bitcoin-git> [bitcoin] laanwj closed pull request #11006: Improve shutdown process (master...201708-fast-shutdown) https://github.com/bitcoin/bitcoin/pull/11006
< wumpus> vNodes.erase(remove(vNodes.begin(), vNodes.end(), pnode), vNodes.end()); , huh
< wumpus> so std::vector::erase removes elements in a range, remove() returns the range vNodes.begin() to vNodes.end() minus pnode
< wumpus> shouldn't this remove everything but pnode?
< wumpus> std::remove "Transforms the range [first,last) into a range with all the elements that compare equal to val removed, and returns an iterator to the new end of that range."
< cfields> I had to look it up first time I saw it
< wumpus> whoa that could have at least used a comment
< wumpus> oh no did i break travis?
< wumpus> E: Failed to fetch https://packagecloud.io/basho/riak/ubuntu/dists/trusty/main/source/Sources 401 Unauthorized no, apparently a download failure
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/a1d78b59fc03...50d72b357081
< bitcoin-git> bitcoin/master f4c4e38 John Newbery: [trivial] Make namespace explicit for is_regular_file...
< bitcoin-git> bitcoin/master 50d72b3 Wladimir J. van der Laan: Merge #11495: [trivial] Make namespace explicit for is_regular_file...
< sdaftuar> hm, this looks like a pretty sketchy comment: https://github.com/bitcoin/bitcoin/issues/11519#issuecomment-337453387
< bitcoin-git> [bitcoin] laanwj closed pull request #11495: [trivial] Make namespace explicit for is_regular_file (master...explicit_is_regular_file) https://github.com/bitcoin/bitcoin/pull/11495
< wumpus> right, deleted it, it made no sense at all (download the app to your mobile? bitcoin.com?)
< sdaftuar> yeah, thanks - your response looks much better :)
< bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/50d72b357081...6759a24eaaf7
< bitcoin-git> bitcoin/master fa9de37 MarcoFalke: qa: Make tmpdir option an absolute path...
< bitcoin-git> bitcoin/master fafa003 MarcoFalke: qa: Remove never used return value of sync_with_ping
< bitcoin-git> bitcoin/master 6759a24 Wladimir J. van der Laan: Merge #11472: qa: Make tmpdir option an absolute path, misc cleanup...
< bitcoin-git> [bitcoin] laanwj closed pull request #11472: qa: Make tmpdir option an absolute path, misc cleanup (master...Mf1710-qaMultiwalletRelDir) https://github.com/bitcoin/bitcoin/pull/11472
< wumpus> thanks :)
< wumpus> can we solve the basho riak download issue? I get an angry travis mail a few seconds after every merge now
< sipa> ?
< wumpus> I have no idea why it's downloading that package in the first place, part of travis infrastructure I guess... hm in that case there's nothing we can do
< bitcoin-git> [bitcoin] laanwj pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/6759a24eaaf7...937613d215de
< bitcoin-git> bitcoin/master c6b07fd practicalswift: Fix a vs. an typo
< bitcoin-git> bitcoin/master 68feb49 practicalswift: Use nullptr instead of NULL
< bitcoin-git> bitcoin/master 0aacfa4 practicalswift: Remove accidental stray semicolon
< bitcoin-git> [bitcoin] laanwj closed pull request #11467: Fix typos. Use nullptr instead of NULL. (master...cleanups-20171009) https://github.com/bitcoin/bitcoin/pull/11467
< cfields> sdaftuar: in case you missed it, see backlog re moving addrman
< sdaftuar> cfields: thanks, i got it -- reviewing #11457 now in fact
< gribble> https://github.com/bitcoin/bitcoin/issues/11457 | Introduce BanMan by theuni · Pull Request #11457 · bitcoin/bitcoin · GitHub
< cfields> ok. with an added interface class, is that what you were after?
< sdaftuar> i think so, yeah (well i haven't looked at the addrman change yet, but if it's just like the banman change, then yes)
< sdaftuar> i think the idea would be in regtest mode, to create a special mockedaddrman and sub it in. so just would want a (minimal) interface class to have to implement
< cfields> sdaftuar: ok
< cfields> though, my hope was that creating interfaces for these would allow us to do actual c++ unit tests, rather than having to launch an entire program instance for tests
< sdaftuar> hm. that would be better!
< sdaftuar> do you think we could really unit test interactions between net and net_processing though? that seems very hard.
< sdaftuar> (then again, what i'm doing now is also very hard)
< cfields> well we're getting close. the message processor is now an interface
< cfields> so you can define your own PeerLogicValidation and ProcessMessages as necessary
< cfields> i assume you need a good bit of stuff pulled out still, though?
< cfields> you're trying to test the outgoing peer eviction here, right?
< sdaftuar> yeah i want to test the logic i'm adding in #11490
< gribble> https://github.com/bitcoin/bitcoin/issues/11490 | Disconnect from outbound peers with bad headers chains by sdaftuar · Pull Request #11490 · bitcoin/bitcoin · GitHub
< cfields> ah thaks, i didn't realize you'd already PR'd it. Will review.
< sdaftuar> i have one more commit that is possibly even harder to test than what i've pushed so far, actually. what is there now is mostly just behavior in net_processing which might be sort-of testable (i was able to rig a single peer in regtest mode that was correctly disconnected)
< sdaftuar> but the next commit, where i have net_processing set an able_to_evict flag for net.cpp to choose to use, is trickier
< sdaftuar> as i have no way to get multiple outbound connections in our regtest environment righ tnow
< sdaftuar> that's partly due to addrman behavior (no more than one peer per ip) and net (don't connect out to a peer in the same network group as an existing peer)
< cfields> sdaftuar: one thing i have in one of my libevent branches is an ip spoofer for CNode. The idea is to setup 2 CConnman, have 1 connect x times to the other, each with a specified (fake) IP. Then you can simulate the behavior of both sides of the conenection.
< sdaftuar> so that aspect of it might be easier to do in a unit test than regtest
< cfields> sounds like it would be, yes. Otherwise you have to shove in a bunch of test-specific behavior
< sdaftuar> i don't think i'm very familiar right now with the existing unit tests that use connman, i should start there
< cfields> heh, well there really aren't any. I'm desperately trying to get the interfaces stubbed out so that we can start writing them :)
< sdaftuar> well i just noticed the existing DoS_tests file while reviewing your banman pr, that is already more than i thought we could do!
< sdaftuar> so maybe it's not crazy to think i could write a unit test with just what we have now
< cfields> yes, that one is a pretty good example of where I'd like to be heading. It allows for all local behavior without relying on a global instance
< cfields> Something else I really needed when testing the libevent code was a way to flood a peer with garbage and see how it would react. Garbage from 1 peer, 100 peers, etc. I think that's the perfect use-case for this kind of testing, because it's easy to code up a quick stub of a message processor that just does ping/pong as quickly as possible.
< cfields> I could try to code that up if it'd be helpful
< sdaftuar> thanks, not sure yet what i need until i think about the unit testing framework a bit more! i'll dive into it and see what issues i run into
< cfields> ok
< cfields> sdaftuar: thanks for the review!
< sdaftuar> np!
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/937613d215de...02ac8c892b1f
< bitcoin-git> bitcoin/master cc9ee80 João Barbosa: Improve ZMQ functional test
< bitcoin-git> bitcoin/master 02ac8c8 Wladimir J. van der Laan: Merge #11452: Improve ZMQ functional test...
< bitcoin-git> [bitcoin] laanwj closed pull request #11452: Improve ZMQ functional test (master...2017-10-improve-zmq-test) https://github.com/bitcoin/bitcoin/pull/11452
< bitcoin-git> [bitcoin] theuni opened pull request #11521: travis: move back to the minimal image (master...travis-minimal) https://github.com/bitcoin/bitcoin/pull/11521
< bitcoin-git> [bitcoin] laanwj pushed 26 new commits to 0.15: https://github.com/bitcoin/bitcoin/compare/51bad9195eb4...1646f9c76036
< bitcoin-git> bitcoin/0.15 9e8aae3 Karl-Johan Alm: [wallet] Close DB on error....
< bitcoin-git> bitcoin/0.15 50bd3f6 practicalswift: Avoid returning a BIP9Stats object with uninitialized values...
< bitcoin-git> bitcoin/0.15 b278a43 Wladimir J. van der Laan: rpc: Write authcookie atomically...
< cfields> wumpus: I forgot to mention in the PR, but I believe 11521 would've mitigated today's travis issue
< ossifrage> This is very strange, the 'copy addess' (all copy to clipboard) functions just stop running on bitcoin-qt (0.15.0 linux), other programs talk to the cut buffer just fine, but not bitcoin-qt
< ossifrage> Ah, there is console output: "Warn Dissector bug, protocol Bitcoin, in packet 3560375: proto.c:5519: failed assertion "idx >= 0 && idx < num_tree_types""
< ossifrage> Never mind, that has nothing to do with this...
< ossifrage> I can't even copy text from the transaction details dialog...
< DEV_> we are working on a development project of the blockchain any donations will be welcome we will update you on the progress of the project as you go btc wallet : 1LJBnabTgobc6xpgH92eV7yha36KSMvtSP