< gmaxwell>
jarthur: I think they are in practice, but as a general rule a test should try to avoid being sensitive to things other than what they're testing, otherwise it makes them brittle.
< phantomcircuit>
the white space linter is confusing when run against multiple commits the corresponding message are nonsensical
< jarthur>
gmaxwell: thanks
< phantomcircuit>
aight i have no idea why it's failing that test and cannot reproduce locally
< phantomcircuit>
the only -6 is see as a constant is the addrman check but that should just write to the log
< gmaxwell>
I dunno what -6 means there... like if it died on due to an unhandled signal it would one hundred and something.
< gmaxwell>
perhaps, $ errno 6
< gmaxwell>
ENXIO 6 No such device or address
< gmaxwell>
there might be a case where the test triggers an error that the old code ignores and the new code propagates.
< gmaxwell>
like what happens if you try to bind to 127.0.0.1 but there isn't a 127.0.0.1?
< phantomcircuit>
gmaxwell, should fail in the same ways im pretty sure
< gmaxwell>
in any case a reason that test may fail on travis and work for you, is that the travis enviroment probably has pretty different networking.
< phantomcircuit>
yeah im sure it does
< gmaxwell>
And the reason it would matter is if under that case, you handle errors differently, e.g. the actual bug may be elsewhere in bitcoin or in the test but were hidden by the old behavior. The fact that you can't reproduce it locally is kind of annoying. you could try to figure out which test case is failing, by adding commits to change the test.
< phantomcircuit>
gmaxwell, indeed
< phantomcircuit>
gmaxwell, i guess travis doesn't have ipv4?
< phantomcircuit>
it's specifically the ipv4 rpc bind that's failing
< phantomcircuit>
hmm it doesn't , not specifically 127.0.0.1
< phantomcircuit>
which is what the test tries to bind to
< phantomcircuit>
gmaxwell, blargh there's still the select() calls in netbase
< kallewoof>
we just edit? I thought credits were automagic
< jonasschnelli>
Bitcoin Qt looks a bit strange in OSX 10.14 (Mojaves) dark mode. :)
< wumpus>
kallewoof: the lists are generated automatically but always need human editing
< wumpus>
kallewoof: the list of PRs is generated from github API data, the authors list from git commit authors—this might be what causes the divergence in this case
< wumpus>
jonasschnelli: that's curious, I always run it with dark gtk themes and that seems to go well
< jonasschnelli>
wumpus: Could be because of the different platformstyles... investigating now
< jonasschnelli>
black icons on black background are not ideal.. but at least you can see them
< jonasschnelli>
correction: black icons on dark-gray background
< wumpus>
jonasschnelli: yes that's it, I think on UNIX it does an attempt to grab the icon color from the theme, on MacOS is defaults to black
< jonasschnelli>
jup...
< wumpus>
kallewoof: that's it; the commits in 9991 are by JeremyRubin and apparently my script lists authors only, not committers
< wumpus>
kallewoof: I'll try to fix the script, otherwise I'll just edit him in manually
< wumpus>
kallewoof: added in committers to my list-authors script; looks like Dorier was the only one missed to this
< wumpus>
phantomcircuit: travis does have IPv4, but no IPv6 IIRC
< wumpus>
(*not even localhost*)
< echeveria>
I fixed testnet, for what it's worth. the most work is now in a valid chain.
< jonasschnelli>
wumpus: using the dark arc theme in Ubuntu Bionic, the background of Bitcoin Qt is still light gray/whiteish? Is that expected?
< wumpus>
jonasschnelli: I don't think so; the bitcoin-qt background should be the same as other gtk applications, say "charmap"
< wumpus>
jonasschnelli: though that might only work if you build from source, theme integration is not available with the shipped binaries
< jonasschnelli>
aha... I see.
< wumpus>
(the latter because it's based on plugins, which are not available when building qt statically)
< jonasschnelli>
wumpus: I would have expected that the OS provides some sort of color scheme for background, etc. (macOS Mojave) handles it that way). Qt could use OsBackgroundColor(), etc. for certain things.
< wumpus>
huh—looks broken on ubuntu 18.04, I get a light grey background now too w/ Adwaita-dark theme
< jonasschnelli>
«The problem has occurred since Qt5.7. In this release, the GTK2 platform theme and style was removed and replaced with the GTK3 platform theme»
< wumpus>
but how! I'm sure my theme also covers gtk3
< wumpus>
'charmap' is gtk3 and has a dark grey background
< * jonasschnelli>
loving QT
< wumpus>
I guess... it is the least of evils, with regard to cross-platform GUIs. Though losing theme integration on by far most linux distributions (except KDE-based ones?) is a pity.
< wumpus>
installing qt5-style-plugins and then doing 'export QT_QPA_PLATFORMTHEME=gtk2' works here, but that's helluva awkward
< hebasto>
wumpus: what is the way to build bitcoin-qt against system qt?
< wumpus>
hebasto: it does that by default if you don't do a depends build
< wumpus>
just follow the steps in build-unix.md
< wumpus>
that way, bitcoin won't magically install its own qt; so it will build against system qt if available, and not build against qt at all if not available
< wumpus>
this is the preferred way, we even used to ship that way for qt4, but unfortunately the range of portability of the executables is much smaller then so it's not an option anymore (then again, nothing of this matters anymore, if qt upstream broke desktop integration...)
< hebasto>
wumpus: thank you
< provoostenator>
wumpus: "great" to hear macOS isn't the only platform having QT headaches now.
< wumpus>
GUIs were a mistake
< Sentineo>
an audio interface would have been betterperhaps :)
< wumpus>
direct brain interface ftw :)
< Sentineo>
yeah :)
< promag>
hebasto: hi
< hebasto>
promag: hi
< promag>
hebasto: I don't have space to install fedora :/ but I hope to get around that today/tomorrow
< hebasto>
promag: I see
< promag>
hebasto: can only test by then
< hebasto>
promag: don't be hurry, I'm working on your suggestions right now
< promag>
so in fact it's necessary to not map?
< hebasto>
yes, it is. I've tested on fedora.
< hebasto>
promag: would you mind to look into #14222?
< provoostenator>
14289: one "solution" could be to recommend people to resync if they're still on v0.13, since it's impractically slow anyway even without the memory problem.
< jonasschnelli>
We just need to make sure to communicate the standardness change in 0.17.0
< provoostenator>
Or they can install 0.15 first, wait for that process to finish and then install 0.17
< jonasschnelli>
meh
< sipa>
hi, i'm sortof here - ping me if needed
< wumpus>
provoostenator: agree; though putting a message in the code itself if people are upgrading from 0.13.0 would make sense, for those not carefully reading the release notes, but anyhow
< gmaxwell>
I think even though 0.16 appears to have added the replay bloat, 0.17 makes bloat worse because it adds an additional place where they're queued. (this doesn't change that I think notices are probably the best move for now)
< wumpus>
but yes for the most common case (0.13.0 upgrade rollback), adding a message to the release notes would be acceptable solution
< luke-jr>
couldn't we detect the reorg needed, and just prompt for user action instead of trying to upgrade it?
< provoostenator>
If it can be done safely, having the upgrade simply refuse and throw an error message seems safer than just a release note.
< sipa>
gmaxwell: i'm not sure anything was added in 0.17
< sipa>
i blamed the txindex change, but the asynchronous processing was added earlier
< wumpus>
so I guess there isn't really a hurry to release 0.17.0 at this point
< gmaxwell>
sipa: txindex also schedulers queues block connections and disconnection, no?
< gmaxwell>
regardless... We don't yet have a proper fix for the issue, and I don't think we're still learning much about 0.17rc.
< sipa>
gmaxwell: i think (sorry no time to look now) is that 0.17 just added the txindex as a listener for those blockconnected events; the issue is not that, but the events in the first place
< gmaxwell>
ah.
< sipa>
my suggestion is to just point out in release notes that upgrading from 0.13.0 or before is a known memory bloat issue, which can be worked around with -reindex
< gmaxwell>
I didn't walk through the patches so it wasn't clear to me that the events existed before. Got it.
< luke-jr>
(ActivateBestChain actually checks for the queue length and blocks on it getting caught up)
< sipa>
luke-jr: it does; but RewindBlock and InvalidateBlock don't
< luke-jr>
sipa: would it be so bad if they did?
< sipa>
luke-jr: they need to release cs_main for that, which would be a major refactor for those functions
< gmaxwell>
luke-jr: that can be tricky to do safely as car has to be taken to make sure they don't wait while holding any locks.
< gmaxwell>
care*
< luke-jr>
hmm
< sipa>
but we can probably remove the upgrading logic from pre-segwit blocks anyway - as provoostenator says, it's already pretty unwieldy even without the memory bloat issue
< gmaxwell>
yea, reindex might already actually be faster.
< sipa>
i'm pretty sure it is
< gmaxwell>
but I assumed we'd want to use the rewind for future softforks.
< provoostenator>
Does reindex just toss out block files it doesn't need?
< sipa>
i don't care so much that invalidateblock takes a lot of memory - it would be a nice to have if we could actually make it revert to genesis, but it's not a priority
< sipa>
provoostenator: yes
< gmaxwell>
sipa: uh it's a little worse than that.
< gmaxwell>
I mean it hits 64+GB usage rewinding only a couple months of blocks.
< sipa>
yeah, ok
< provoostenator>
And it doesn't stop once it's going.
< gmaxwell>
indeed, and it doesn't return the memory, ever.
< sipa>
we'll need to refactor InvalidateBlock to deal with that; doesn't sound impossible, but probably for 0.17.1?
< gmaxwell>
Not a reason to hold 0.17, but it's not an irrelevant inefficiency.
< sipa>
fair
< gmaxwell>
sipa: sounds fine to me.
< luke-jr>
<2% of the network (<2000 nodes) run non-segwit software at this point; throwing an error that we can't upgrade them anymore seems reasonable
< wumpus>
yes
< sipa>
luke-jr: that's a useful statistics
< gmaxwell>
I still think we shouldn't just can the rewinding code though.
< provoostenator>
Maybe also throw an error if the user tries to invalidate more than 10K blocks? They can always do it in increments.
< gmaxwell>
ugh.
< gmaxwell>
just release note it, then we'll fix it later, please don't add additional falure cases to the function.
< wumpus>
agree with gmaxwell
< wumpus>
please don't overdesign temporary code
< sipa>
the refactor will effectively just do that - split it up in batches of X blocks to disconnect at once
< wumpus>
this needs to be fixed properly, in master, and in the future we should be careful to review anything that adds a queue or global data structure for this possiblity
< sipa>
agree
< wumpus>
but don't spend too much time on the workaround
< provoostenator>
I guess anyone upgrading all the way from 0.13 will probably pay more than average attention to the Upgrade Notice section in bold at the top...
< wumpus>
I think most people still running 0.13.x do so intentionally, and won't be upgrading to 0.17.x any time soon
< wumpus>
e.g. some users want to run old node software to cross-validate
< wumpus>
so: for 0.17.0, we should add a mention to the release notes for users upgrading from 0.13.0. This would require no new rc, so the way to final can continue as normal.
< achow101>
there's a pr for backports, will that be fore 0.17.1, or are those going in for .0?
< wumpus>
I haven't seen it, but unless they solve critical problems, they are not going in .0
< wumpus>
I guess this is a action item, that people shoudl try the PR locally?
< gmaxwell>
instagibbs had a related question, where are the bitcoind exit codes coming from. phantomcircuit's travis failure bitcoind has a return value of -6
< wumpus>
after the meeting, yes, doesn't make sense to do a real-time debugging session I think :)
< instagibbs>
I shall wait
< jonasschnelli>
would be fun... but yes. Better later.
< promag>
wumpus: topic suggestion: multiwallet
< wumpus>
#action try to run tests on #14336 on different environments to see if it reproduces travis error
< wumpus>
jonasschnelli: I haven't paid attention to the high-prio list at all this week, with the CVE issue and Riga travel so I'm not sure there's much to do there, but sure we can discuss it
< jonasschnelli>
I think Concept ack on both (promag)! Will test more soon.
< promag>
and I'm going to submit a refactor PR introducing WalletsModel
< promag>
that combines loaded wallets and existing wallets
< jonasschnelli>
wumpus: Yeah. I only wanted to ask for a review on #14046 from gmaxwell and sipa since they both commented already on it (fixed stuff)
< promag>
any objections to WalletsModel? IIRC it was previously suggested
< gmaxwell>
I'll look at 14046 again, thanks for the ping.
< jonasschnelli>
topic proposal: factor out berkeley-db
< wumpus>
#topic factor out berekey-db
< wumpus>
(jonasschnelli)
< jonasschnelli>
I tried this serval times but seems more complex then initially though..
< jonasschnelli>
But I think we should slowly consider alternative database systems (internal) for wallet files
< jonasschnelli>
And factroing out BDB should be done sooner then later
< jamesob>
where did the complexity come from?
< provoostenator>
Could be combined with jnewbery's standalone wallet tool, which can hold on to the direct dependnecy a bit longer.
< jonasschnelli>
jamesob: I think mostly due to the complex code layering...
< jonasschnelli>
I think switching the database backend on runtime should be possible....
< promag>
switching the database backend on runtime should be possible <- why?
< jonasschnelli>
I hope someone working on the wallet can initiate that refactor: jamesob, jnewbery, ryanofsky
< provoostenator>
I assume it makes most sense to move it to leveldb since we're using that for most other things?
< sipa>
leveldb is very annoying
< gmaxwell>
what gah no
< jamesob>
I don't think so; leveldb can't use a single .dat-ish file
< jonasschnelli>
promag: we must assume there will be a pretty long "transition" phase from the old to the new database layer
< jonasschnelli>
Not leveldb...
< sipa>
it needs whole directories, and provides way too many features
< gmaxwell>
and not a particularly good fit for what its used for here.
< jonasschnelli>
sipa one wrote a simple append only database...
< jamesob>
sqlite IMO seems like a good bet
< gmaxwell>
jonasschnelli: do we need to assume there is a long transistion instead of a standalone conversion util?
< wumpus>
yeah, though FWIW for the berkekeydb we also suggest a whole directory per wallet at the moment
< jonasschnelli>
(logdb),... there is a 2-4 year old PR somewhere (search after logdb)
< sipa>
sqlite is fine, though we also don't need any of its features, apart from safe updating
< gmaxwell>
wumpus: yes, but we know we don't like to do that. :)
< jamesob>
or honestly just a raw format of our own creation
< wumpus>
but the dangerous thing here is that anything you choose for the wallet will need to be supported more or less forever, so it's not an easy choice
< jonasschnelli>
gmaxwell: could also be a conversion utility,.. but at least the utility must be capable to run both database systems,... so won't change that much for the refactroing)
< provoostenator>
If we add another dependency, maybe pick one that's potentially useful for block and index storage?
< gmaxwell>
esp since we just end up loading the whole thing into memory, a fancy database is kinda overkill.
< jonasschnelli>
gmaxwell: agree
< sipa>
provoostenator: those things actually need efficient querying, atomic batch updates, ...
< wumpus>
we don't *need* to load the whole thing in memroy, that's a current design choice
< sipa>
provoostenator: for the wallet we just need a key-value store with some append only records :)
< gmaxwell>
(and also makes the files much more fragile and harder to work with than they would be otherwise)
< cfields>
sqlite is also nice in that they provide a monolithic source file and encourage direct integration.
< wumpus>
if there's proper indexing, loading every single transaction into memory could be avoided
< sipa>
yeah, i'm not opposed to sqlite
< jonasschnelli>
logdb (#5686) would be a simple append only hard to corrupt "database"... everything is hold in memory
< gribble>
https://github.com/bitcoin/bitcoin/issues/5686 | [Wallet] replace BDB with internal append only (logdb) backend by jonasschnelli · Pull Request #5686 · bitcoin/bitcoin · GitHub
< sipa>
it has very thorough tests
< luke-jr>
cfields: that's not a good thing. -.-
< provoostenator>
Someone once complained that the wallet didn't have atomicity guarantees.
< gmaxwell>
wumpus: indeed. but that decision should be made jointly.
< jonasschnelli>
Or sqlite... yeah
< wumpus>
I like sqlite, especially with deterministic wallets it wouldn't need to store all the keys
< jamesob>
sqlite seems like a pretty safe bet
< cfields>
luke-jr: the alternative is the reason we're switching away...
< wumpus>
so it's pretty much a metadata database, and sqlite is great for metadata and querying metadata
< sipa>
wumpus: with descriptor based wallets we don't need that anyway :)
< luke-jr>
cfields: what?
< wumpus>
sipa: even beter
< gmaxwell>
I don't think sqlite makes much sense unless the intent is also to move away from pulling everything into memory.
< jonasschnelli>
Is it guaranteed that sqlite databases are interoperatable between platforms and versions of sqlite?
< jamesob>
gmaxwell: what would you propose in lieu?
< wumpus>
please move away from loading everyting into memory in the long run
< gmaxwell>
And if we're going to do that, the scheme in the database matters a lot, so that change should probably be made at the same time.
< wumpus>
on the short term it's not a priority
< wumpus>
but don't make it impossible
< wumpus>
(in a new format)
< gmaxwell>
jamesob: if we're loading the whole thing into memory, a simple serialized format like logdb is I think vastly superior.
< jonasschnelli>
I agree with gmaxwell: sqlite makes most sense if we want to one active handling of merchant size wallets
< jonasschnelli>
and not sure if we want that
< promag>
does it have to be an embedded database?
< wumpus>
some large users of the wallet run into memory issues, and have to remake a new wallet perioidically because of this limitation
< sipa>
promag: no
< gmaxwell>
if we just use sqllite but then just treat it like a blob holder, then the whole schema will need to change to avoid memory loading it in any case.
< phantomcircuit>
jonasschnelli, think it makes most sense to have a tool which is a separate binary to convert from bdb to "new" wallet format
< jonasschnelli>
I don't think it has to be a "database" at all
< wumpus>
(due to storing all the transactions in memory, and also the time overhead of loading the whopping thing at startup)
< instagibbs>
wumpus, or abusing rpc calls to whiddle it down
< phantomcircuit>
and for the new wallet format to simply be a flat file
< wumpus>
instagibbs: oh :-)
< jonasschnelli>
phantomcircuit: I agree. But that tools would require the refactoring also
< cfields>
luke-jr: eh, not worth getting into it and muddying the conversation
< wumpus>
instagibbs: I mean, :-(
< gmaxwell>
wumpus: More than memory issues, they run into problems that many of our rpc operations iterate over all txn in the wallet and then become super slow.
< instagibbs>
gmaxwell, yeah that one
< phantomcircuit>
jonasschnelli, yes but has the advantage that you can write the conversion tool and then just rip out a ton of the walletdb logic entirely
< wumpus>
gmaxwell: right - another lmitation of not having indexing, either in memory or on disk
< instagibbs>
i know people who delete completely spent tx(plus 100 confs or something) to speed it wallets
< phantomcircuit>
which makes refactoring much easier, cause you dont have to support both simultaneously
< jonasschnelli>
phantomcircuit: the logic must still be available somewhere,... could be in a tool source only. yeah
< phantomcircuit>
cfields, sqlite doesn't actually provide a monolithic file in the same way bdb doesn't
< wumpus>
instagibbs: ah yes, the "wallet only needs a view of current utxos, not all of history" view
< gmaxwell>
"pruned wallet"
< instagibbs>
wumpus, either that or listunspent takes forever :(
< cfields>
phantomcircuit: eh? They for sure used to.
< phantomcircuit>
to operate in the fast safe mode it needs a separate write ahead log file
< wumpus>
gmaxwell: right
< luke-jr>
well, at least we don't need to keep the history in memory
< wumpus>
luke-jr: indeed
< cfields>
phantomcircuit: oh, I was talking about source file, not the database format.
< phantomcircuit>
cfields, you cant have a single file but it's amazingly slow
< phantomcircuit>
oh
< phantomcircuit>
yes it does have that but like
< phantomcircuit>
meh
< wumpus>
that's where something like sqlite would be, more or less, useful, I like how clightning uses it
< gmaxwell>
going back to the prior point. ... if the history isn't in memory, then the database storing the wallet needs to be structured in a way that suports that
< cfields>
phantomcircuit: that makes integration into our build a no-brainer. That's a signifacant feature imo.
< phantomcircuit>
gmaxwell, and needs to be quite fast actually
< wumpus>
integrating sqlite into a project is trivial, indeed can be done as a single .cpp file if that's desirable
< jamesob>
if we're thinking longterm (esp. about not loading everything into memory simultaneously), I think it makes sense to come up with a normalized, relational schema for the wallet and use sqlite. shouldn't be hard to come up with something non-controversial (famous last words)
< promag>
any reason to not consider postgres for instance?
< wumpus>
AHHHH
< sipa>
gmaxwell: i don't think the choice of container format and the choice of database layout need to be made at the same time
< sipa>
promag: god why
< jamesob>
wat
< luke-jr>
promag: uh, lots?
< jonasschnelli>
Oracle?
< cfields>
haha
< wumpus>
jonasschnelli: :-) <3
< sipa>
Oracle BDB?
< promag>
luke-jr: name one
< jonasschnelli>
I think however we proceed (sqlite, logdb, etc.), factoring out BDB in a nice layered way will be require (even helps if we keep BDB forever)
< luke-jr>
I mean, if we're using sqlite, the queries could be compatible with multiple backends, but expecting regular users to set up Postgres is crazy..
< jonasschnelli>
I hope someone picks that up
< sipa>
promag: let's do that outside this meeting
< cfields>
this might work better in terms of concrete proposals rather than rounds of "how about xyz?"
< jonasschnelli>
Also BDB is a compile pitfall
< promag>
sure
< wumpus>
cfields: good point
< wumpus>
'what about mongodb?' :')
< wumpus>
any other topics? 4 minutes left
< cfields>
haha
< provoostenator>
We should just store it on a blockchain.
< luke-jr>
provoostenator: it would be nice if it was possible to commit to it in such a way
< luke-jr>
eg, if you could get a historical hash of the wallet state for commitments
< wumpus>
luke-jr: right, optional support for a large-scale DBM like postgres would be useful for really big users, but that's really a long term goal I suppose, if at all
< gmaxwell>
I'd prefer it if just ban anyone that ever directly uses the name of any database system from the channel.
< jonasschnelli>
but leveldb!
< wumpus>
except oracle, of course *ducks*
< gmaxwell>
sipa: "container" is basically my point, if we're just using it as a "container" a simple log would be a lot better.
< jonasschnelli>
heh
< wumpus>
#endmeering
< wumpus>
#endmeeting
< lightningbot>
Meeting ended Thu Sep 27 19:58:55 2018 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< sipa>
gmaxwell: yes, but sqlite is essentially two layers; a well-tested safe updatable blob storing mechanism, and then a database layer on top
< gmaxwell>
sipa: if you just dump transactions/keys into a container and then have to load them into memory and index them thats fine.. if you want to avoid having to do that, you can't just use the storage as a container, it needs to be used as a database.
< jonasschnelli>
The append-only easyness got a bit lost since we added abandontransaction
< wumpus>
jonasschnelli: yes
< jonasschnelli>
Its still possible but comes at high disk-mem cost
< gmaxwell>
jonasschnelli: why?
< promag>
gmaxwell: how about aws s3? o/
< gmaxwell>
jonasschnelli: abandon transaction just logs the txn as abandoned.
< luke-jr>
^
< sipa>
gmaxwell: by switching to sqlite as a container and only using the low level format, you get pretty much what we need (apart from the extra code for the database stuff), but you also get the ability to later switch the db schema
< gmaxwell>
So we could start using sqllite as a container, but then later actually have a schema, thats a thing that could be done, but really its just two incompatible wallet formats.
< jamesob>
ripping out bdb in favor of sqlite sounds fun... I might take a crack if no one gets around to it in the next few months. sounds like a good christmas break project :)
< promag>
gmaxwell: and support log compaction?
< jonasschnelli>
gmaxwell: yeah.. but if you abandon a 10k bytes transaction you have 10k unused bytes (where only 32bytes change)
< gmaxwell>
promag: why would that be needed?
< jonasschnelli>
log compactation is of course possible
< wumpus>
jamesob: it's not too hard to replace the database, I replaced berkeleydb with leveldb once in a day or so
< gmaxwell>
jonasschnelli: yes so? no one cares about 10k extra data in their wallet, if they did any of these database based solutions would be unacceptable from the getgo as they're all fairly space inefficent.
< phantomcircuit>
jonasschnelli, nobody is using abandon transaction because of 10kb of disk space being used lol
< wumpus>
jamesob: (that's in a very naive way, though, proper schemas will be more work)
< sipa>
i don't see how abandomtransaction changed anything more than a transaction confirming does
< jamesob>
wumpus: what's the tricky part? providing a migration path?
< sipa>
it's just an update "this tx is now abandoned"
< instagibbs>
so, what does exit status "1" mean in the functional test suite, when shutting down a node?
< jonasschnelli>
wumpus, jamesob: I think the hard part of the refactoring is to make two database work (which is ultimatively required for a conversion tool)
< promag>
why not? for systems that use bitcoin wallet in a couple of months it can be pretty large
< sipa>
jonasschnelli: i think that's the easiest part
< phantomcircuit>
what does -6 mean?
< wumpus>
jamesob: the "don't load everything into memory and use proper indexes" part, and yes ,migration is also an issue
< sipa>
jonasschnelli: agreeing on a new database schema will be impossible, however :)
< achow101>
instagibbs: 1 is EXIT_FAILURE
< achow101>
phantomcircuit: where do you get -6?
< luke-jr>
append a "drop the tx" command, and punch a hole where the original data was
< jonasschnelli>
gmaxwell: Yes. I just meant the easyness (or the effectiviness) got a bit lost with state changes on transactions like abandontx
< phantomcircuit>
jonasschnelli, that is for sure the principle issue, supporting two formats at the same time is difficult
< sipa>
jonasschnelli: i really don't see where that comes from
< luke-jr>
then it can be saved append-only in backups, and also frees the disk space (potentially)
< sipa>
jonasschnelli: a tx confirming is equally a state change
< instagibbs>
achow101, ah ok, i shall hunt for this error
< gmaxwell>
sipa: in any case, my point is that just swapping the backing store doesn't seem to produce any real benefit, but it creates incompatiblity. I might be missing it, but it sounds like replacing a black box with a more trendy black box. Actually making use of it to to keep stuff out of memory would potentially be a big gain, but that isn't the discussion, and if it were done would create
< gmaxwell>
effectively another new wallet format.
< phantomcircuit>
test_framework.test_node.FailedToStartError: [node 0] bitcoind exited with status -6 during initialization
< jamesob>
wumpus: as you say earlier, though, we can gradually build in better use of the database; I think it'd be nice just to shed the bdb dependency asap
< phantomcircuit>
have literally no idea what would cause that
< sipa>
gmaxwell: well the low-level part of sqlite looks like a well-tested implementation of what we'd want actually
< gmaxwell>
jonasschnelli: I'm still not seeing it, we always made changes to tx.. adding labels, or as sipa notes, confirming them.. spending their outputs, etc.
< sipa>
gmaxwell: of course, needs investigation
< luke-jr>
wumpus: I don't have the exact APIs memorised, but probably something like that
< luke-jr>
wumpus: so long as the loader ignores null bytes, it should be fairly trivial
< wumpus>
luke-jr: I remembered because it sounds so senselessly aggressive, it replaces a page in a file with a hole (all-zeros) page
< luke-jr>
and frees the space used by it
< jonasschnelli>
gmaxwell: I think confirmation is stateless from the wallet perspective (only in conjunction with the blockchain/header state)... but yes. Its non crucial.
< luke-jr>
aggressive in what sense?
< gmaxwell>
does windows do sparse files?
< wumpus>
luke-jr: I mean, as in violent
< luke-jr>
gmaxwell: yes, but not sure if it has an easy punch-hole
< sipa>
jonasschnelli: ... no, we add the block hash that contains the tx to the CWalletTx record
< wumpus>
gmaxwell: modern versions of windows do
< achow101>
phantomcircuit: I think maybe that's a SIGABRT
< jonasschnelli>
uh. yes. Right. There is then at least one update from mempool to in-block,... right (and eventually some reorg writes)
< luke-jr>
we don't support versions that don't I think :p
< wumpus>
so does MacOS support sparse files?
< gmaxwell>
achow101: how would sigabrt show up that way? signals are normally very high exit codes
< sipa>
promag: short answer to why not postgres; we don't need a multi-user networked database, and we don't need sql
< sipa>
we need a file on disk
< sipa>
that's safe to interact with and move around
< wumpus>
requireing postgres would be really evil, though not as evil as requiring oracledb
< wumpus>
besides being unnecessary it has lots of extra setup steps to get started in the first place
< achow101>
gmaxwell: exit codes are signed, so anything above 128 would be negative, right?
< gmaxwell>
exit codes are a char?
< sipa>
i think so yes
< wumpus>
on most UNIX, yes
< gmaxwell>
ha, if I knew that it was long since lost!
< provoostenator>
There are of course some other cryptocurrency projects dealing with lots of I/O. Anything useful we can use e.g. from libbitcoin, or LMDB that Monero uses?
< gmaxwell>
okay well SIGABRT would mean he was hitting an assertion most likely.
< wumpus>
and negative values are generally only used when the process was killed with a signal
< provoostenator>
(not that the wallet is I/O heavy, so maybe that's not the thing to worry about)
< gmaxwell>
provoostenator: I have detected you mentioned a database. You are a bad person
< jamesob>
would it be an acceptable user experience for us to completely strip bdb out in some major release, provide an upgrade tool, and throw an error if users try to start bitcoin with bdb-format wallets?
< gmaxwell>
jamesob: I had assumed that we would eventually do exactly that.
< wumpus>
provoostenator: monero doesn't use LMDB for the wallet ,as far as I know, only for the block chain
< sipa>
provoostenator: we are not talking about a database. we're talking about the wallet
< wumpus>
monero has some really simple format for the "simplewallet" IIRC
< achow101>
so I think that means -6 becomes just signal number 6, which is sigabrt
< wumpus>
achow101: yes
< sipa>
provoostenator: there are very different trade-offs relevant if we're talking about the UTXO set etc, but that's not the topic now
< jamesob>
gmaxwell: makes sense - didn't know if anyone was intent on having some kind of runtime migration
< promag>
sipa: I understand, just playing devil's advocate
< provoostenator>
I just think it's suboptimal to pull in a whole new dependency _only_ for the wallet.
< provoostenator>
So might as well think ahead a bit, unless we're sure we never want to move away from leveldb for everything else.
< gmaxwell>
have you all considered rewritting the software in javascript? it has webscale. it's very popular now, I hear nodecoin uses it.
< gmaxwell>
provoostenator: the wallets needs are almost but not entirely unlike the needs of the system state.
< gmaxwell>
It's basically an entirely different task, with very different requirements.
< wumpus>
yes
< wumpus>
and sqlite would be a trivial dependency at least
< gmaxwell>
(as an aside, people have also tested lmdb and sqllite for what we use leveldb fore. lmdb seemed to be about the same or slightly worse, depending on hardware in use... and the report was on sqllite that no one had enough patience to let it finish syncing after the Nth day.. :) )
< jamesob>
leveldb strikes me as totally fine for the utxo set and uprooting it seems very dangerous (not sure if I'm attacking a strawman here)
< gmaxwell>
s/fore/for/
< provoostenator>
"SQLite does not use Git" that's a great start...
< provoostenator>
(but nothing a checksum can't handle)
< sipa>
heh
< sipa>
what does that have to do with anything
< luke-jr>
SQLite isn't consensus-critical.
< luke-jr>
just pkgconfig sqlite3, done
< wumpus>
yes
< gmaxwell>
phantomcircuit was saying above that the safe mode of sqllite requires a seperate log file. :(
< gmaxwell>
if so, thats a bummer.
< BlueMatt>
wait, huh, what happened to append-only wallet?
< wumpus>
anyhow I don't think we're ever going to agree on this
< luke-jr>
gmaxwell: do we need the safe mode?
< BlueMatt>
seems like a really, really bad idea to move from a design like that to some standard db env
< sipa>
BlueMatt: great idea, just discussing alternaives :)
< BlueMatt>
heh, we havent had enough fun with bdb?
< sipa>
sqlite is nothing like bdb :)
< wumpus>
at least sqlite supports proper indexing and queries...
< sipa>
(but if what gmaxwell says is true, that makes it mch less attractive)
< BlueMatt>
true, but still
< wumpus>
which is great for finding transactions matching a certain criterion, for example
< jamesob>
wumpus: isn't the sqlite logfile just some optional performance tweak?
< gmaxwell>
wumpus: unless our way of using it is just throwing in a lot of binary blobs like we pretty much use bdb. :P
< wumpus>
with keyvalue databses all that indexing has to be implemented manually
< wumpus>
gmaxwell: as I said, I like how clightning uses it
< wumpus>
it's really sensible and well-designed
< provoostenator>
Problem solved then, a SQL lite binary blob inside bdb.
< wumpus>
somehow I really dislike how we ended up with a wallet embeded in our project and being unable to move away from it
< gmaxwell>
wumpus: to be clear, I'm +1 on using it with a schema, -1 on using it in a blobby way, neutral on blobby in general, but if we were going to stay blobby a simple thing that just writes out a change log for in memory state would probably be better (more reliable, simpler, etc.).
< wumpus>
I wish it was just some external thing, but that's too much to wish for at this point
< jamesob>
wumpus: huge agree
< wumpus>
everyone is always going to disagree how to move on from here, it's like a really bad local maximum in evolution
< gmaxwell>
if it were, I think bitcoin would be nearly dead... there would be very little reason for people to bother even trying to start a node.
< gmaxwell>
Altcoins where node operations are totally distinct from using a wallet basically just don't haver users using nodes.
< wumpus>
the node could still be shipped with some wallet
< luke-jr>
on that point, we do need to make it easier to pair nodes with other wallets
< luke-jr>
the hard part is the port forwarding stuff :/
< jamesob>
luke-jr: in your opinion is that an issue of making the rpc interface more granular/complete or something else?
< wumpus>
or like monero simply ship with the most simple wallet possible, with no ambitions of anything more
< gmaxwell>
luke-jr: because there is so much development surplus that it would be good to spend it on facilitating duplication of efforts that aren't even being duplicated yet. :P
< luke-jr>
jamesob: RPC interface isn't really used much for other wallets
< jamesob>
anecdotally, if someone just wants to transact in bitcoin they often just go to electrum/trezor/bread/etc.
< luke-jr>
gmaxwell: people are using other wallets whether we facilitate it or not
< jamesob>
I think few people who _just_ want a wallet go to the bother of syncing bitcoind
< gmaxwell>
luke-jr: I don't see how that connects to your comment.
< jamesob>
though I am sympathetic to that reasoning... wish there was a way of quantifying it
< provoostenator>
jamesob: that's probably because they don't understand the privacy tradeoff
< gmaxwell>
luke-jr: how someing using a hosted web wallet doesn't really have anything to do with bitcoind.
< luke-jr>
gmaxwell: I mean electrum/trezor/bread/etc
< wumpus>
in any case, this comes back every time
< jamesob>
of course - but even a user like me doesn't actually use the core wallet because it doesn't, e.g., support hardware devices
< wumpus>
the same discussions since 2012
< luke-jr>
ideally, they could just scan a QR code from Core, and have it use their own node
< luke-jr>
the trouble is getting past firewalls
< gmaxwell>
luke-jr: and what does that hae to do with bitcoind?
< jamesob>
provoostenator: oops ^
< gmaxwell>
It's a lot more than that.
< wumpus>
even the "let's use a SQL database, oh why not postgres" I'm fairly sure we alrady had back in 2012
< gmaxwell>
Trezor, for example, is effectively a hosted web wallet, though with signing using your hardware fob.
< jamesob>
gmaxwell: right - which I hate but nonetheless use
< gmaxwell>
I am really frustrated by the fact that any time anyone mentions storing anything on disk there is a tiresome discussion of the trendy database things. Someone above joked mongo, but in the 2013 (IIRC) flavor of these rehashings, it would actually get suggested seriously.
< provoostenator>
And because people are physically holding it, they're less aware of that problem.
< provoostenator>
Everyone always saying "don't use web wallets, use a hardware wallet"..
< wumpus>
the other wallets aren't better in every regard, that's for sure, the core wallet is pretty good, I'm just tired of some kinds of discussions
< gmaxwell>
jamesob: my point in responding to luke there, that no amount of changed to bitcoind is going to make the 'trezor wallet' work with it.
< gmaxwell>
(getting support for hardware wallets, OTOH, would enable people to not use the trezor wallet, and thats another matter)
< booyah>
gmaxwell: as for trezor, well with electrum it's a signing engine, not webwallet at all
< luke-jr>
gmaxwell: Electrum already supports Trezors (altohugh to support Electrum, we need a lot more than just port forwarding)
< gmaxwell>
booyah: right, and PSBT was a first step in getting thins so we could use devices like that.
< wumpus>
I'm happy that we managed to depreacte accounts at least, to simplify things, that was also an issue running almost since satoshi left
< booyah>
gmaxwell: freaking finally ;)
< wumpus>
yes hardware wallet support is something worth working on!
< jamesob>
gmaxwell provoostenator: I think even now you can do some bitcoind -> electrum personal server -> electrum -> trezor thing, but I haven't tried it because it sounds like a ton of work
< wumpus>
that would be actually nice
< wumpus>
in contrast to the eternal database discussion
< gmaxwell>
booyah: unfortunately its hard going because the vendors of these things have not been helping, instead mostly just making their own custom stuff for time to market reasons.
< jonasschnelli>
jamesob: its working and setup is easy
< booyah>
gmaxwell: satoshi labs was not helpful?
< provoostenator>
I've tried the electrum personal server approach. Works reasonably well. I've also tried the achow101 approach, which works great once you're used to it, but too manual for now.
< jamesob>
jonasschnelli: oh cool, I'll try that then.
< jonasschnelli>
jamesob: Bitbox has a native desktop app that can connect to EPS <-> Core
< wumpus>
(haven't tried it yet though)
< gmaxwell>
jamesob: and it involves using electrum... which has plenty of its own downsides.
< jamesob>
right
< gmaxwell>
I'm certantly not faulting the HW wallet vendors for focusing on time to market, it's just what it is.
< provoostenator>
I have some WIP stuff where I try to turn achow101's HWI scripts into an RPC server, which could eventually be a universal "sign something" service, not necessarily restricted to hardware wallets.
< wumpus>
me neither, though they have been kind of shooting themselves in the foot by not thinking about a common standard, but maybe that's something taht can only arise after experimentation... happy about PSBT
< provoostenator>
In that case all Core needs to do is make a bunch of RPC calls to a user configured URI (or a file socket maybe).
< gmaxwell>
provoostenator: I'd suggested previously that we exeute a configured command and shove crap at it over stdin.
< provoostenator>
And then it's up to hardware wallet makers to listen for those calls.
< gmaxwell>
(I'm less of a fan of URIs because of the security problems, e.g. when some local JS starts connecting to it...)
< sipa>
provoostenator: or PSBT and invoke a binary to sign it
< provoostenator>
Executing commands sounds scary too
< gmaxwell>
provoostenator: hows it scary? it's configured explicitly.
< provoostenator>
True, malware gonna malware, no matter how you do it.
< gmaxwell>
provoostenator: the URI thing in general has a challenge that browsers can make connections to things that listen to the network. There have been a LOT of thefts in eth land, due to 'wallets' that take commands from local sockets.
< clarkmoody>
Apply UNIX philosophy to wallet: text is the universal interface
< gmaxwell>
provoostenator: I'm not talking about malware running unconstrained on your host, I'm talking about just stuff in your webbrowser.
< wumpus>
in any case I think it makes sense to, in meetings, discuss the things each participant plans on working on, not wild far future plans
< provoostenator>
gmaxwell: c-lightning uses file sockets which at least prevent hat.
< luke-jr>
provoostenator: on Windows? O.o
< wumpus>
otherwise we end up discussing the same things from 2012 every time
< provoostenator>
luke-jr: there's that...
< gmaxwell>
provoostenator: okay, thats essentially the same security model as what I was talking about. didn't know you could do that on windows.
< provoostenator>
I wish browsers didn't let websites talk to any URL in the universe, but it seems they do.
< gmaxwell>
wumpus: The way it seems to go though is.. "I plan on working on something something something database" "Excuse me sir, but have you considered Monodb? It has webscale" then goto 2012;
< gmaxwell>
maybe I just need to write a webpage to link to, with links to every other time we've gone through this loop.
< wumpus>
windows also has various local RPC methods
< wumpus>
gmaxwell: agree, there seems to be a lack of memory in these kind of things, although it seems to slightly improve, back then we had the literal same discussion within weeks of each other with new people
< provoostenator>
gmaxwell: a nice use case for RPC calls would be a web based multisig service.
< gmaxwell>
provoostenator: in any case the model of "you tell your wallet to talk to some other local program that has the task of signing things" sounds great to me. It also could be used "talk to some other local program that has the task of collecting other signatures for multisig" easily.
< wumpus>
(I think recent windows 10 even has UNIX sockets)
< gmaxwell>
provoostenator: I'd say jinx except this was all pointed out last time I was lobbying in that direction. ;)
< provoostenator>
Yes, it just means those services have to ship an executable, but that trade-off might be worth it.
< wumpus>
but in any case, if you want a *secure* RPC method, you will have to determine for every OS what is the best choice
< gmaxwell>
provoostenator: "Driver", it might well be the case that eventually we ship drivers for common cases ourselves.
< jamesob>
gmaxwell: you know we're trending upwards because no one *seriously* recommended mongodb this time around
< wumpus>
just deciding for 'UNIX sockets' is very UNIX-centric by default
< gmaxwell>
jamesob: I think thats just because the world at large no longer thinks mongodb is the new hotness. :)
< jamesob>
heh, true
< provoostenator>
gmaxwell: especially if someone comes up with a proper USB protocol standard for talking with devices, so we wouldn't have to merge device specific stuff?
< gmaxwell>
wumpus: well thats why I liked stdio, as it does exist everwhere. :P
< wumpus>
mongodb suggestion was a reductio ad absurdum by me this time
< gmaxwell>
provoostenator: right, but it's not just the usb, there are UI considerations. e.g. many hardware wallets have some pin entry on the host computer or similar.
< gmaxwell>
Basically the need to control enough of the host probably contributes to making your hardware wallet supported by creating a whole web wallet for it.
< wumpus>
gmaxwell: yes, communicating through a pipe over stdin/stdout seems to be something every OS has
< provoostenator>
At least Ledger and Trezor these days do pin entry on device. So in that case it's just a matter of waiting.
< wumpus>
(though on windows it's different; stdio only exists for console applications, not for winmain ones)
< jamesob>
the Trezor One requires on-host software for pin entry
< wumpus>
it's such a crazy labyrinth people have created, nothing is straightforward
< gmaxwell>
jamesob: okay that one was the one that I was thinking about.
< provoostenator>
There's a bunch of ports that some browsers ban, but it's quite narrow, and those are already used by the system (that's why they're blocked). I also don't think there's a standard.
< provoostenator>
You can add authentication, maybe some tofu mechanism. But browers are extremely powerful these days.