19:00:23 <achow101> #startmeeting 
19:00:23 <core-meetingbot> Meeting started Fri Jan 27 19:00:23 2023 UTC.  The chair is achow101. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
19:00:23 <core-meetingbot> Available commands: action commands idea info link nick
19:00:29 <achow101> #bitcoin -core-dev Wallet Meeting: achow101 _aj_ amiti ariard BlueMatt cfields Chris_Stewart_5 darosior digi_james dongcarl elichai2 emilengler fanquake fjahr furszy gleb glozow gmaxwell gwillen hebasto instagibbs jamesob jarolrod jb55 jeremyrubin jl2012 jnewbery jonasschnelli jonatack josibake jtimon kallewoof kanzure kvaciral laanwj larryruane lightlike luke-jr maaku marcofalke meshcollider michagogo moneyball morcos Murch nehan NicolasDorier
19:00:29 <achow101> paveljanik petertodd phantomcircuit promag provoostenator ryanofsky sdaftuar S3RK sipa vasild
19:00:41 <S3RK> hi
19:00:59 <furszy> hi
19:01:06 <achow101> There's one preproposed wallet meeting topic: hd key rotation vs new wallet (achow101). Anything else to add to the list for today?
19:01:35 <S3RK> If we have time, I'd be curious to discuss apporach for #22693
19:01:39 <gribble> https://github.com/bitcoin/bitcoin/issues/22693 | RPC/Wallet: Add "use_txids" to output of getaddressinfo by luke-jr · Pull Request #22693 · bitcoin/bitcoin · GitHub
19:02:22 <achow101> #topic hd key rotation vs new wallet (achow101)
19:02:22 <core-meetingbot> topic: hd key rotation vs new wallet (achow101)
19:03:19 <achow101> in #26728, S3RK brought up the question that there isn't a well defined use case for having key rotation, as all of the cases that we've brought up are sufficiently handled by making a new wallet
19:03:22 <gribble> https://github.com/bitcoin/bitcoin/issues/26728 | wallet: Have the wallet store the key for automatically generated descriptors by achow101 · Pull Request #26728 · bitcoin/bitcoin · GitHub
19:04:20 <achow101> if that's the case, then we could simply the implementation to not allow key rotation and just instruct people to use new wallets
19:05:20 <achow101> I've had conversations with sipa and luke-jr about this in the past, but I think everything that was discussed could also just work with using a new wallet?
19:06:26 <S3RK> yes, I think it's preferrable to have a simpler implementation if all the use cases are covered by creating a new wallet
19:06:39 <S3RK> One thing I wonder is key rotation during encryption
19:07:14 <sipa> a new wallet means losing tx history?
19:08:18 <achow101> iirc the main point was that a user would want to rotate keys when if they think their keys were compromised, but then it follows that making a new wallet is better than rotating as there is less likelihood of reusing potentially compromised keys afterwards
19:08:37 <achow101> sipa: with multiwallet, does that really matter? The old wallet can still be loaded and history viewed.
19:09:17 <sipa> it also requires moving all the funds over
19:09:29 <S3RK> I guess I'd like to understand first when people feel like they need to rotate HD key
19:09:40 <provoostenator> hi
19:09:40 <sipa> i think periodically rotating keys makes sense as a security safeguard, even without any known compromised keys
19:09:49 <sipa> but if you it involves losing history and moving all funds over, that's a lot less appealing
19:11:16 <S3RK> it might be a stupid question, but safeguard against leaked keys? IIUC you're saying there is some risk of the keys being compromised so I'd like to rotate them, but the risk is not high enough to warrant extra caution to avoid mixing utxos unintentionally
19:11:16 <achow101> but what kind of safeguard does that provide? istm you would do it because maybe something was compromised, but if that's the case, wouldn't you want to move all of the funds to the new key(s)
19:11:39 <sipa> hmm, that's a fair point
19:11:44 <S3RK> yes
19:11:50 <provoostenator> The wallet could encourage such moving in other ways of course.
19:12:08 <S3RK> we have sendall now to facilitate migration
19:12:25 <provoostenator> But a fresh wallet makes that more intuitive.
19:12:35 <Murch1> Hi
19:12:39 <sipa> another downside is that if somehow someone still sends coins to the old wallet after rotation, you may not notice if it's a separate wallet, and you don't keep all the old ones loaded and watched
19:12:43 <S3RK> :waves:
19:13:34 <provoostenator> And the old wallet might fall behind the prune window too if it's not loaded, making it very painful to access unexpected payments to it.
19:13:55 <provoostenator> (though that's fixable)
19:14:27 <achow101> with multiwallet and the settings.json stuff, it's not that hard to keep multiple wallets loaded automatically
19:14:42 <Murch1> Would it be possible to monitor multiple wallets, but to only have one active in the sense that you are using its UTXO Pool to send and its addresses to receive?
19:15:03 <sipa> a wallet can contain multiple SPKMs
19:15:13 <sipa> so you can do all that already, within a single wallet
19:15:14 <achow101> Murch1: yes, multiwallet
19:15:38 <S3RK> the problem I see with one wallet is that you don't have any separation between "old" spkms and "new" ones
19:15:48 <sipa> well multiwallet is something else still, as API wise those are completely separated
19:15:50 <S3RK> if you rotate more than one time it can easily become a mess
19:15:58 <sipa> S3RK: active and inactive ones?
19:16:13 <achow101> with rotating by using a new wallet, it would become obvious when something was sent to the old one
19:16:22 <S3RK> you can still spend from inactive spkms
19:16:27 <achow101> as the tx shows in the old wallet's history and not the new one's
19:16:40 <sipa> right
19:16:50 <achow101> even with inactive spkms, we don't distinguish in the tx history unless you go look at the address and then getaddressinfo it and compare the descriptors
19:16:58 <sipa> and in case you rotate because of compromise, you probably actually do want to see payments to the old one as separate
19:17:09 <sipa> "crap, i need to move more funds over now"
19:17:19 <S3RK> sorry :D
19:17:53 <sipa> having some mode to mark a wallet as compromised, so that all funds sent to it are automatically transferred elsewhere may be useful too...
19:19:08 <S3RK> but are there cases when people want to rotate without keys being compromised?
19:19:31 <Murch1> Sure, e.g. if you‘re still on a legacy wallet and want to move to a descriptor wallet maybe?
19:19:56 <achow101> Murch1: strictly speaking, it doesn't have to. but it's a lot easier to implement migration in that way
19:20:32 <sipa> S3RK: Well, historically, pre-HD wallets had a "feature" that they get "unstolen" over time, as change outputs and new receives would over time end up in new keys that an old stolen backup did not contain.
19:20:57 <achow101> we rotate on encrypting, but considering the above conversation, that maybe doesn't make sense anymore?
19:21:12 <sipa> This discussion I think started from the question whether wallet migration is ever not strictly an upgrade, and one example is that if the migration only supports going to HD wallet, this "unstealing" effect that the user (very questionably, hypothetically) could be relying on, disappears.
19:22:04 <achow101> sipa: arguably it disappeared a long time ago when the default keypool was bumped up to 1000
19:22:25 <sipa> It's very questionable as it only works on pretty high-turnover wallets anyway, with unclear timelines, and only when an attacker gets access to private keys and then bides their time without actually stealing.
19:22:31 <S3RK> that sounds like an argument for non-HD wallets, which is separate from the question of whether HD key rotation within same wallet is a valid use case
19:23:01 <sipa> S3RK: I don't think so. Because the extent to which these non-HD wallet had that feature was really accidental, and unreliable.
19:23:12 <sipa> If we actually want it as a feature, key rotation is the proper solution.
19:24:02 <S3RK> hm... key rotation when exactly?
19:24:13 <sipa> periodically
19:24:19 <S3RK> sounds like non-HD to me :D
19:24:52 <achow101> that doesn't sound like a feature that even makes sense to rely on, even with periodic key rotation
19:24:56 <sipa> Hmm, no, not all. The point is that it'd need to be time or volume based or so, not transaction count based.
19:25:08 <achow101> you'd still have to have a high turnover wallet
19:25:30 <achow101> and coin selection that selects older coins
19:25:36 <Murch1> And then you need to make a new backup setup at the same time
19:25:58 <sipa> Anyway, I also see the point of view that key rotation in this sense isn't actually a desirable feature, even when done properly, and the only reasonable way to get "unstealing" protection is actually migrating to a new wallet entirely from time to time.
19:27:15 <achow101> also, we can add it in later if there is a genuine use case for it
19:27:47 <achow101> for #26728, I'll remove the key rotation handling stuff. we can always reintroduce it
19:27:49 <Murch1> achow101: We have `maxHeight` now ;)
19:27:50 <gribble> https://github.com/bitcoin/bitcoin/issues/26728 | wallet: Have the wallet store the key for automatically generated descriptors by achow101 · Pull Request #26728 · bitcoin/bitcoin · GitHub
19:28:19 <Murch1> Or minConf which is very similar
19:28:38 <S3RK> Murch1: I don't follow, how this is relevant?
19:28:40 <sipa> I don't think it's crazy to have a way to explicity rotate a wallet, which would involve re-generating all descriptor keys, possibly move all coins over, possibly make sends to the old ones automatically trigger a transfer, and then force a new backup.
19:28:46 <sipa> But that's a big feature.
19:29:29 <furszy> but that would periodically reveal all your balance in a large tx
19:29:45 <furszy> i mean, all your history to everyone
19:30:17 <sipa> Well, there are many ways the transferring could be done, but indeed - doing it in a privacy-preserving way would complicate it even further.
19:30:57 <furszy> yeah sure, make smaller changeless sends over time.
19:31:37 <achow101> just make a bunch of 1 in 1 out txs moving each utxo individually :)
19:32:10 <sipa> and send them all at the same time? ;)
19:32:30 <provoostenator> With random delays and fees.
19:33:00 <provoostenator> And via the new coinjoin implementation :-)
19:33:04 <achow101> #topic approaches for #22693 (S3RK)
19:33:04 <core-meetingbot> topic: approaches for #22693 (S3RK)
19:33:07 <gribble> https://github.com/bitcoin/bitcoin/issues/22693 | RPC/Wallet: Add "use_txids" to output of getaddressinfo by luke-jr · Pull Request #22693 · bitcoin/bitcoin · GitHub
19:33:08 <gribble> https://github.com/bitcoin/bitcoin/issues/22693 | RPC/Wallet: Add "use_txids" to output of getaddressinfo by luke-jr · Pull Request #22693 · bitcoin/bitcoin · GitHub
19:33:45 <S3RK> yep, so I'm not a huge fan of the current approach and I see another one suggested by furszy and yet another one by achow101
19:34:13 <achow101> that last commit is extremely bleh
19:34:50 <S3RK> we already have avoid reuse feature with bool stored in mem within address book
19:34:56 <S3RK> we can reuse/extend that
19:35:03 <Murch1> <S3RK> "Murch: I don't follow, how..." <- If you want to prioritize spending the coins in your old wallet, you set the `minConfs` to the time before your new wallet was created
19:35:16 <S3RK> Murch1: got it
19:35:27 <S3RK> or we can track use script as achow101 suggested
19:35:38 <furszy> have to admit that I don't remember what I commented there. Guess that I said to reuse the value that we are already storing in the addressbook
19:35:39 <achow101> S3RK: I generally don't like shoving these things into the address book since that ends up being displayed to the user
19:36:22 <S3RK> right, that's the part I don't like about current implementation. but allow me a stupid question first to help guide my thinking
19:36:41 <S3RK> what do we want to actually achieve with this PR?
19:36:51 <S3RK> maybe that's wrong timing because we need to ask Luke
19:38:20 <S3RK> to my best understanding we want a warning when the address we _send to_ has been used already. Is that correct?
19:38:33 <achow101> it's to enable #15987
19:38:35 <gribble> https://github.com/bitcoin/bitcoin/issues/15987 | Wallet, GUI: Warn when sending to already-used Bitcoin addresses by luke-jr · Pull Request #15987 · bitcoin/bitcoin · GitHub
19:38:44 <achow101> S3RK: yes
19:38:55 <S3RK> why would a sender care?
19:39:05 <S3RK> Isn't this receivers responsibility?
19:39:56 <achow101> I think it's useful in some cases
19:40:30 <achow101> e.g. it can remind the sender to double check that the address is up to date
19:41:05 <S3RK> hm.. ok. So the address in question has nothing to do with the wallet, in that case it's indeed seems weird to store the info in the address book
19:41:27 <achow101> there are some services which give out new addresses and stop watching txs to old ones (or are unable to credit the payers to old addresses), so it could remind the sender that they should check the address is correct
19:41:43 <S3RK> got it, thanks
19:42:04 <furszy> well, the addressbook contains "send" addresses.
19:42:28 <S3RK> but IIUC the warning should happen regardless of whether we spent to this address before or somebody else
19:42:31 <S3RK> or not?
19:42:58 <achow101> furszy: those are the explicit destinations. this would include the other addresses in our own incoming too, i.e. the previous sender's change
19:43:40 <achow101> S3RK: probably, but that's a lot of storage
19:44:20 <achow101> i think ideally it would check against every output in the blockchain, but that's a lot of data
19:44:21 <S3RK> yes, does any of the node indexes cover that?
19:44:32 <achow101> no
19:44:58 <furszy> so, seeking to make chain addresses index?
19:45:19 <furszy> if that is the goal, it doesn't sounds to be something that should be placed in the wallet
19:45:42 <S3RK> yep, that's my thinking at the moment
19:46:39 <S3RK> unless we want to restrict the search space to only wallet txs, but then I'm not if that satisfies the requirements
19:46:55 <achow101> I think that's something to ask luke in the pr
19:47:17 <furszy> yeah, I thought that was restricted to already used destinations only
19:48:48 <S3RK> yes, I think we need luke's input here
19:48:57 <S3RK> I don't have more questions on this topic
19:49:28 <achow101> anything else to discuss today?
19:49:43 <furszy> just a small reminder
19:49:56 <furszy> we still have #26699 bugs in master.
19:49:58 <gribble> https://github.com/bitcoin/bitcoin/issues/26699 | wallet, gui: bugfix, getAvailableBalance skips selected coins by furszy · Pull Request #26699 · bitcoin/bitcoin · GitHub
19:50:51 <achow101> furszy: master only?
19:51:01 <achow101> I can add it to the 25.0 milestone
19:51:23 <furszy> IIRC yes, I think that 25685 wasn't included in 24
19:51:36 <furszy> otherwise we would have users complaining already
19:51:44 <furszy> *would be having
19:51:56 <achow101> added it to the milestone so we won't forget it for the next release
19:52:05 <furszy> thx
19:52:28 <achow101> #endmeeting