19:00:07 <meshcollider> #startmeeting
19:00:07 <lightningbot> Meeting started Fri Oct  9 19:00:07 2020 UTC.  The chair is meshcollider. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:07 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:00:14 <achow101> hi\
19:00:24 <meshcollider> #bitcoin-core-dev Wallet Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator aj Chris_Stewart_5 dongcarl gwillen jamesob ken281221 ryanofsky gleb moneyball ariard digi_james amiti fjahr
19:00:24 <meshcollider> jeremyrubin emilengler jonatack hebasto jb55 kvaciral ariard digi_james amiti fjahr jeremyrubin lightlike emilengler jonatack hebasto jb55 elichai2
19:00:31 <jonatack> hi
19:00:49 <hebasto> hi
19:00:55 <meshcollider> 2:09 AM <achow101> #proposedwalletmeetingtopic: wallet.dat vs wallet.sqlite
19:01:26 <meshcollider> #topic wallet.dat vs wallet.sqlite
19:01:50 <achow101> for sqlite wallets, there's been an ongoing question of whether the sqlite wallet files should be named wallet.dat or wallet.sqlite
19:02:52 <hebasto> what are pros and cons of each approach?
19:03:12 <achow101> the PR currently implements wallet.dat. ryanofsky has been arguing for wallet.sqlite in his review comments
19:03:19 <achow101> i wanted to hear what everyone else thinks
19:03:42 <Murch> hello
19:03:47 <hebasto> I've just started to review that pr, so have no opinion
19:03:50 <achow101> for wallet.dat, the arguments are to maintain backwards compatibility with external documentation and tooling, as well as not causing a problem with a specific downgrade scenario
19:04:44 <meshcollider> Yeah calling it wallet.dat has the advantage that automatic backup scripts, etc. will continue working fine, and also that users are already conditioned to protecting wallet.dat files
19:04:56 <achow101> for wallet.sqlite, it's a clearer naming convention, follows sqlite naming convention, and can't be confused with bdb
19:05:38 <jonatack> hm, good arguments for both
19:05:41 <achow101> wallet.sqlite also avoids a different set of compatibility prolems
19:05:47 <sipa> yeah, i'm very much in the middle
19:06:08 <sipa> some of the naming conventions and expectations around them were already broken when we moved to per-wallet directories
19:06:20 <sipa> and i don't recall that causing many issues for users
19:06:35 <achow101> relevant commens. for wallet.dat: https://github.com/bitcoin/bitcoin/pull/19077#issuecomment-705180018 for wallet.sqlite: https://github.com/bitcoin/bitcoin/pull/19077#pullrequestreview-504980287
19:06:43 <sipa> though, the specific "wallet.dat must be protected with your life" filename convention remained
19:06:50 <luke-jr> meshcollider: it is already wrong and risks corruption to copy wallet.dat directly
19:06:57 <fjahr> hi
19:07:00 <hebasto> if users adopted per-wallet dir, we could expect such adoption for .sqlite
19:07:03 <sipa> luke-jr: it doesn't if you do it while bitcoind is shut down
19:07:14 <meshcollider> luke-jr: that doesn't stop users doing it
19:07:24 <luke-jr> meshcollider: breaking such scripts would be an advantage to renaming
19:07:37 <sipa> luke-jr: i couldn't disagree more with that
19:07:39 <luke-jr> a possible issue is restoring though
19:07:56 <achow101> luke-jr: but breaking such scripts would probably result in backups not being made, which is dangerous
19:08:11 <luke-jr> achow101: most likely would result in errors instead of possibly-corrupt backups
19:08:17 <achow101> I would be surprised if said scripts failed in a way that was obvious to the person running it
19:08:21 <luke-jr> O.o
19:08:26 <sipa> luke-jr: i agree that we should discourage bad practice, but (a) not by making decisions that can actually cause people to lose money and (b) i disagree this is unsupported - it's only supported when bitcoind is not running though
19:08:28 <meshcollider> And there are also other tools I imagine, not just backup scripts, which look for wallet.dat by default
19:08:34 <luke-jr> if a cronjob fails, typically you get an email
19:09:00 <luke-jr> sipa: at least some versions would reuslt in corruption even if bitcoind exited cleanly
19:09:07 <achow101> luke-jr: fwiw I have a system backup cronjob and I don't know when/if it fails until I check the logs, and that happens maybe once every 6 months
19:09:09 <sipa> how so?
19:09:17 <luke-jr> sipa: we used to not flush/close the db
19:09:32 <luke-jr> achow101: you should fix that :p
19:09:47 <sipa> luke-jr: i believe that was very briefly the case, in ancient times
19:09:50 <achow101> luke-jr: right, but that's an example of a backup script failing and the user not knowing
19:10:05 <luke-jr> I suppose people doing backups wrong, are also likely to do error notifications wrong
19:10:33 <sipa> people will do lots of things wrong
19:10:43 <sipa> doesn't mean we shouldn't do a best effort to avoid them losing money
19:10:55 <luke-jr> but wallet.dat are currently in a dedicated directory
19:11:00 <luke-jr> there's no need for that for sqlite, right?
19:11:10 <sipa> that's a good question
19:11:34 <achow101> luke-jr: yes, but I think it would be more confusing to users if we stopped doing that
19:11:42 <sipa> hmm
19:11:49 <jonatack> modulo the risk of users losing money if renamed, a risk i don't feel competent to evaluate, i tend to agree with ryanofsky's arguments
19:12:09 <sipa> to me, that'd be one of the advantages of sqlite... not needing a directory for every wallet anymore
19:12:51 <luke-jr> btw, even if it's wallet.sqlite, it's not like we're renaming without the user knowing
19:13:15 <luke-jr> wouldn't you expect anyone setting up a backup script to check that it works when they create the wallet, at least once? :P
19:13:20 <achow101> luke-jr: not necessarily. they'd need to read the release notes, and who the hell does that?
19:13:47 <luke-jr> achow101: you're seriously suggesting automatically transforming BDB to sqlite?
19:14:04 <luke-jr> without user interaction?
19:14:15 <achow101> luke-jr: there's no transformation
19:14:31 <achow101> what I mean is that sqlite would be default for descriptor wallets, but the only way you would know that is to read the release notes
19:14:32 <luke-jr> ok, so wallet.dat would remain wallet.dat even if new wallets are wallet.sqlite…
19:14:51 <achow101> existing wallets are unaffected
19:15:17 <luke-jr> so the only way someone should lose data is if they never check for a successful backup ever..
19:15:31 <achow101> sipa: I suppose that getting rid of the wallet directory thing would solve both of these problems
19:15:33 <luke-jr> or  maybe are backing up numerous wallets and expect newly created ones automatically included
19:16:57 <achow101> luke-jr: when we get around to implementing bdb to sqlite migration, there could be problems there with the rename
19:17:18 <luke-jr> achow101: but we get the chance to tell users when they opt into it
19:17:23 <achow101> true
19:17:46 <luke-jr> a reason not to rename: acting on file extensions has been kindof deprecated for a long time?
19:18:46 <achow101> there's also the problems with restoring, and that one downgrade case where a new wallet.dat is made
19:19:43 <meshcollider> IMO we should get rid of individual directories for sqlite, I don't think that would be confusing
19:21:29 <achow101> meshcollider: that still has the backup and restore problems, although not the downgrade one if we name the file as the wallet name
19:26:25 <achow101> any other comments on this topic?
19:27:13 <meshcollider> Did this help make a decision ;)
19:27:28 <achow101> not really
19:27:33 <fjahr> I'm undecided as well, sorry
19:28:14 <achow101> I'll experiment with a no wallet directory approach and see how big the diff is
19:28:25 <meshcollider> Yeah that sounds good
19:28:27 <meshcollider> Any other topics then?
19:29:56 <jonatack> fjahr: at some point, sometime, we should maybe discuss #18418
19:29:58 <gribble> https://github.com/bitcoin/bitcoin/issues/18418 | wallet: Increase OUTPUT_GROUP_MAX_ENTRIES to 100 by fjahr · Pull Request #18418 · bitcoin/bitcoin · GitHub
19:30:15 <jonatack> perhaps Murch can look at it
19:30:43 <jonatack> (just a thought, no need to duscuss now)
19:30:55 <fjahr> Yeah, thanks, I guess at the moment nobody has time but maybe in 2 weeks
19:32:30 <meshcollider> Ok let's discuss it next time then :)
19:32:38 <meshcollider> #endmeeting