< dcousens> petertodd: maybe change the PR title to `Full-RBF with nSequence opt-out`?
< dcousens> petertodd: it reads as though its an opt-out for the node (until you read the PR itself, obviously)
< dcousens> (but even still, I had to check the code to be sure)
< GitHub159> [bitcoin] jgarzik opened pull request #6873: [cleanup] leveldbwrapper becomes more generic 'dbwrapper' module (master...2015_dbwrapper) https://github.com/bitcoin/bitcoin/pull/6873
< gmaxwell> It would also be less confusing if it were called opt-in. (since transactions are currently opt out under the design)
< dcousens> gmaxwell: agreed
< dcousens> though it is called opt-in in the commit
< dcousens> So, maybe consistency there
< dcousens> jgarzik: in 6873
< dcousens> what actually makes CDBWrapper agnostic of leveldb?
< dcousens> It seems to still use all the statuscode types
< dcousens> And the wrapper can still be instantiated by itself
< dcousens> Or is this just a transitionary step?
< sipa> dcousens: it's API-agnostic... code using it doesn't need to know that it's backed by leveldb
< sipa> even though you can't compile it any other way
< jgarzik> nod. The interface is backend agnostic
< dcousens> jgarzik: then maybe the PR should be about removal of leveldb types in DbWrapper
< jgarzik> dcousens, did you look at the sqlite commit?
< dcousens> sqlite commit?
< dcousens> nvm
< dcousens> right
< dcousens> cheers
< dcousens> jgarzik: my bad for not reading the PR before reading the changes
< dcousens> Given its called a 'wrapper', but its now generic, seems a bit odd IMHO
< sipa> call it DbInterface then
< phantomcircuit> jgarzik, :)
< dcousens> sipa: but its not just an interface no, it instantiates and manages the entire thing right?
< dcousens> its more like 'Db', ha
< jgarzik> it is a class that wraps the backend database in a nice snuggly generic interface
< gmaxwell> (1) dear god, are people really arguing about this, (2) dbwrapper says what it does!
< jcorgan> jgarzik: compile ack on the 2015_sqlite branch, but looks like retains all the leveldb naming in the build system, checking now
< dcousens> gmaxwell: wasn't arguing, just seemed better to ask here than ask about it on GH seeing as its probably me just missing context
< jgarzik> jcorgan, naming? 2015_sqlite branch should not include any leveldb naming in the bitcoin->sqlite path. it _does_ build leveldb. it _does not_ link leveldb to bitcoind.
< jcorgan> oh, that's what i'm seeing then
< sipa> dcousens: then DbWrapper is a better name! it's something that wraps a db, but you don't know or care what kind of database that is
< dcousens> gmaxwell: in any case, a wrapper typically wraps an existing object no?
< dcousens> in this case, the object is entirely owned by the wrapper
< sipa> grrrrr, EncapsulatedDb then?
< dcousens> haha, sipa this isn't important, I'm just curious
< jcorgan> jgarzik: there are some stray leveldb name issues
< jgarzik> jcorgan, where?
< jcorgan> like using 'leveldb_error' in dbwrapper.cpp
< jgarzik> hrm, I thought I got that one :(
< jgarzik> jcorgan, no, wait, I did. git pull. :)
< jcorgan> it's still defined in dbwrapper.h, try commenting that and see where the errors
< dcousens> sipa: yeah, quick investigation of the wrapper/adapter pattern is that it is typically applied as a glue code, aka, between two things with their own ownership.
< dcousens> In this case, its not that, so, anyway. Just food for though
< dcousens> thought*
< dcousens> tl;dr EncapsulatedDb would probably be better
< jcorgan> i'm using b778481
< jgarzik> jcorgan, git pull
< CodeShark> HighlyVerboseButGenericNameForDbInterface
< jgarzik> dbwrapper is just fine, let's move on.
< sipa> yes, please
< jcorgan> jgarzik: ok, that fixed it
< dcousens> jgarzik: well, it causes confusion in anyone who hasn't read through that part of the source. So, IMHO it matters enough to be discussed. But not to block your PR. Moving on :)
< jcorgan> but to be pedantic, there are a bunch of 'leveldb' strings in the comments :-)
< * jgarzik> is focusing on operating stuff, like why this unit test doesn't want to compile my sql
< jgarzik> *operational
< jcorgan> no worries. i haven't tested anything, but compiles fine on ubuntu trusty
< jcorgan> and 6873 is fine
< GitHub80> [bitcoin] pstratem opened pull request #6874: Net: Cork socket send writes with MSG_MORE (master...msg_more) https://github.com/bitcoin/bitcoin/pull/6874
< phantomcircuit> jgarzik, it would be nice if there was a virtual base class
< phantomcircuit> DBWrapperInterface
< * phantomcircuit> goes to look for some InterfaceFactories
< dcousens> phantomcircuit: if the default is being hard-coded in at each entry anyway
< dcousens> probably not much point
< dcousens> unless the wrapper is instantiated at a much earlier point in the program, and passed through. At which point then the generic interface could be used
< dcousens> s/earlier/higher
< * jgarzik> reminds channel: we've moved on
< phantomcircuit> dcousens, well it's only actually named in one place i think so it doesn't much matter
< GitHub95> [bitcoin] TheBlueMatt opened pull request #6875: Fix pre-push-hook regexes (master...verify-commits-fixes) https://github.com/bitcoin/bitcoin/pull/6875
< GitHub11> [bitcoin] TheBlueMatt closed pull request #6875: Fix pre-push-hook regexes (master...verify-commits-fixes) https://github.com/bitcoin/bitcoin/pull/6875
< phantomcircuit> jgarzik, updated 6874
< phantomcircuit> <phantomcircuit> jgarzik, updated 6874
< phantomcircuit> possibly we should at least try to reconnect to disconnected peers...
< GitHub136> [bitcoin] laanwj opened pull request #6877: rpc: Add maxmempool and effective min fee to getmempoolinfo (master...2015_10_mempool_effective_fee) https://github.com/bitcoin/bitcoin/pull/6877
< GitHub27> [bitcoin] laanwj pushed 1 new commit to 0.11: https://github.com/bitcoin/bitcoin/commit/95a50390e1052a0a501eb446f87d63f58d95b7e7
< GitHub27> bitcoin/0.11 95a5039 Gregory Maxwell: Set TCP_NODELAY on P2P sockets....
< GitHub187> [bitcoin] laanwj pushed 1 new commit to 0.10: https://github.com/bitcoin/bitcoin/commit/5297194bbd6e0d6730515567248caf9c135e296c
< GitHub187> bitcoin/0.10 5297194 Gregory Maxwell: Set TCP_NODELAY on P2P sockets....
< GitHub44> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/b2b173acabea...fa1d252e82a8
< GitHub44> bitcoin/master 338f62f MarcoFalke: [devtools] add clang-format.py
< GitHub44> bitcoin/master 8c15f33 MarcoFalke: [trivial] Update contrib/devtools/README.md
< GitHub44> bitcoin/master fa1d252 Wladimir J. van der Laan: Merge pull request #6790...
< GitHub153> [bitcoin] laanwj closed pull request #6790: [devtools] add clang-format.py (master...MarcoFalke-2015-clangFormatWrapper) https://github.com/bitcoin/bitcoin/pull/6790
< GitHub149> [bitcoin] TheBlueMatt reopened pull request #6875: Fix pre-push-hook regexes (master...verify-commits-fixes) https://github.com/bitcoin/bitcoin/pull/6875
< GitHub150> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/fa1d252e82a8...923c5e93a90a
< GitHub150> bitcoin/master b48da5c David Hill: script: Remove magic numbers...
< GitHub150> bitcoin/master 923c5e9 Wladimir J. van der Laan: Merge pull request #6818...
< GitHub148> [bitcoin] laanwj opened pull request #6878: doc: Add developer notes about gitignore (master...2015_10_ignore_files) https://github.com/bitcoin/bitcoin/pull/6878
< GitHub128> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/923c5e93a90a...bf7c1958d199
< GitHub128> bitcoin/master 212bcca Tom Harding: Add optional locktime to createrawtransaction...
< GitHub128> bitcoin/master bf7c195 Wladimir J. van der Laan: Merge pull request #5936...
< GitHub70> [bitcoin] laanwj closed pull request #5936: [RPC] Add optional locktime to createrawtransaction (master...createraw_locktime) https://github.com/bitcoin/bitcoin/pull/5936
< GitHub151> [bitcoin] laanwj pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/bf7c1958d199...2a1090d4f55c
< GitHub151> bitcoin/master 287f54f Peter Todd: Add CHECKLOCKTIMEVERIFY (BIP65) soft-fork logic...
< GitHub151> bitcoin/master cde7ab2 Peter Todd: Add RPC tests for the CHECKLOCKTIMEVERIFY (BIP65) soft-fork...
< GitHub151> bitcoin/master 65ef372 Peter Todd: Add BIP65 to getblockchaininfo softforks list
< GitHub0> [bitcoin] laanwj closed pull request #6706: CLTV IsSuperMajority() soft-fork, rebased against v0.10.2 (0.10...cltv-soft-fork-v0.10) https://github.com/bitcoin/bitcoin/pull/6706
< * btcdrak> jumps for joy
< GitHub121> [bitcoin] laanwj opened pull request #6879: doc: mention BIP65 softfork in bips.md (master...2015_10_bip65) https://github.com/bitcoin/bitcoin/pull/6879
< GitHub155> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/2a1090d4f55c...c719cefc417c
< GitHub155> bitcoin/master d3b09f6 Alex Morcos: Do not allow blockfile pruning during reindex....
< GitHub155> bitcoin/master c719cef Wladimir J. van der Laan: Merge pull request #6856...
< GitHub81> [bitcoin] laanwj closed pull request #6856: Do not allow block file pruning during reindex. (master...noPruneDuringReindex) https://github.com/bitcoin/bitcoin/pull/6856
< GitHub177> [bitcoin] laanwj pushed 1 new commit to 0.11: https://github.com/bitcoin/bitcoin/commit/dfe55bdc32b5333dcce1a7f2c74628f64028d1fe
< GitHub177> bitcoin/0.11 dfe55bd Alex Morcos: Do not allow blockfile pruning during reindex....
< GitHub89> [bitcoin] jgarzik pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/c719cefc417c...46f74379b86b
< GitHub89> bitcoin/master 6ec4b7e Jeff Garzik: leveldbwrapper: Remove unused .Prev(), .SeekToLast() methods...
< GitHub89> bitcoin/master 8587b23 Jeff Garzik: leveldbwrapper symbol rename: Remove "Level" from class, etc. names
< GitHub89> bitcoin/master 3795e81 Jeff Garzik: leveldbwrapper file rename to dbwrapper.*
< GitHub179> [bitcoin] jgarzik closed pull request #6873: [cleanup] leveldbwrapper becomes more generic 'dbwrapper' module (master...2015_dbwrapper) https://github.com/bitcoin/bitcoin/pull/6873
< GitHub199> [bitcoin] gavinandresen opened pull request #6880: New -logtimerelative option to do millisec debug.log timestamps (master...millisec_debuglog) https://github.com/bitcoin/bitcoin/pull/6880
< GitHub54> [bitcoin] gavinandresen closed pull request #6880: New -logtimerelative option to do millisec debug.log timestamps (master...millisec_debuglog) https://github.com/bitcoin/bitcoin/pull/6880
< GitHub0> [bitcoin] sdaftuar opened pull request #6881: Debug: Add option for microsecond precision in debug.log (master...add-microsecond-timestamps) https://github.com/bitcoin/bitcoin/pull/6881
< maaku> I am leaving bitcoin core development for a while, not sure how long. If someone wants to take over pushing for #6312, #6564, and #6566 please contact me.
< Dyanisus> why you leaving?
< btcdrak> maaku: why are you leaing?
< btcdrak> maaku: we're literally right at the end!
< Dyanisus> maaku: did you get some sort of subpoena or something?
< btcdrak> maaku: I've sent you an email about those PRs.
< GitHub65> [bitcoin] petertodd opened pull request #6883: Add BIP65 CHECKLOCKTIMEVERIFY to release notes (master...cltv-release-notes-v0.12.0) https://github.com/bitcoin/bitcoin/pull/6883
< GitHub95> [bitcoin] btcdrak opened pull request #6884: Backport #6566, median-past locktime, rebased against 0.11 (0.11...mpl-0.11) https://github.com/bitcoin/bitcoin/pull/6884
< dhill> maaku: hi
< dhill> maaku: regarding checksequenceverify, i ran your tests against btcd, and they all pass
< dhill> but
< dhill> + if (txToSequence & CTxIn::SEQUENCE_LOCKTIME_DISABLED_FLAG)
< dhill> + return false;
< dhill> was not hit
< dhill> i think there is a dup test in tx_invalid.json
< maaku> I added btcdrak as a collaborator for my personal repo, so he should be able to update the 'sequencenumbers' 'checksequenceverify' and 'medianpastlocktime' branches
< btcdrak> maaku: thanks!
< cfields> sipa: ping. I'm trying to understand the current dnsseed thread behavior. It looks like in the non-proxied case, we resolve all seed nodes and add them to addrman without testing them. In the proxied case, we connect to each resolved entry as a oneshot...
< cfields> i assume the goal is to query them all once if possible in order to never have to touch 'em again?
< cfields> just seems a bit unbalanced between the proxy/non-proxy cases, want to be sure i'm not missing anything in the big picture
< * jgarzik> wonders about the proxy case behavior
< jgarzik> cfields, The add-untested logic follows the getaddr logic, where it is unknown possibly garbage data coming in from unknown sources
< jgarzik> cfields, overall I do think addrman must do a much better job of testing and expiring bad addresses
< jgarzik> cfields, notably we engage in the poor network behaviors of (1) propagating known bad addresses and (2) repeatedly trying the same dead nodes, in some near-empty-bucket edge cases
< gmaxwell> The proxy behavior is simply because we cannot do the dns lookup via the proxy. So connecting to the addresses returned (by proxy connecting to the names) was the best thing we could do.
< cfields> jgarzik: ok, that lines up with how i read it. Looks like in the proxied case where we can't run a query, we just connect to all reachable nodes (query done through the proxy), getaddr on them, and disconnect. seems to me like that'd yield less garbage, but i suppose you're right about letting addrman do it.
< gmaxwell> It does a less good job of initilizing the state for sure.
< gmaxwell> not to all reachable nodes, we connect 'to' each dnsseed by name (meaning connect to one of the results of it).
< cfields> gmaxwell: less good because the dns seeds tend to be much fresher, i assume?
< jgarzik> dns seeds are fresh and quick and easy and wonderfully centralizing ;p
< gmaxwell> cfields: no, just less good because it any one of the dnsseed results could be broken or malicious.
< cfields> gmaxwell: ah right, only one-per-seed that way.
< jgarzik> notably there is now code that "waits a bit, before checking seeds" to attempt to P2P without DNS seeds
< gmaxwell> e.g. dns seed returns nodes a,b,c,d,e,f,g ... and we only end up connecting to g (out of that dns seed's results) and perhaps g gives us useless answers.
< jgarzik> so DNS seeding is intentionally not immediate
< cfields> ok, all clear now. thanks.
< cfields> ok, along those lines, are there concerns (fingerprinting) about hitting the seeds too quickly or uniformly? I don't see any active approach in the current code to avoid that
< jgarzik> cfields, sure those concerns exist
< jgarzik> cfields, that's part of the reason why the current code tries to avoid DNS lookups
< jgarzik> cfields, a DNS seed can build a picture of bitcoin users etc.
< jgarzik> cfields, Current logic is: (1) if addrman.size() > 0, Attempt to connect to network given current peer database (2) If "too few" P2P connections after 11 seconds, perform DNS lookup.
< cfields> obviously not worried about the seeds themselves, since they're already trusted. more about someone trying to get 2+ entries in a seed's current entries in order to distinguish individuals
< gmaxwell> cfields: DNS seeds are prohibited by policy from having a TTL less than 60 seconds, so recursive resolution provides some increase in privacy.
< jgarzik> I think it's quite likely that a single DNS seed might spit out two addresses which are in fact controlled by one person/org
< gmaxwell> it's almost certan. well plus a substantial number of nodes on the network are sybil attackers in any case.
< gmaxwell> :P
< cfields> jgarzik: sure, i'm speaking of the procedure when we do fall into the dns lookup logic.
< jgarzik> sure. the logic is dumb: query all DNS seeds, let addrman sort it out
< gmaxwell> cfields: addrman knows where addresses came from to e.g. prevent any one source from dominating the table. There is only so much it can do.
< cfields> ok. no problem with that, just want to be sure i'm understanding the logic that's there
< gmaxwell> Lack of actual users running nodes provides a very limiting upper bound on how much protection is possible.
< cfields> got it. thanks jgarzik/gmaxwell.