< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/452acee4da20...e50853501b79
< bitcoin-git> bitcoin/master e1b6436 Hennadii Stepanov: Fix build after pr 15266 merged
< bitcoin-git> bitcoin/master e508535 Wladimir J. van der Laan: Merge #15347: Fix build after pr 15266 merged
< bitcoin-git> [bitcoin] laanwj merged pull request #15347: Fix build after pr 15266 merged (master...20190205-fix15266) https://github.com/bitcoin/bitcoin/pull/15347
< bitcoin-git> [bitcoin] Empact closed pull request #15344: travis: Enable functional tests against Trusty (master...trusty-functional) https://github.com/bitcoin/bitcoin/pull/15344
< meshcollider> provoostenator: I've just cleaned up #14491 now, feel free to rebase your stuff on top of it again :)
< gribble> https://github.com/bitcoin/bitcoin/issues/14491 | Allow descriptor imports with importmulti by MeshCollider · Pull Request #14491 · bitcoin/bitcoin · GitHub
< meshcollider> whats up with the "Reading package lists...
< meshcollider> No output has been received in the last 10m0s" failures on travis atm
< meshcollider> nvm its working now
< meshcollider> anyone willing to give #14667 a quick review?
< gribble> https://github.com/bitcoin/bitcoin/issues/14667 | Add deriveaddresses RPC util method by Sjors · Pull Request #14667 · bitcoin/bitcoin · GitHub
< instagibbs> pure utility rpcs? Not that I wouldn't appreciate the functionality but I don't think the motivation for adding it versus other utility rpcs has been given
< promag> 0.18 list is quite big
< fanquake> promag Some stuff on there that'll get dropped, as well as issues that dont actually require any more action
< fanquake> sipa I know it's arbitrary, but how long would you expect test-exhaust (from minisketch) to run for?
< bitcoin-git> [bitcoin] MarcoFalke pushed 8 commits to master: https://github.com/bitcoin/bitcoin/compare/e50853501b79...bbdcc0b0ff0e
< bitcoin-git> bitcoin/master 95a812b Russell Yanofsky: Rename ScanResult stop_block field
< bitcoin-git> bitcoin/master a8d645c Russell Yanofsky: Update ScanForWalletTransactions result comment
< bitcoin-git> bitcoin/master db2d093 Russell Yanofsky: Add suggested rescanblockchain comments
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15342: Suggested wallet code cleanups from #14711 (master...pr/wclean) https://github.com/bitcoin/bitcoin/pull/15342
< bitcoin-git> [bitcoin] dongcarl opened pull request #15348: doc: Add separate productivity notes document (master...2019-02-productivity-md) https://github.com/bitcoin/bitcoin/pull/15348
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/bbdcc0b0ff0e...fc21bb4e3590
< bitcoin-git> bitcoin/master 851380c Gregory Sanders: remove deprecated mentions of signrawtransaction from fundraw help
< bitcoin-git> bitcoin/master fc21bb4 MarcoFalke: Merge #15245: remove deprecated mentions of signrawtransaction from fundra...
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15245: remove deprecated mentions of signrawtransaction from fundraw help (master...fundraw_signraw) https://github.com/bitcoin/bitcoin/pull/15245
< dongcarl> I'm talking to distro maintainers about #12255, when checking if a directory is a valid bitcoin directory, would checking `[ -f foo/bar/peers.dat ]` be sufficient? Would it be better to check `[ -f foo/bar/.lock ]`?
< gribble> https://github.com/bitcoin/bitcoin/issues/12255 | Update bitcoin.service to conform to init.md by dongcarl · Pull Request #12255 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] laanwj pushed 5 commits to master: https://github.com/bitcoin/bitcoin/compare/fc21bb4e3590...3a573fd46c75
< bitcoin-git> bitcoin/master 1bd9ffd Chun Kuan Lee: windows: Set _WIN32_WINNT to 0x0601 (Windows 7)
< bitcoin-git> bitcoin/master d8a2992 Chun Kuan Lee: windows: Call SetProcessDEPPolicy directly
< bitcoin-git> bitcoin/master d0522ec Ben Woosley: Drop defunct Windows compat fixes
< bitcoin-git> [bitcoin] laanwj merged pull request #14922: windows: Set _WIN32_WINNT to 0x0601 (Windows 7) (master...patch-1) https://github.com/bitcoin/bitcoin/pull/14922
< promag> is #15153 for 0.18? if so it needs the label
< gribble> https://github.com/bitcoin/bitcoin/issues/15153 | gui: Add Open Wallet menu by promag · Pull Request #15153 · bitcoin/bitcoin · GitHub
< wumpus> sure, the 0.18 label isn't more than 'would be nice if it made 0.18'
< promag> just saw #15309 :/
< gribble> https://github.com/bitcoin/bitcoin/issues/15309 | tests: Running with --usecli ~5x slower than without · Issue #15309 · bitcoin/bitcoin · GitHub
< promag> I'll dig into that cc sdaftuar
< gleb> It seems like we restricted getdata on sending to 100 entries, and on receiving to 50,000 entries :)
< gleb> If we restrict getdata to 100 on receive, it seems we would punish btcd nodes for misbehaving :)
< gleb> If this is done on purpose, perhaps we should leave a comment at least...
< sdaftuar> you mean leave a comment as to why we picked 100?
< gleb> Why there are 2 different numbers where it is expected to have the same number (because we don't want to punish btcd?)
< sdaftuar> or punish old nodes
< sipa> well, it's always been 50000
< sipa> we can't just change what we accept
< sipa> we can change what we send
< sdaftuar> ^ right
< sipa> so indeed, the needed comment is why are we only sending 100 :)
< gmaxwell> we should probably limit rx too, that just takes coordination time.
< sdaftuar> limiting rx implies creating some rate limit (to be useful). not sure what we gain otherwise except coordination headache
< sdaftuar> gleb: i picked 100 by looking at the already_asked_for size relative to the number of peers i thought you might reasonably have, and picking a round number that would give us some buffer
< gmaxwell> sdaftuar: not a rate limit, but the number of invs in a single message.
< sdaftuar> gmaxwell: what purpose would that serve?
< gmaxwell> We should not have 1.8MB packets in the protocol, they insert huge latency/processing spikes.
< sdaftuar> ah, i guess that's fair
< sdaftuar> we could also make our code smarter and allow interruption of processing such messages
< gmaxwell> not the most critical of issues, but I can't see any reason why we'd ever want getdatas that large.
< sipa> sdaftuar: i think we do, for getdata
< sdaftuar> sipa: yep
< sdaftuar> but for tx announcements
< gmaxwell> sdaftuar: still have to buffer them.
< gmaxwell> an obvious thing to do would be to limit them with the introduction of the encrypted transport.
< sdaftuar> sure
< gleb> But huge getdatas are as bad as huge invs, right?
< sdaftuar> i think so
< sdaftuar> we queue them up for processing
< sdaftuar> in theory getdatas should be limited by what we announce, which is rate limited
< sdaftuar> a misbehaving peer could send us garbage
< sdaftuar> and we wouldn't punish (instead, we helpfully send notfound's)
< gmaxwell> I thought we stopped sending notfounds?
< sdaftuar> no, we stopped sending rejects i think
< sdaftuar> i have a PR coming that will use the NOTFOUNDs! so i hope we don't get rid of those
< gmaxwell> what would you use them for?
< sdaftuar> to request a transaction from someone else. my thought was that we can make it so that not providing a transaciton is no longer a DoS on our peer
< sdaftuar> and this would be a way to eventually drop mapRelay
< gmaxwell> that requires the peer to cooperate, a DOS attacker wouldn't...
< sdaftuar> gmaxwell: agree, but a DoS attacker has no bearing on whether we keep mapRelay around
< gmaxwell> So all that would do is speed things up in the cooperative case? which sounds okay to me, but I don't see how we could drop maprelay?
< sdaftuar> well i think then we can just serve things from the mempool
< sdaftuar> and give a notfound otherwise
< gmaxwell> oh maprealy sorry, being dumb and thinking you were talking about the map we use to schedule rerequests.
< gmaxwell> sounds good.
< sdaftuar> this should be easy to implement after gleb's PR, so i was hoping we might slip it into 0.18
< sdaftuar> (the smarter behavior after receiving a NOTFOUND, i mean)
< gmaxwell> I like this idea.
< phantomcircuit> gmaxwell, im not sure how much of a processing spike a 1.8MB inv would cause, since it gets pushed into it's own queue and each inv is processed individually
< phantomcircuit> (s/inv/getdata/)
< gmaxwell> not processing spike, as much as it head of line blocks the socket, uses up a big buffer just checksumming it, etc. It's not much of a problem but it serves no use.
< phantomcircuit> think the only issue there is the memory it uses
< phantomcircuit> blocking the socket for the peer that's being stupid seems fine to me
< gmaxwell> right, I said about it isn't so much a problem as its just dumb.
< gmaxwell> though it does mean that we need to be willing to buffer 1.8mb of memory, but we have to for other messages in any case.
< gmaxwell> (in particular, block messages)
< bitcoin-git> [bitcoin] MeshCollider pushed 4 commits to master: https://github.com/bitcoin/bitcoin/compare/3a573fd46c75...30e799a5f705
< bitcoin-git> bitcoin/master 8602a1e João Barbosa: wallet: Close dbenv error file db.log
< bitcoin-git> bitcoin/master 2f8b8f4 João Barbosa: wallet: Close wallet env lock file
< bitcoin-git> bitcoin/master d3bf3b9 João Barbosa: qa: Test .walletlock file is closed
< bitcoin-git> [bitcoin] MeshCollider merged pull request #15297: wallet: Releases dangling files on BerkeleyEnvironment::Close (master...2019-01-close-dbenv-files) https://github.com/bitcoin/bitcoin/pull/15297
< promag_> sdaftuar: found the problem
< sdaftuar> promag: great! what is the issue?
< promag> as usual (?) code is right
< promag> so the issue is in wait_for_rpc_connection
< promag> when using --usecli the TestNode still establishes a RPC connection
< promag> to see if the RPC interface is ready
< promag> however that connection is persistent by default
< sdaftuar> so the issue is that the python tests aren't properly closing the connection, when using --usecli?
< promag> in other words, if the daemon has 2 persistent connections, and 1 sends stop, the server (at the moment) won't force disconnect the other, it will timeout
< promag> yes
< sdaftuar> what's the mechanism for that to work when we're not using --usecli?
< sdaftuar> oh the daemon will initiate the close?
< promag> when not using --usecli the rpc connection will receive the header Connection: close, because the server is shutting down
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/30e799a5f705...9b63c436a699
< bitcoin-git> bitcoin/master 364cff1 Chris Moore: Fix issue #9683 "gui, wallet: random abort (segmentation fault) running ma...
< sdaftuar> ok i think i sort of understand
< bitcoin-git> bitcoin/master 9b63c43 MarcoFalke: Merge #15203: Fix issue #9683 "gui, wallet: random abort (segmentation fau...
< promag> but if the connection is idle then no header is sent
< sdaftuar> i did notice that my test in #15305 seems very slow -- the issue there is that i'm constructing a scenario where bitcoind AbortNode()'s
< gribble> https://github.com/bitcoin/bitcoin/issues/15305 | [validation] Crash if disconnecting a block fails by sdaftuar · Pull Request #15305 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15203: Fix issue #9683 "gui, wallet: random abort (segmentation fault) (master...fix-startup-crash) https://github.com/bitcoin/bitcoin/pull/15203
< sdaftuar> and the http server is slow to shut down
< promag> one way to fix this is to send the close header if --usecli
< promag> however I'm still curious of what should be the correct behavior of stop
< promag> stop command should force close everything?
< sdaftuar> yeah i don't know, i haven't given this much though
< sdaftuar> thought*
< promag> anyway I'll see if I can fix this with the above idea
< sdaftuar> ok thanks for working on this, hopefully others more knowledgeable about the design here can chime in with suggestions
< promag> well maybe wait_for_rpc_connection should also --usecli
< promag> jnewbery: ^
< promag> bbl with a pr, sorry for the inconvenient
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/9b63c436a699...9e7f8f6c8271
< bitcoin-git> bitcoin/master ef0b012 practicalswift: tests: Make updatecoins_simulation_test deterministic
< bitcoin-git> bitcoin/master 9e7f8f6 MarcoFalke: Merge #15327: tests: Make test updatecoins_simulation_test deterministic
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15327: tests: Make test updatecoins_simulation_test deterministic (master...SeedInsecureRand(true);) https://github.com/bitcoin/bitcoin/pull/15327
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/9e7f8f6c8271...baf125b31d0b
< bitcoin-git> bitcoin/master 4701239 Amiti Uttarwar: [Docs] Small updates to getrawtransaction description
< bitcoin-git> bitcoin/master baf125b MarcoFalke: Merge #15332: [Docs] Small updates to getrawtransaction description
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15332: [Docs] Small updates to getrawtransaction description (master...get_transaction_docs) https://github.com/bitcoin/bitcoin/pull/15332
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/baf125b31d0b...5029e94f855c
< bitcoin-git> bitcoin/master 58180b5 James O'Beirne: tests: add utility to easily profile node performance with perf
< bitcoin-git> bitcoin/master 13782b8 James O'Beirne: docs: add perf section to developer docs
< bitcoin-git> bitcoin/master 5029e94 MarcoFalke: Merge #14519: tests: add utility to easily profile node performance with p...
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #14519: tests: add utility to easily profile node performance with perf (master...2018-10-func-test-profiling) https://github.com/bitcoin/bitcoin/pull/14519
< gleb> It's confusing that we do not log "AcceptToMemoryPool" upon accepting orphan transactions (when receive a valid parent)
< jnewbery> promag: I'm not sure without digging into it. Feel free to @ me on the PR
< gleb> We do it for a parent, but not for an not-orphan-anymore child...
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #15349: travis: Only exit early if compilation took longer than 30 min (master...Mf1902-travis30) https://github.com/bitcoin/bitcoin/pull/15349
< gmaxwell> gleb: weird. patch accepted?
< gleb> gmaxwell: Sorry not sure what you're asking, are you suggesting me to fix it?
< gmaxwell> gleb: yes, I agree it's weird that we don't log. it also should be a trivial change to add a log entry for it.
< gmaxwell> So I'm suggesting you go add it. :)