< Luke-Jr> re sigop-limit flooding, can we just merge acceptnonstdtxn (mainnet option, off by default) finally? this would not be a problem if people were using code with it merged..
< gmaxwell> Luke-Jr: explain your logic?
< Luke-Jr> gmaxwell: the top commit limits (in policy) tx sigop count based on its size
< Luke-Jr> so the spam would need to be larger for it to use 15 sigops
< Luke-Jr> thus hitting the block size limit approximately at the same time as the sigop limit
< gmaxwell> I ~think~ the suggestion I made of max(size, sigops*1e6/20000) is more general than the limit thing.
< Luke-Jr> (and triggering higher fees, of course)
< Luke-Jr> gmaxwell: yes, my point is that the bug wouldn't exist in the first place had this already been merged
< BlueMatt> morcos: you asked about tx cache? https://github.com/TheBlueMatt/bitcoin/tree/limitucache
< BlueMatt> actually, I'll just pr
< GitHub110> [bitcoin] TheBlueMatt opened pull request #6868: Limit txn cache growth (master...limitucache) https://github.com/bitcoin/bitcoin/pull/6868
< gmaxwell> BlueMatt: for your don't fork from here-- I suggest https://github.com/gmaxwell/secp256k1
< BlueMatt> gmaxwell: yes, but that takes effort....
< gmaxwell> well if someone does it to the repo first you can check out theirs and force push it into yours. :P
< gmaxwell> I guess I should do bitcoin core too.
< GitHub26> [bitcoin] TheBlueMatt closed pull request #6868: Limit txn cache growth (master...limitucache) https://github.com/bitcoin/bitcoin/pull/6868
< morcos> BlueMatt: interesting idea.. i'm not sure what i think of it yet... you don't thinks also potentially inefficient
< morcos> did you see the conversation wumpus and I were having earlier
< morcos> one thing we could do in ATMP, is not pull into the pcoinstip cache coins for txs which aren't actually accepted into the memory pool
< sipa> yeah, we should do that
< sipa> i see sometimes huge increases in utxo cache size after a transaction
< sipa> however, with bith limited utxo cache and mempool, memory usage actually remains *very* low
< sipa> have a node with 100 MB utxo and 200 MB mempool, and total memory is staying below 570 M
< morcos> just don't run GetBlockTemplate! :)
< morcos> sipa: why does CreateNewBlock need to hold cs_main the whole time
< sipa> no idea
< sipa> also no idea why it actually needs to verify the result
< morcos> it seems like after the first round of calling HaveCoins on all the txins of all the txs.. you now have a view with all of of your coins in it
< morcos> and you could release cs_main
< sipa> the mempool could change then, though?
< morcos> i took a first stab at a background miner, but realized its useless if its holding cs_main
< morcos> well... what do you mean change? i think we don't want to lose txs that we might be trying to mine
< morcos> but maybe you can handle that by not removing things
< morcos> a smaller lock that just guards actually removing txs
< morcos> why would it not need to verify the result, you mean because it shouldn't have been able to create something invalid
< sipa> yes
< morcos> i do think thats a nice double check
< sipa> i don't think it's worth the cost
< sipa> given that it hasn't failed in years
< morcos> well it will be if we're chaning the mining algorithm!
< morcos> changing
< sipa> maybe a spot check would be useful
< sipa> run it once every 1000 calls
< jgarzik> since it's in the background the cost is mitigated
< morcos> holds cs_main
< sipa> block validation will not be background
< sipa> it needs the utxo set
< sipa> though i guess you are right that of it actually does make a full copy of all inlits, it can release the lock afterwards
< jgarzik> yes it needs utxo set but doesn't need cs_main long term
< sipa> well it needs the utxo lock, whatever that is
< morcos> yeah i think we could be smart about it
< sipa> i think it shouldn't need the utxo set or copies at all
< sipa> the mempool should be consistent
< sipa> do a spot check occasionally to make sure the code isn't broken
< morcos> right now it already pulls the potentially used utxos into caches TWICE. so you could use one for creation and the other for validation without needing to grab cs_main again
< sipa> yup
< sipa> it should be 0, imho
< morcos> oh
< morcos> thats what you mean by mempool should be consistent
< sipa> the mempool is known to not double spend
< morcos> yikes!
< sipa> not contain invalid transactions
< morcos> but thats also been broken
< sipa> so we should fix it
< morcos> but what i'm saying is that breaks often enough that you wouldn't want it to cause you to mine an invalid block?
< morcos> at least one check should still exist
< sipa> that's like saying "we're not sure about the utxo cache, let's run checkblockchain on every update"
< sipa> sure the check should exist
< sipa> but it should not be slowing down GBT by seconds!
< morcos> ok ok
< sipa> every single call
< morcos> its just scary when you have a giant block of code protected by cs_main.. trying to reason about what in there really cares about it
< sipa> well you don't want the best block or the mempool to change underneath it
< sipa> or you will be pulling in transactions that may conflict with each other
< Luke-Jr> sipa: GBT verifies the result because it's worth it. No miner wants to find out after the fact they mined an invalid block.
< morcos> if the best block changes i've got other problems.. i don't want to be calculating this block anyway
< sipa> Luke-Jr: the code should not produce invalid blocks, period
< Luke-Jr> also, spot checking just leads to people /expecting/ a lower time than they can rely on
< sipa> Luke-Jr: if you run the check once every 1000 calls you will equally well detect broken code
< Luke-Jr> not equally well, no.
< sipa> if the code is broken, it will fail badly
< Luke-Jr> and it will mislead people into thinking it's faster than it's necessarily
< Luke-Jr> an edge case will not necessarily fail badly.
< sipa> so run with -checkmempool on then
< Luke-Jr> without the verification, there is no consensus code involved in producing blocks here..
< Luke-Jr> valid transactions != valid block
< sipa> then you will also not get an invalid memlool state
< sipa> just get that check out of the mining path
< sipa> i wonder how much gbt delay has encourage validationless mining...
< Luke-Jr> removing the check *is* validationless.
< morcos> I'm with Luke-Jr that I think consensus code should happen at least once in the path
< sipa> it is not if the code is not broken
< morcos> but it doesn't have to happen twice as it does now
< Luke-Jr> morcos: wait, twice?
< sipa> once for building, once for checking
< morcos> CheckInputs on the transactions and TestBlockValidity
< Luke-Jr> CheckInputs doesn't check the entire block, but maybe it can be removed
< sipa> it does check the whole block eventually, as it's called for all transactions that end up in it
< Luke-Jr> sipa: no, because all the transactions can be valid without the block itself being valid
< morcos> but i think we're talking about different things here
< sipa> Luke-Jr: i know that
< morcos> i'm concerned with having the block-template thread hold cs_main too much...
< morcos> CheckInputs doesn't need cs_main once you have a view
< sipa> but checkinputs is called for the equivalent amount of work of validating the whole block
< morcos> sipa your concern seems to be latency in GBT
< morcos> but thats solvable in other ways
< sipa> yes
< sipa> ok
< morcos> as long as you think its ok to have brief periods of mining on an empty block or something
< Luke-Jr> GBT isn't supposed to be time-critical anyway (but I already plan to improve it)
< sipa> well it can't take seconds...
< Luke-Jr> maybe ideally it wouldn't, but it shouldn't hurt much
< morcos> forget about how long it takes, what you care about is how long after a new block comes in that you have a header you can mine that builds off it right?
< Luke-Jr> also afaik right now it only takes seconds when the miner is neglecting to keep a clean mempool
< morcos> it doesn't matter if it then takes seconds to generate a new one with txs right?
< Luke-Jr> since it needs to go over every tx in the mempool
< Luke-Jr> not because of the test afterward
< sipa> morcos: as long as fees are negligablez yes
< morcos> although seconds is really absurdly slow and we should be able to get it under that even for that case
< morcos> Luke-Jr: yeah thats one of the problems, is that now it loops all the txs twice
< morcos> once putting them all in vecPriority
< morcos> and then trying to go through vecPriority and put them all in a block
< Luke-Jr> yeah
< morcos> with a sorted mempool
< morcos> both become unnecessary (the 2nd was already unnecessary)
< Luke-Jr> sorted mempool is essentially what I'm working on
< Luke-Jr> although I need to rebase on top of the packages stuff I expect
< morcos> sdaftuar is very close to finishing the ancestor package tracking
< morcos> if you want to build a more general sorting priority score, thats probably what you should build off of
< Luke-Jr> not priority score
< Luke-Jr> sorting based on the user-defined policy ;)
< morcos> yeah thats what i meant
< morcos> don't you have to somehow turn their policy into a score?
< Luke-Jr> no
< morcos> how can you sort without a score?
< Luke-Jr> the policy is implemented mostly by a method that compares two transactions
< morcos> and if A < B and B < C then A < C right?
< Luke-Jr> right
< Luke-Jr> or at least, that's assumed by the mempool
< Luke-Jr> it might be fun to some day make blocks with a policy that just returns rand() :P
< morcos> ok well the point i was going to make was that if you could look at it as a score
< morcos> and A has child A2 and B has child B2
< sipa> you're defining a total ordering, that is equivalent to a score function
< Luke-Jr> yes, you could look at it that way; my point was that actually using a score is overcomplex
< morcos> then if you use suhas's ancestor package tracking with your score instead of fee you'll get CPFP in terms of your policy
< morcos> sipa: what are your thoughts on maintaining ancestor package tracking on nodes that aren't mining
< morcos> its not totally useless and it hopefully shouldn't be too expensive. but certainly adds more mememory/cpu for something they are basically not using
< morcos> the one thing i think it could be used for is fee estimation? but thats only a maybe, i haven't really figured out how or if that would be worth it. but estimating what miners are going to do seems like potentially a valuable tool for fee estimation.
< gmaxwell> cfields_: good catch, apparently it's not inhereted reliably and there is a bunch of broken software out there.
< gmaxwell> (by reliably I mean it's system dependant)
< phantomcircuit> can confirm bitcoin core requires reindex on power failure under windows
< phantomcircuit> probably leveldb env driver is stupid
< gmaxwell> phantomcircuit: fixy fixy? unfortunately I think it's unmaintained.
< gmaxwell> as chrome uses some chrome specific abstractions for IO. :(
< phantomcircuit> gmaxwell, i can take a look at it, probably it should just be replaced with the current posix version which is just fopen/fread/fwrite basically
< GitHub16> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/0fbfc5106cd9...a09297010e17
< GitHub16> bitcoin/master 579b863 Wladimir J. van der Laan: devtools: Add security-check.py...
< GitHub16> bitcoin/master a092970 Wladimir J. van der Laan: Merge pull request #6854...
< GitHub129> [bitcoin] laanwj closed pull request #6854: devtools: Add security-check.py (master...2015_10_security_checks) https://github.com/bitcoin/bitcoin/pull/6854
< wumpus> phantomcircuit: yeah https://github.com/bitcoin/bitcoin/issues/5610
< wumpus> no clue on solving it though
< wumpus> for some reason most people with a clue about windows internals seem to be blackhats, and they like things broken :p
< phantomcircuit> wumpus, it's using windows memory mapping
< gmaxwell> phantomcircuit: reports went up astronomically after 0.10 and some users reported it worked fine for them before them and fails every time since.
< phantomcircuit> im going to assume it's that
< gmaxwell> phantomcircuit: oh it's writing via mmap too?
< phantomcircuit> i just checked and there's a Win32Map something or other
< phantomcircuit> i assume it's using that
< wumpus> it has been a while since we updated leveldb, maybe some things got fixed since?
< wumpus> heh
< phantomcircuit> gmaxwell, that's about right
< phantomcircuit> it's windows... how do you even begin to debug this
< wumpus> that's a dark art
< wumpus> wouldn't be surprised if it requieres at least some, possibly expensive, proprietary software applications
< wumpus> debugging-by-mutating-the-code may be the most promising approach, extract a bare bones program that uses leveldb and crashes, then change it (adding flushes everywhere) until it manages to not corrupt anymore
< gmaxwell> it's 100% reproducable it seems, which helps.
< wumpus> this is the approach that works for fixing problems with underdocumented hardware, it should work for windows too...
< gmaxwell> the fact that it went way up when we turned up checking suggests to me that it's something like a truncated record at the end that was otherwise silently recoverable.
< gmaxwell> e.g. something like the osx bug.
< wumpus> yes good point - can you send me a database that is corrupted by this problem?
< gmaxwell> phantomcircuit: you would be you, as I don't have a repro.
< phantomcircuit> heh
< phantomcircuit> gmaxwell, i suspect you're right that it's similar to the OSX bug
< phantomcircuit> i also suspect it would be easier to replace leveldb with lmdb
< wumpus> phantomcircuit: can you send me a database that is corrupted by this problem?
< gmaxwell> mmap only, goodbye 32 bit hosts.
< gmaxwell> Among other limitations. :(
< Arnavion> It's funny because if you ask Windows devs they'll tell you the dev tooling on Windows is far better than on Linux
< phantomcircuit> wumpus, i mindlessly clicked the reindex button
< gmaxwell> Arnavion: there is some aspect of "tools you know are always better than ones you don't"
< phantomcircuit> i can certainly try to do so though
< wumpus> Arnavion: it may be more user friendly on windows, but it's harder to get insight about what is happening on a system
< Arnavion> There's also the aspect of GUI tools that don't suck
< Arnavion> but I'm biased
< gmaxwell> Arnavion: you mean GUI tools inherently sucking? :P
< Arnavion> Oh, but that's what WinDbg is for
< Arnavion> You can do everything with a keyboard if you wish
< wumpus> anyhow this is not constructive
< wumpus> if you feel helpful, please demonstrate your skills by solving the leveldb corruption issue Arnavion :)
< Arnavion> I have no skills
< Arnavion> but I can take a look
< wumpus> demonstrate the great capability of windows debugging tools, then
< wumpus> phantomcircuit: but you can reproduce it! just start a new datadir, crash the thing, and send it to me
< wumpus> lmdb is not the panacea some people think - but if you feel like patching bitcoind to use it, that'd be pretty neat as we could run some benchmarks and comparisons
< phantomcircuit> wumpus, the main thing that's missing from leveldb is... sequence numbers in the journal
< phantomcircuit> if you miss a full record of writes it doesn't know
< wumpus> still, we need to debug this issue, as it is clearly windows specific, so not fundamental to leveldb
< wumpus> if this was broken on all platforms I'd agree that looking for a replacement would be advisable...
< phantomcircuit> wumpus, it *is* broken on all platforms
< phantomcircuit> the failure rate is simply much lower
< gmaxwell> one step at a time.
< gmaxwell> lets making the bleeding stop on windows first.
< wumpus> that's a nihilistic way of looking at it phantomcircuit, I like that, yes, it's all broken, some things with a very low failure rate... but it doesn't help solving the issue :)
< wumpus> I'm sure lmdb is also broken in some subtle ways
< wumpus> anyhow, if you're not going to send me a corrupted datbase I'm going to start working on something else
< gmaxwell> I assume he has to wait for the reindex to complete to reproduce. :P
< wumpus> wha?
< wumpus> I assumed it would also happen when crashing during reindex?
< Luke-Jr> wumpus: what is Windows specfic?
< Luke-Jr> [08:13:52] <phantomcircuit> can confirm bitcoin core requires reindex on power failure under windows <-- I can confirm it requires reindex on power failure under Linux..
< wumpus> in that case, sorry, no hurry implied
< Luke-Jr> wumpus: if you in fact want such a corrupt bitcoin dir, I can probably get one for oyu
< * Luke-Jr> ponders where he put his USB Armory
< Luke-Jr> oh, it's plugged in
< wumpus> Luke-Jr: great!
< phantomcircuit> Luke-Jr, ive never had that issue on linux
< gmaxwell> it's interesting that the usb armory corrupts, perhaps not the same issue as windows though (though perhaps also worth fixing)
< wumpus> indeed, may be a different issue
< Luke-Jr> FWIW, removing power immediately after IBD started did NOT reproduce it
< Luke-Jr> anything in debug.log to look for to know when it does the first flush to disk?
< wumpus> set a low dbcache to force lots of flushes?
< wumpus> AFAIK it doesn't flush unless necessary until the initial block download is complete
< Luke-Jr> 'often' is too slow
< Luke-Jr> 2015-10-22 09:27:59 UpdateTip: new best=000000003f7e074587fa1684ac863519fea3c64040b05ddd04948a13f7b19b42 height=415 log2_work=40.700462 tx=423 date=2009-01-14 05:56:05 progress=0.000002 cache=419
< Arnavion> From just lazy-mode browsing the code, Win32MapFile::Sync() flushes but Win32MapFile::Flush() is a no-op, whereas the equivalent functions for posix both do things
< Arnavion> I did not see how they are called from common code to see if that matters
< wumpus> it all depends on whether this map file is used for writing
< wumpus> AFAIK on other OSes, writing uses normal file system commands, whereas reading can use mmap
< wumpus> if t his is different on windows that would explain something
< Arnavion> Windows has a dedicated FlushViewOfFile function to flush a mapped region
< Arnavion> That is called by Sync(), but not by Flush()
< Luke-Jr> hmm
< Luke-Jr> it seems I can't reproduce during IBD or something
< Luke-Jr> and a fully synced state is huge :/
< midnightmagic> Luke-Jr: you're reproducing on a Windows OS?
< Luke-Jr> midnightmagic: no
< * Luke-Jr> ponders
< Luke-Jr> wumpus: well, the last chainstate I had before today is 922 MB and demanded a reindex.. not sure if that's good enough, because I can't upload 44 GB :/
< wumpus> that's kind of huge - do you perhaps know what ldb file the corruption is in?
< Luke-Jr> no, I have no idea how leveldb works :/
< Luke-Jr> I could probably just give you access to the device?
< Luke-Jr> wumpus: see PM for login info
< wumpus> ok
< Luke-Jr> setup auth for your github ssh key; let me know if there's a better one
< Luke-Jr> wumpus: the leveldb in question is under ~/.bitcoin.bak
< Luke-Jr> perhaps relevant: tar: /home/usbarmory/.bitcoin.bak/chainstate/1463292.ldb: File shrank by 2020259 bytes; padding with zeros
< wumpus> uh...
< wumpus> yes, that doesn't sound good. Were you tarring while bitcoin running?
< Luke-Jr> not sure if bitcoind is running, but if it is, it's not running on *this* database
< * Luke-Jr> wonders what it's doing so much to hang SSH
< wumpus> ok, it would not be strange if it is happening while running tar on the datadir that bitcoin was running in, those files are deleted and recreated all the time
< Luke-Jr> right
< sipa> wumpus: leveldb seems unmaintained
< wumpus> bleh
< wumpus> I'm happy we at least never decided to use it for the wallet
< sipa> maybe we should switch to sqlite... at least that's very well tested and maintained
< wumpus> it is, but it's not very fast for key/value storage, and I don't think it handles such large databases very well
< btcdrak> sipa: sqlite is very slow
< wumpus> if you need more advanced query features it is very nice though
< btcdrak> it would be so much easier and flexible if we were using SQL of some kind...
< wumpus> what would be easier?
< sipa> btcdrak: leveldb's own benchmark says that sqlite is faster for random reads
< btcdrak> sipa: interesting
< wumpus> for wallet metadata sql would be reasonably useful
< sipa> writes are much slower, but we do very few writes
< btcdrak> wumpus: well you get a lot of stuff for free with a SQL database stack.
< sipa> such as?
< Luke-Jr> # dmesg
< Luke-Jr> Segmentation fault
< Luke-Jr> -.-
< wumpus> during initial sync we do quite a lot of writes (every coin that is touched is written back), after that, most are reads
< btcdrak> sipa: indexes for a start. the ability to query the dataset externally from bitcoind etc
< wumpus> using database indexes would mean using a very verbose format
< sipa> btcdrak: we're not storing any data in a way that's useful to index
< sipa> we'd lose massive performance by expanding the data
< wumpus> that kind of defeats the purpose - bitcoind's database should optimized for its own purposes as running a node, not external ones
< btcdrak> I had wondered about leveldb's ongoing support. it really looks like an orphaned project at this time.
< wumpus> sipa: right
< jgarzik> btcdrak, the expanded dataset in SQL is something like 40GB at least
< jgarzik> not including the block data itself
< jgarzik> sipa, worse comes to worse, we can do our own key/value store...
< wumpus> also bitcoin's databases are not an external interface, don't query them directly unless it's to write troubleshooting/recovery tools
< jgarzik> there are actually a few optimizations you can make if you assume your keys are hashes already
< jgarzik> not that seeks matter these days, but some seeks can be eliminated
< wumpus> reinventing the wheel should be the last recourse
< Luke-Jr> wumpus: whatever just happened appears to have fried the device, so I guess it'll be a while :/
< wumpus> Luke-Jr: I hope it wasn't me logging in! :p
< Luke-Jr> heh, I don't see how that could do it
< sipa> leveldb does have a benchmark about batch write performance
< sipa> where sqlite is much slower
< wumpus> I don't see either, I didn't even get a prompt
< wumpus> I've disconnected now
< sipa> though sqlite does win in synchronous write speed, which we also do frequently
< Luke-Jr> wumpus: makes me wonder if it's just a bad microSD card, which could make the failures invalid
< wumpus> would be interesting to patch sqlite into bitcoind and compare and do benchmarks
< sipa> the only reason i'm suggesting sqlite is because it is *very* well tested afaik
< * wumpus> says that for the second time today about a different database
< wumpus> sipa: also on windows?
< wumpus> leveldb is also very well tested, it is used by many companies in production... just mostly on linux/unix servers
< sipa> yes
< wumpus> do you know, how does sqlite do at caching?
< btcdrak> sipa: wumpus: maintenance is probably more of a concern than anything else. I do remember wondering about leveldb's longevity. sqlite is way too popular to disappear.
< jgarzik> sqlite leans a lot of OS caching, temporary files etc.
< wumpus> jgarzik: it can't do a much worse job than leveldb at that at least
< btcdrak> if we're just doing key/value what about all those database that do that sort of thing, like those nosql stacks?
< jgarzik> I'm a big fan of sqlite, and used it in my DNS server project. I don't know that sqlite has batch update - does begin/commit suffice given our workflow? They are -mostly- equivalent but not 100% equiv
< btcdrak> still. sqlite would be a better choice. But here's the thing, if we use sqlite then you're opening up the door for just about any database backend. specially if we made an abstraction layer
< wumpus> "The maximum size of a database file is 2147483646 pages. At the maximum page size of 65536 bytes, this translates into a maximum database size of approximately 1.4e+14 bytes " ok, apparently I misremembered
< wumpus> btcdrak: there aren't that much embeddable key/value stores
< btcdrak> wumpus: true.
< btcdrak> and sqlite wins in terms of support/maintenance.
< jgarzik> I certainly wouldn't suggest expanding the bitcoind db in SQL, but using it as a dumb blob datastore shouldn't be a big issue
< wumpus> and most of them are likely one-off projects, even worsely maintained than leveldb
< wumpus> right
< wumpus> jgarzik: +1
< btcdrak> +!
< btcdrak> +1
< Luke-Jr> reminder: UTXO db is consensus-critical.
< sipa> very aware
< wumpus> sqlite is extremely easy to embed in a project, it's just one C file
< jgarzik> kv datastores off the top of my head: leveldb, gdbm, [n]dbm, tokyo cabinet, kyoto cabinet, berkeley db
< jgarzik> wumpus, er huh?
< Luke-Jr> wumpus: surely one *big* C file. all of SQL can't be trivial.
< jgarzik> wumpus, sqlite is quite bigger than one C file
< wumpus> hm it used to be at least
< jgarzik> berkeley db is well maintained :) :)
< Luke-Jr> sqlite is like 140kLOC
< jgarzik> for sqlite, if you pre-compile the SQL queries, it goes pretty fast
< sipa> of course we'd precompile them...
< sipa> i din't think git subtreeing sqlite is reasonable
< wumpus> oh this is why I thought it was one C file, yes it's a very large one with everything pasted together: https://www.sqlite.org/amalgamation.html
< wumpus> sipa: license-wise?
< sipa> size wise
< jgarzik> sqlite is public domain, one of the few
< sipa> oh, that amalgation is pretty awesome
< sipa> we can compile it with various flags for extensions disabled
< wumpus> apparantly leveldb is 26kLOC in total
< wumpus> sipa: yep
< jgarzik> imo LOC and size are secondary factors
< jgarzik> primary is on going maintenance, reliability, ...
< wumpus> LOC would be important if there is the chance we end up having to maintain/troubleshoot it ourself
< wumpus> (like now for leveldb)
< jgarzik> indeed
< jgarzik> I think pgdb will likely be 10k loc when finished
< wumpus> "SQlite as a key-value database" https://www.sqlite.org/cvstrac/wiki?p=KeyValueDatabase
< wumpus> looks very easy to do, the only question is indeed jgarzik's whether the TRANSACTION/COMMIT is equivalent to batching as we use it now
< jgarzik> another sql caveat - based on experience - you have to be careful that your 'strings' are not translated in any way by the engine via unicode etc.
< Luke-Jr> jgarzik: nobody is going to maintain consensus-critical behaviour
< wumpus> sql injections? *ducks*
< jgarzik> I used sqlite to store binary DNS records
< sipa> sqlite is fully transactional
< Luke-Jr> no matter what we use, we will need to maintain it
< Luke-Jr> even if that just means reviewing each upstream release
< sipa> or just reviewing their test practices
< jgarzik> sqlite is very well hammered
< wumpus> (no, sqlite has proper parametrized queries, and from my experience its BLOBs are binary clean)
< jgarzik> few maintain software to consensus critical standards though
< sipa> agree, but a database interface clearly should... it's perfectly specified in both dirextion: it should find every record that exist, and not find any reorc that doesn't exist
< jgarzik> another bit - make sure there are no limitations on rows-modified-at-once
< sipa> so anything that is nkt consensus compatible is a bug
< jgarzik> otherwise we re-introduce the BDB fork issue (locks)
< Luke-Jr> sipa: fixing bugs is a consensus compatibility bug!
< sipa> jgarzik: the "bug" in the bdb case was that we didn't chefk tue bdb return value, and regarded failure to write as a block validation failure
< sipa> Luke-Jr: fully agree there
< Luke-Jr> anyhow, I need to go to sleep, but I think we will regret it if we import sqlite to consensus-critical code.
< jgarzik> sipa, well there was an issue where we were hitting BDB max lock
< jgarzik> sipa, the point still stands - sql engines do somethings have max-rows-updated-in-one-transaction type limits
< sipa> jgarzik: sure, but that wouldn't have been a consensus failure threat if we didn't treat out-of-locks as invalid-block
< jgarzik> *sometimes
< sipa> jgarzik: it was us who turned an administrative restriction into a consensus problem
< jgarzik> so, sql transaction limits is another one for the eval list
< sipa> if we would just have gone "oops! can't write! quitting", like we do for out of disk, bdb would not have caused forks
< wumpus> right, if it would treat database errors as a fatal error instead of rejection, it'd wouldn't have been as bad
< jgarzik> nod
< sipa> it would have been a bad DoS attack, though
< jgarzik> yep
< sipa> but not a fork
< wumpus> but fairly easy to resolve
< wumpus> right
< sipa> of course, you are absolutely right we need to be aware of such limits in sqlite or whatever we're incestigating
< sipa> eh, investigating
< jgarzik> lol
< sipa> the keys are like right nezt to each other
< sipa> also, sqlite databases are single files, right?
< jgarzik> yes*
< jgarzik> * some temporary files such as journals are created along the way, and must exist for recovery post-crash
< sipa> ok, similar to bdb
< sipa> that's fine for application database stuff
< sipa> maybe not wantes for wallets
< sipa> *wanted
< jgarzik> sqlite files do want periodic maintenance via 'VACUUM'
< sipa> we can do that in our flushtodisk function
< jgarzik> they get fragmented, old records cluttered, performance degrades a bit over time
< jgarzik> it is a very, very heavyweight operation
< jgarzik> auto_vacuum handles large deletes, but not fragments over time
< jgarzik> could just run it at startup and then auto_vacuum
< sipa> ok
< btcdrak> I've not seen everyone get so excited about something for a long time.
< wumpus> "The author disclaims copyright to this source code. In place ofa legal notice, here is a blessing: May you do good and not evil. May you find forgiveness for yourself and forgive others. May you share freely, never taking more than you give." I love sqlite's appraoch to licensing
< btcdrak> wumpus: if only there were more like that
< jgarzik> At https://www.sqlite.org/cvstrac/wiki?p=KeyValueDatabase sqlite seems across the board worse... except for large numbers of records (our use case)
< jgarzik> I'm happy to create an sqlite branch for bitcoind, if nobody else is working on it
< btcdrak> jgarzik: go for it
< sipa> how mature is sqlite4?
< jgarzik> good question
< jgarzik> vast majority in field is 3
< sipa> its design seems much closer to leveldb
< sipa> as it is... a key/value store internally
< sipa> i wonder if there is an api to access that key/value store interface
< sipa> i have read before that you can
< wumpus> I was starting on it, but will happily leave it to you jgarzik
< sipa> The default built-in storage engine is a log-structured merge database. It is very fast, faster than LevelDB, supports nested transactions, and stores all content in a single disk file.
< sipa> ^ from the sqlite4 design
< wumpus> where do you seea nything about sqlite4? at least the download page only has 3
< sipa> ok, sqlite4 is far from released
< wumpus> oh, "trunk" :-)
< sipa> not an option at this point
< wumpus> sounds great though
< sipa> alsoz sqlite3 does not promise backward write compatibility... so it is like bdb, and we need to guarantee only increases in version
< wumpus> it does promise backward read compatibility?
< sipa> yes
< wumpus> well then it's not too bad, if it detects a downgrade, it could export|import the database
< sipa> wait
< wumpus> berkeleydb doesn't even guarantee that much
< sipa> databases created by 3.3 can't be read by earlier 3.x versions
< sipa> lol, and this caused so much problems they reverted to the old format by default in 3.3.6
< jgarzik> note - single file db implies large file support req
< sipa> is that not available on every platform these days?
< wumpus> is that unreasonable these days?
< jgarzik> every platform, but some filesystems may be e.g. fat32 for weird reasons
< wumpus> ... and has been for 10 years or so
< jgarzik> I think it's reasonable
< sipa> oh: a database created by version X will always be read/write compatible with version X, even if modified by later versions
< jgarzik> just noting
< wumpus> fat32 is pretty much dead
< sipa> so it's not an auto-update to whatever the format in the later version is, like bdb
< jgarzik> sipa, yeah, kernel filesystem rules
< wumpus> it also doesn't support bigger disks
< jgarzik> wumpus, sadly very much alive on USB sticks
< sipa> put bitcoind'd chainstate on a USB stick: you are eaten by a grue
< wumpus> *small* USB sticks
< sipa> also: for now the chainstate still fits in 2 GB...
< sipa> ... for now
< jgarzik> as I said... just making a note because it's something of which people should be aware. continuing to hack on it :)
< sipa> awesome, thanks
< wumpus> great
< wumpus> 32GB is pretty much the limit for FAT32 devices, it's possible to have larger volumes, but it becomes really ineffiecient and at least windows doesn't allow formatting them
< GitHub122> [bitcoin] MarcoFalke closed pull request #6866: [trivial] fix white space in rpc help messages (master...MarcoFalke-2015-rpcWhitespace) https://github.com/bitcoin/bitcoin/pull/6866
< morcos> sipa: i'm trying to think about how to preserve the state of the mempool while generating block templates
< sipa> morcos: how about being able to mark mempool txn as locked?
< sipa> individual transactions, that is
< morcos> sipa: how do you mean? the block template generation code needs to mark all of them as locked?
< morcos> or do you mean if you need to delete something, such as with RBF, then you mark it as deleted and go ahead and add the new one?
< sipa> morcos: while the template generator runs, it locks transactions one by one as they are added to the template
< sipa> so the next transactions it fetches are guaranteed to be not conflicting with the ones it already has
< sipa> you want a new incoming block to override that though, and cancel the generator to make it start over
< morcos> yes agreed with that last part, i'll tackle that later
< morcos> but i'm not sure how to just lock txs one by one, you'd have to lock the mempool.cs each time you went to look up a tx to decide to add it to the mempool
< morcos> i'm starting off by trying to modify the existing CNB to hold locks a lot less
< morcos> so i'm imagning it basically copies ptrs to all the txs in the mempool and the score it needs for them
< morcos> then it doesn't need to hold mempool.cs any more as long as the ptr does not become invalid
< morcos> which would only happne if the txn is deleted
< sipa> i guess transactions would get a refcount
< sipa> and the refcount value itself would be protected by mempool.cs
< morcos> but i think that has to be for all of the txs as you're running the logic of figuring out which ones need to be in the template
< morcos> yeah, but i was imagining it was a singular refcount
< sipa> hmm, does the mempool not hold some index sorted by template inclusion preference?
< morcos> instead of per tx
< morcos> yeah so thats where my approach might not be the best approach b/c the new template generation code will have such an index
< sipa> another idea is to remove the storage if txn from the mempool entirely
< morcos> but even still you need to iterate far past the size of a block
< sipa> you just get a transaction store manager, which you give transactions, and it returns a refcounted pointer
< sipa> the mempool stores the pointers
< sipa> i guess that's just smart pointers
< sipa> so the template generator would grab pointers to the top N (some multiple of a block), increase their refcount, release memlool.cs
< sipa> then goes off to build a block, verify if wanted, ...
< sipa> and then release the pointers
< sipa> the mempool can change during that time... you just know that the set you grabbed at that point while holding the lock is internally consistent
< sipa> and consistent with the block is was building on
< morcos> yes thats what i'm going for
< morcos> i guess it depends on how big the set is compared to the whole mempool
< morcos> so my idea was that rather than marking individual txs
< morcos> you just tell the mempool, hey, referring to things, don't delete
< morcos> and then occasionally that gets freed, and anything marked for deletion can happen
< sipa> hmm
< sipa> not sure
< morcos> since deletes are rarer and smaller than adds
< sipa> sounds more complicated
< morcos> ok, maybe i'll explore both
< morcos> next question and this might be related to CSV
< morcos> really annoying that CheckInputs needs cs_main
< morcos> doesn't seem like it should have to?
< sipa> so i think it's relative easy this way: turn the storage of txn in the mempool into smart pointers, and you can very cheaply and efficiently ask the mempool for the set of all its transactions
< morcos> you're storing the hashblock that its valid at, and that can't change height
< sipa> it shouldn't be needed, indeed
< morcos> sipa: not that easy, you need stuff from the mempool entries, thats where fees are stored
< morcos> so you still have to iterate to figure out which pointers you need and copy the meta information
< sipa> ok, so that fee information needs to be inside the smart pointed-to objects
< morcos> so all you've done now is taken the multi-index approach and say ehh forget that, lets just have multiple indexes referring to this same set of pointers
< sipa> i may be missing something
< morcos> i think the only complication from my idea is damn RBF... otherwise you just don't call any eviction code except periodically... and there is no problem running it only periodically
< morcos> and you don't even have to track anything
< sipa> what i'm suggesting is a way to make cheap snapshots of subsets of the mempool at a given time
< sipa> those snapshots themselves don't have an index, but can be ordered by one at the time they are created
< sipa> isn't that enough?
< morcos> yeah maybe, i think i'm just leary of the double indirection now where the multi-index is a multi-index of ptrs, but maybe thats something stupid to worry about
< sipa> the transactions inside the mempool already have a dozen or so indirections
< sipa> ok, more like 6
< sipa> but still!
< morcos> so something like a boost::shared_ptr
< sipa> yes, that specifically :)
< sipa> sorry, i thought it was called smart_ptr :)
< sipa> you probably want to wrap CTransaction inside something that also contains its (direct) fee and perhaps other immutable statistics
< sipa> and then have sharee_ptrs to those inside the mempool entries
< sipa> heh, we could even serialize the transactions inside to reduce memory usage...
< morcos> yes, so then it'll get a bit more complicated with ancestor package tracking when you have mutable state like ancestor fee rate that you want to copy to your template generation code, but that was going to be difficult period
< sipa> why do you need it in the template generator code?
< morcos> thats the sort the logic uses
< morcos> yikes
< sipa> sort the list of copied pointers according to the sorting criterion you want while creating it (so while holding mempool.cs)
< sipa> then release mempool.cs and you can forget it
< morcos> ok, but see thats the complicated part that takes a long time
< sipa> ah, ok
< morcos> the tx thats sorted at the top , really links to a lot of other txs , whose ptrs you have to grab to
< sipa> hmm, right
< morcos> but then how do you solve the problem of once you include that in your template
< morcos> everything elses sort changes
< sipa> i wouldn't bother with changing sorts
< morcos> well that'll be the magic of the algorithm, it'll probably have to be some heuristic, but its very easy to have expensive chain A-B-C and so C is sorted first, but then cheap C2 is still sorted quite high.
< morcos> uh not exactly but close
< morcos> but yes, i agree we don't have to be perfect
< morcos> ok you've given me some ideas... let me see what i come up with
< sipa> great
< sipa> feel free to completely disregard my ideas :)
< jgarzik> RE locking transactions one-by-one - that is the typical parallelism solution
< jgarzik> just need a gatekeeper (core struct lock) for insert/delete/move
< GitHub181> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/a09297010e17...2cd020d054d6
< GitHub181> bitcoin/master 3cb56f3 Daniel Cousens: *: alias -h for --help
< GitHub181> bitcoin/master 2cd020d Wladimir J. van der Laan: Merge pull request #6846...
< GitHub119> [bitcoin] laanwj closed pull request #6846: [Trivial] bitcoind: alias -h for -help (master...aliash) https://github.com/bitcoin/bitcoin/pull/6846
< GitHub145> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/2cd020d054d6...f2c869aef2e7
< GitHub145> bitcoin/master c6824f8 J Ross Nicoll: Add DERSIG transaction test cases...
< GitHub145> bitcoin/master f2c869a Wladimir J. van der Laan: Merge pull request #6848...
< GitHub77> [bitcoin] laanwj closed pull request #6848: Add DERSIG transaction test cases (master...bip66-tests) https://github.com/bitcoin/bitcoin/pull/6848
< gavinandresen> FYI: git HEAD master isn't working for me on OSX. Running make check gets me:
< gavinandresen> libc++abi.dylib: terminating with uncaught exception of type boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::lock_error> >: boost: mutex lock failed in pthread_mutex_lock: Invalid argument
< gavinandresen> I'm very confused, because git bisect blames a commit that works....
< cfields> gavinandresen: sounds like your boost may be built against the wrong stdlib? checking master on osx here now.
< gavinandresen> cfields: I thought that too, did a brew uninstall / brew install of boost
< btcdrak> morcos: sorry was afk. My other question was how does mempool evictions play into the mix? I assume one doesnt want to evict a txn that you're trying to mine.
< morcos> btcdrak: yes, thats what my concern was. the only way a pointer to a tx becomes invalid now is if the tx is deleted. A) throught being mined, in which case you want to abort block template generation anyway or B) through eviction or RBF
< morcos> for B1) eviction, i thought it would just be easy to just skip eviction if the template_tx_lock is held, and just make sure eviction gets to run every so often.
< morcos> but B2) RBF, doesn't have an ideal solution, i wanted to just flag those txs, but maybe sipas idea with shared_ptrs is cleaner
< cfields> gavinandresen: ok here. What compiler/boost/osx version?
< gavinandresen> cfields: Apple LLVM version 7.0.0 (clang-700.0.72), boost /usr/local/Cellar/boost/1.58.0, osx 10.10.5 ....
< gavinandresen> ... but it is very possible my machine is in some weird state, I upgraded my XCode yesterday.
< btcdrak> gavinandresen: first rule of fight club - "never upgrade XCode"
< jgarzik> heh
< jgarzik> just did last night...
< cfields> gmaxwell / jgarzik: ping. I must be going crazy. From what I can tell experimentally (and backed by man pages), on Linux, our incoming connections are being treated as blocking.
< jgarzik> cfields, post-accept(2) you set sockets to non-blocking
< cfields> because O_NONBLOCK isn't inherited by accept
< jgarzik> nod
< cfields> jgarzik: should, sure
< cfields> but we don't
< jgarzik> oops
< jgarzik> seems surprising, seems like it would have been well noticed before now
< sipa> that would imply that the "opportunistic write" always succeeds
< cfields> for the most part it should be ok because reads wait for select(), but i'm surprised it hasn't caused issues
< sipa> that should be trivial to test?
< jgarzik> no wait - send/recv have per-call NB flags
< cfields> jgarzik: ah right
< cfields> jgarzik: still though, i've read about select() acting funny on linux with blocking sockets
< jgarzik> like haha, that's a laugh funny?
< * jgarzik> attempts to implement db iteration without brute force 'select * from table' into RAM ;p
< cfields> "Under Linux, select() may report a socket file descriptor as "ready for reading", while nevertheless a subsequent read blocks... Thus it may be safer to use O_NONBLOCK on sockets that should not block."
< jgarzik> recv() will fail in that case
< cfields> Either way, forcing them to nonblock after accept can't hurt, right?
< cfields> hmm
< sipa> jgarzik: i guess you can get some sort of object/state which you can ask for "more results" ?
< cfields> it wouldn't just block?
< GitHub132> [bitcoin] MarcoFalke opened pull request #6870: [trivial] Misc cleanup and translations (master...MarcoFalke-2015-trivial3) https://github.com/bitcoin/bitcoin/pull/6870
< cfields> jgarzik: er, the DONTWAIT. nm.
< jgarzik> sipa, you can 'select *' and iterate in one direction (forwards; next-result), whereas the CDBIterator wants Next, Prev and Seek operations
< * jgarzik> is studying the callers/users of CDBIterator now
< sipa> jgarzik: i don't think we use prev
< jgarzik> so it seems
< sipa> and seek would be equivalent to just starting the search
< cfields> gavinandresen: that's no good. i'm confused about that error, though
< jgarzik> sipa, well seek-start, seek-end and seek-key are all different
< sipa> jgarzik: i think the abstraction can be changed so we have a few 'entry types' (which would translate to separate sqlite tables)
< sipa> jgarzik: and only have iterators for all-data-within-one-entry-type
< jgarzik> sure
< sipa> which would be compatible with both leveldb and sqlite
< jgarzik> ah a few of these db ops are like Prev(), unused. job easier++
< jgarzik> hrm
< jgarzik> sipa, lex order?
< jgarzik> seems so
< sipa> some things need lex order, yes
< gavinandresen> FYI: my build-failing-on-OSX problem went away with a 'git clean -dxf' then re-autogen/configure/build....
< * jonasschnelli> did accidentally sent untracked files to nirvana serval times with `git clean -dxf`...
< * sipa> too
< jonasschnelli> heh
< gmaxwell> I haven't tried it on bitcoin core yet, but the latest functionality in rr (replay debugger) looks pretty great: http://robert.ocallahan.org/2015/10/rr-40-released-with-reverse-execution.html
< jcorgan> did i hear rumblings about switching to sqlite?
< maaku> i wish there was a 'git stage --clean' command
< btcdrak> meeting over at #bitcoin-dev now
< jcorgan> isn't it git checkout . ?
< btcdrak> jcorgan: yes, jgarzik is working on it now
< jcorgan> oh sweet. then we can use sqlcipher and get full db ondisk encryption
< jcorgan> it's a drop-in replacement/fork for sqlite that operates at the db page level, like dmcrypte for block devices
< sipa> why do you need encryption for public data? :)
< maaku> sipa: i presume he's talking about wallet?
< sipa> i wouldn't use sqlite for the wallet
< gmaxwell> jcorgan: you saw that discussion, but I expect it won't happen (At least with sqllite3) as-- if I'm not mistaken-- btcd tried it and found the performance unacceptable. (but we should anyways, both to have a comparison point, and because sqllite4 might be faster even if 3 is too slow)
< jcorgan> encrypt ALL THE THINGS
< gavinandresen> RE: switching to sqlite: this CACM article was very interesting: http://cacm.acm.org/magazines/2015/10/192379-crash-consistency/abstract
< gavinandresen> ... bashes on leveldb a bit, praises sqlite ....
< * maaku> is very confused as to what the benefit of switching to sqlite would be for this particular use case
< jgarzik> maaku: it's maintained and reliable
< gavinandresen> maaku: assuming sqlite is better about crash consistency (and from all I've read, it is), would cut down on the "core sucks, because my PC crashed and then eleven days to reindex..."
< sipa> yes, that's the only reason
< sipa> sqlite is known to be rock solid
< sipa> however, performance would need to be acceptable
< maaku> sipa: well, also interop and the ease of using a different sql backend
< jcorgan> i would definitely advocate making a sql abstraction layer with pluggable back ends, with sqlite as the default
< CodeShark__> I'm using ODB in my stack for that
< sipa> jcorgan: to be clear, i think SQL is a downside
< sipa> we're not looking for a database, and turning things into a database is against what we're trying to achieve
< jgarzik> nod
< jcorgan> my secret plan revealed, again
< sipa> our database is not designed to be accessible by multiple processes
< sipa> nor do we need any fancy indexes or queries
< gavinandresen> if there's a key-val store as well-supported and rock solid as sqlite..... that's the right answer.
< jgarzik> I still like the custom db approach, but I'm highly biased
< sipa> if sqlite offered a mechanism to just use its low-level key-value store, i would
< sipa> (because it's a database layer built on top of a key-value store)
< jgarzik> program in sqlite VM assembler and you can...
< * jgarzik> runs
< maaku> imho the answer would be to switch to a better maintained descendent of leveldb (there are many)
< maaku> also minimizes risk of fork
< gmaxwell> Its unhelpful to have this conversation in parallel with the meeting.
< kanzure_> upstream maintainers, apparently
< kanzure_> oops, bad scrollback
< bsm1175321> Don't want to interject in the meeting, but I re-ran the SQLite vs. libdb benchmarks sipa mentioned (because they were 6 years old).
< kanzure_> huh? why should that not be mentioned?
< bsm1175321> Because the topic has moved on, and the conclusion was to continue research.
< bsm1175321> Above link updated with 500k records, transactional DB. (Took a few minutes to run)
< BlueMatt> IIRC gmaxwell was gonna ask the crash consistency guys for their test harness?
< BlueMatt> so we could test other dbs
< BlueMatt> or maybe I'm crazy
< BlueMatt> someone should do that, though
< BlueMatt> before we decide
< sipa> maaku: sqlite is better at large writes than leveldb
< maaku> btcdrak: sqlite4 maybe matches leveldb ... because it's basically using leveldb's datastructure under the hood
< sipa> as leveldb writes everything twice tovdisk
< btcdrak> gmaxwell: this was the discussion earlier today about sqlite: https://botbot.me/freenode/bitcoin-core-dev/2015-10-22/?msg=52492157&page=2
< gmaxwell> btcdrak: yes I read it. I am confident from expirence that its incorrect, but I still think jeff's work is very useful.
< btcdrak> gmaxwell: yeah, I was also surprised, but fingers crossed something impressive comes out of jeff's research.
< gmaxwell> (in part because sqllite4 may be faster)
< btcdrak> sqlite4 is still in development though isnt it?
< gmaxwell> sure, not useful yet.
< btcdrak> i wonder how far on they are and if they have a release schedule yet
< Luke-Jr> am I missing something? why is this being treated as a non-consensus thing?
< sipa> it obviously is consensus critical
< sipa> it's not a consensus change though
< Luke-Jr> we don't and can't know that.
< sipa> recompiling with a different libc is also consensus critical
< Luke-Jr> replacing 26kLOC to 140kLOC is impossible to be sure on
< BlueMatt> Luke-Jr: we should be confident that it is not before anything moves forward
< sipa> or running on a new linix kernel
< Luke-Jr> BlueMatt: I don't think we can.
< helo> i really appreciate the meetings, great idea (btcdrak?)
< btcdrak> helo
< sipa> we are making certain assumptions from other software we are using
< BlueMatt> Luke-Jr: then we're stuck with a shitty db that corrupts randomly and isnt self-consistent (ie not consensus-compatible)
< Luke-Jr> best I can see happening, is knowing it's unlikely to have any accidentally-triggered failures
< gmaxwell> Luke-Jr: it's not being treated as anything right now, it's just a test.
< sipa> that includes compilers, libraries, and operation systems with _well specified_ behaviour
< gmaxwell> Luke-Jr: by that basis we don't know that leveldb is self-consistent, or consistent between windows / unix, etc.
< maaku> Other than Windows corruption errors, is this solving any real problems right now?
< btcdrak> Luke-Jr: research is a good thing.
< gmaxwell> maaku: we have durability problems on all platforms, though the windows ones are very severe.
< Luke-Jr> btcdrak: yes, if this is just research fine
< gmaxwell> The performance of leveldb is also problematic already, (well reported for very large databases), enough that our fat caching layer is very very criticial for performance, and thats causing us other problems.
< gmaxwell> maaku: durability problems in this database are incompatible with the survival of a network where puning is widely used.
< sipa> puny network
< gmaxwell> hah
< gmaxwell> pruning*
< btcdrak> LOL
< wumpus> in general leveldb works fine; greatest concern is that it seems to be no longer maintained, so any bugs (like corruption on crashes in windows) is unlikely to get solved
< maaku> i thought gmaxwell was declaring sipa a risk to bitcoin there
< gmaxwell> maaku: well that much is obvious.
< sipa> we couls try to find another win env for leveldb
< sipa> there may be bugs in that
< jonasschnelli> i know this sounds hard: but did we never consider to stop supporting windows? Would this be so wrong?
< wumpus> I'm sure it is possible to fix the windows issue, but it's not really nice to inherit a dependency and suddenly have to maintain it
< Luke-Jr> jonasschnelli: I think it would be problematic to do so right now.
< Luke-Jr> jonasschnelli: as bad as it is, many people still use it
< gmaxwell> jonasschnelli: I think it would be a bad idea, its a very widely used platform and dropping it would be a major move against the ability of people to independnantly run, audit, etc. the bitcoin system.
< wumpus> jonasschnelli: well I've been close to declaring that. it wouldn't be pretty but we have zero active developers for windows.
< gmaxwell> The security problems of windows stink, but hardware wallets are a tidy tool to address that.
< wumpus> jonasschnelli: then again, the software is working for a lot of people also on windows
< jonasschnelli> not even speaking for the wallet. Just the node/core itself.
< gmaxwell> we are in a state right now where it is is almost constructively non-supported by our failure to do a a great job about it.
< maaku> well to be clear, gmaxwell's concerns about a pruned network wouldn't apply if the durability issues were wholly isolated to the windows platform
< Luke-Jr> jonasschnelli: then people will just stop running a node :/
< wumpus> gmaxwell: I disagree
< Luke-Jr> before we drop Windows support, we would ideally want the alternative to be "run a node on Linux", not "stop running a node altogether"
< gmaxwell> maaku: they are not wholly isolated to windows, though perhaps its safe enough elsewhere that my concern is less of an issue.
< jonasschnelli> wumpus: Agree. It's running. But i think we should not move away form leveldb because of windows...
< wumpus> gmaxwell: many people are running it on windows, and not complaining at all
< wumpus> gmaxwell: no need to blow this issue out of proportion
< gmaxwell> wumpus: and many people have stopped running it too.
< wumpus> gmaxwell: obviously
< gmaxwell> I believe more people have stopped running bitcoin core on windows than currently run it.
< gmaxwell> (stopped due to corruption issues of varrious forms)
< wumpus> well this is an open source project - if we don't have developers supporting windows, we can't support windows
< jonasschnelli> are most corruption issues caused by leveldbs layer?
< gmaxwell> (not just leveldb but also AV)
< sipa> jonasschnelli: we don't know
< gmaxwell> We know there is leveldb corruption on unclean shutdown, and we know about antivirus mediated corruption. But there may be more things we don't know about.
< wumpus> if the corruption issues are as bad as you say they are, and no one is fixing them, then we should indeed drop windows support
< Luke-Jr> maybe if we stop supporting Windows "officially", but leave it open, others will step up to do it
< wumpus> but I don't believe you
< gmaxwell> wumpus: or we should get more windows developers.
< wumpus> gmaxwell: well, good luck with that
< gmaxwell> Which can be done.
< gmaxwell> wumpus: I mean I can _hire_ people for this, but it hasn't been on my radar.
< wumpus> in any case, this is not constructive *goes back to sleep*
< jonasschnelli> we could ship it together with a vmbox ubuntu. *duck*
< * Luke-Jr> wonders if Diapolo is good enough at C++ that he could transition to more than just GUI if he gets funding
< * jonasschnelli> looks strange at Luke-Jr
< gmaxwell> wumpus: :-/ I think you're misunderstanding my perspective, sorry I've communicated poorly.
< Luke-Jr> jonasschnelli: got a better suggestion? :P
< Luke-Jr> I don't know many Windows developers..
< jonasschnelli> Luke-Jr: hah. No. Not really. :)
< jonasschnelli> serious: what would be the overhead to run bitcoin-qt/core in a vm on windows?
< Luke-Jr> maybe the people who did those libconsensus bindings?
< wumpus> gmaxwell: it's not impossible to just solve the windows crash corruption issue
< jonasschnelli> 10% CPU loss, 512MB ram overhead?
< Luke-Jr> jonasschnelli: I would be surprised if a VM kill reproduced the problem
< wumpus> gmaxwell: I even proposed to help with it today if someone can send me a corrupted database
< gmaxwell> wumpus: yup. did anyone do that?
< wumpus> gmaxwell: no
< jonasschnelli> Luke-Jr: the idea would be to bundle bitcoin-qt/core on windows together with vbox and a tiny distro. This would eliminate some ugly platform dependents.
< Luke-Jr> gmaxwell: I tried to, but couldn't reproduce it with IBD, and shortly thereafter my USB Armory died entirely. So it seems likely my problem was a bad microSD
< Luke-Jr> jonasschnelli: that sounds like a terrible UX
< jonasschnelli> Luke-Jr: the UX could still be native (once it's decoupled)
< wumpus> Luke-Jr: yes, your problem is likely adifferent one. You're the only one reporting it on linu
< wumpus> jonasschnelli: if you go that far to make a bundle you may as well fix the windows version :-) (it would be about as much development work)
< gmaxwell> jonasschnelli: doesn't jive really well with the resource costs of running a node.... VM with the constantly expanding storage is no fun. :P plus overheads. :P
< Luke-Jr> does UML support Windows? ;)
< gmaxwell> Fixing the windows shouldn't be a big deal once we get enough repros and data, similar to OSX.
< gmaxwell> sounded like it might be the same issue.
< wumpus> yeah, let's just solve the database problem instead of trying to work around it
< wumpus> even if we have to put a flush after every line in leveldb ...
< gmaxwell> I considered the exploration of other options to be orthorgonal with windows being on fire FWIW.
< jonasschnelli> gmaxwell: nowadays VM overhead is tiny... it's not perfect. But better a solution that works with a lost of 10-20% in overhead than unsolved corrupted databases.
< wumpus> gmaxwell: sure, I still like the idea of exploring other databases
< wumpus> but yes, orthogonally
< sipa> i really like the idea of just being able to use a maintained and tested database
< gmaxwell> (also the attempt may turn up bugs elsewhere in our codebase)
< wumpus> don't we all
< sipa> and not a hack we had to pull together
< * wumpus> thought leveldb was that
< gavinandresen> a bug bounty worked for the last leveldb corruption issue we had (if i recall correctly). I'm still holding some bitcoin in the core dev expenses fund
< sipa> wumpus: leveldb windows certainly isn't
< Luke-Jr> jonasschnelli: if someone really wants to investigate VM stuff, I'd suggest instead a thin OS that runs *Windows* in the VM, and provides a hardwarewallet-like interface on top
< jonasschnelli> is there an approach to test sqlites performance which represents our db-style?
< sipa> wumpus: we have local modifications to the win env
< wumpus> jonasschnelli: best would be to just try it
< gmaxwell> jonasschnelli: yes, testing bitcoin core using <alternative database> :P it shouldn't be that much work; presumably jeff will report back on that soon
< jonasschnelli> sqlite could initially be slower. But it's better maintained and much more portable.
< sipa> jonasschnelli: there is slower and slower
< jonasschnelli> Indeed
< sipa> if it's slower for things our caches compensates for, who cares
< gmaxwell> jonasschnelli: speed is a security consideration for us currently, if you were talking about 5% or something I'd agree. but thats now what I expect.
< gmaxwell> s/now/not/
< sipa> if it's slower to the point that it affects block propagation, it's a no go
< gmaxwell> s/currently/sadly/
< * wumpus> doubts it will make much of a difference
< jonasschnelli> Quote from sqlite4: The default built-in storage engine is a log-structured merge database. It is very fast, faster than LevelDB, supports nested transactions, and stores all content in a single disk file. Future versions of SQLite4 might also include a built-in B-Tree storage engine.
< jonasschnelli> promising. :)
< wumpus> there are so many factors influencing performance of bitcoind, that a slightly slower database won't be a big issue
< sipa> yes, sqlite4 sounds awesome _once_ it has had the same amount of testing and battle hardending as sqlite3
< wumpus> jonasschnelli: yes that was pasted before today :)
< jonasschnelli> ha.. okay. I'd love to play around with it. But my stack of things to work down is just to big right now.
< gmaxwell> we might also want to maintain a patch for sqllite4 and use it to try to get the sqllite devs to use us as a test harness. :P
< jonasschnelli> sipa: sqlite is widely used, also in almost every browser (local dbs) and smartphone (android and iOS IIRC). They very likely to update once to sqlite4...
< sipa> but quote from one of the sqlite devs a few months ago "sqlite4 is a dev toy"
< jonasschnelli> sipa: that sounds perfect for our bitcoin <1.0 version. :)
< sipa> jonasschnelli: if only we didn't have this pesky economy thing that relies on bitcoin
< jonasschnelli> sipa: na... that are just some fiat exchange bubbles.. :)
< gmaxwell> Lets fix that first then. :P
< wumpus> trying it out doesn't hurt, no one is talking about releasing with it...
< sipa> wumpus: exactly
< sipa> dev toy :p
< jonasschnelli> wumpus: +1 ... and a such pr would at least take 1/2 year to get in.
< gmaxwell> well as I said, might even be possible to get the sqllite developers to test using us. We're a pretty cool load generator (esp with signatures off)
< sipa> gmaxwell: we're boring
< sipa> single-threaded access, only bulk writes and small random reads
< jcorgan> < Luke-Jr> jonasschnelli: if someone really wants to investigate VM stuff, I'd suggest instead a thin OS that runs *Windows* in the VM
< jcorgan> *cough* Qubes OS *cough*
< Luke-Jr> jcorgan: Qubes OS is not very thin, and doesn't do GPU passthrough
< jcorgan> didn't think about GPU
< jcorgan> but thinness is poorly defined
< btcdrak> sipa: are the tests in #6816 (versionbits) enough? I was thinking it would be good to have some rpc-tests there as well, generating blocks and running through a couple of scenarios.
< btcdrak> or is that overkill?
< CodeShark> btcdrak: I think we should probably do some regtests with the integration, not necessarily over RPC
< CodeShark> well...
< CodeShark> other than just generating blocks
< CodeShark> I guess just calling generate and getblockchaininfo
< btcdrak> jgarzik: That was fast
< CodeShark> jgarzik: nicely done :)
< sipa> yes, but does it run linyx?
< CodeShark> btcdrak: the -blockversion thing will probably not be a good idea anymore using versionbits
< CodeShark> it will be better to provide a list of BIPs you don't want to set the bit for
< jgarzik> a few bits can be forwarded upstream immediately, like Seek* and Prev() removal
< btcdrak> CodeShark: well obviously the bipdersig.py tests are for an ISM sf.
< btcdrak> CodeShark: I'm just saying a set of tests like that which generate blocks and simulate a sf rollout but using versionbits protocol.
< CodeShark> I suppose we can just use VERSION_HIGH_BITS and VERSION_HIGH_BITS | 0x1 instead of versions 2 and 3 :)
< Luke-Jr> CodeShark: does versionbits sanitise the encoding btw? ;)
< Luke-Jr> ie, define the first 32-bits to be a big endian number
< maaku> Luke-Jr: it's little endian
< Luke-Jr> would be nice to fix that at the same time
< CodeShark> encoding? versionbits doesn't deal with serialization stuff
< btcdrak> CodeShark: it makes the most conclusive tests that it works, and provides some protection against regressions.
< maaku> Luke-Jr: why? nothing else is big endian
< Luke-Jr> CodeShark: anything consensus-relevant must deal with encoding
< maaku> Luke-Jr: he means versionbits doesn't touch serialization
< CodeShark> Luke-Jr: versionbits deals with block header data that has already been deserialized
< Luke-Jr> maaku: big endian is standard for protocols; and hashes at least are
< sipa> Luke-Jr: please
< sipa> we're not changing the serialization of block headers
< jgarzik> big endian is dead
< sipa> no endianness flamewar please
< CodeShark> in any case, the serialization format is entirely a separate issue...versionbits will play no part in that either way
< Luke-Jr> so the bits are numbered 7 6 5 4 3 2 1 15 14 13 12 11 10 9 8 23 22 21 20 19 18 17 16 31 30 29 28 27 26 25 24 ?
< sipa> irrelevant
< Luke-Jr> …
< CodeShark> versionbits deals with ints, uint32_t, etc...
< Luke-Jr> it's relevant because if I set the wrong BIP, it won't work.
< sipa> the block header serialization defines nVersion as a 32-bit little-endian signed integer
< Luke-Jr> wrong bit*
< CodeShark> it doesn't care what the underlying representation is for the particular architecture
< sipa> versionbits changes the semantics of the nVersion integer
< CodeShark> by the time versionbits comes into play, the version field has already been decoded into an int
< Luke-Jr> CodeShark: that is an implementation detail. the whole point of version bits is that it is no longer an int, but a bit array
< CodeShark> to test a bit, you do (nVersion >> bit) & 0x1
< sipa> Luke-Jr: it's still an int; an int in which particular bits are set
< * Luke-Jr> sighs.
< sipa> now please stop this silly discussion, we're not redefining how integers work
< gmaxwell> but, base-3!!!
< Luke-Jr> fine, not worth the time to argue about this stupid design decision.
< CodeShark> base-3 would allow us to do yea/nay/abstain :p
< Luke-Jr> the annoying part will be having to answer questions in a few years why it's misordered such. like I had to do with the getwork data for years
< phantomcircuit> jgarzik, you have benchmarks for sqlite?
< phantomcircuit> my experience using it in the past has been that it's slow as hell for inserts or any kind of concurrent access
< phantomcircuit> probably time would be better spent on building an interface spec for databases
< CodeShark> for insertions we could just use a raw file - the blockchain is a linear structure anyhow. then all we would need to do in sqlite is keep track of file positions ;)
< phantomcircuit> CodeShark, utxo only
< CodeShark> yeah, so we maintain a utxo index
< CodeShark> or are you talking about a pruned node?
< phantomcircuit> the index would be roughly the same size as the utxo data :P
< CodeShark> lol
< CodeShark> ok, for a utxo database perhaps it doesn't make so much sense
< phantomcircuit> yeah the actual data is really small
< gmaxwell> CodeShark: txindex works like you suggest there.
< gmaxwell> but for utxo set we get benefits from reducing the working set. saving 20 bytes of scriptpubkey/value in exchange for another random seek to the middle of some huge block file where 2/3rd of the data is not utxo relevant, basically defeates disk caching entirely. :)
< GitHub160> [bitcoin] petertodd opened pull request #6871: Full-RBF with opt-out (master...2015-10-rbf-with-opt-out) https://github.com/bitcoin/bitcoin/pull/6871
< bsm1175321> G
< GitHub51> [bitcoin] TheBlueMatt opened pull request #6872: Remove UTXO cache entries when the tx they were added for is removed/does not enter mempool (master...limitucache) https://github.com/bitcoin/bitcoin/pull/6872
< dcousens> petertodd: I hope you don't mind my questions :)
< phantomcircuit> BlueMatt, the last commit on #6872 seems to evict from the cache things which other transactions in the mempool want cached
< phantomcircuit> ie AcceptToMempool fails because it's a double spend of something with a higher feerate
< petertodd> dcousens: no worries!
< phantomcircuit> scratch that
< phantomcircuit> i missed the check
< petertodd> dcousens: I'm doing three things at once so the responses are probably sounding a bit terse :)
< BlueMatt> phantomcircuit: ahh, ok, good, i was about to say
< dcousens> petertodd: all good, I read everything in good spirit, just hope it comes across the same :)