19:00:49 <achow101> #startmeeting 
19:00:49 <core-meetingbot> Meeting started Fri Oct 22 19:00:49 2021 UTC.  The chair is achow101. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
19:00:49 <core-meetingbot> Available commands: action commands idea info link nick
19:01:08 <achow101> #bitcoin -core-dev Wallet Meeting: achow101 _aj_ amiti ariard BlueMatt cfields Chris_Stewart_5 darosior digi_james dongcarl elichai2 emilengler fanquake fjahr gleb glozow gmaxwell gwillen hebasto instagibbs jamesob jarolrod jb55 jeremyrubin jl2012 jnewbery jonasschnelli jonatack jtimon kallewoof kanzure kvaciral laanwj lightlike luke-jr maaku marcofalke meshcollider michagogo moneyball morcos nehan NicolasDorier paveljanik petertodd
19:01:08 <achow101> phantomcircuit promag provoostenator ryanofsky sdaftuar sipa vasild
19:01:20 <michaelfolkson> hi
19:01:29 <S3RK> hi
19:01:33 <meshcollider> hi
19:02:00 <achow101> there is one preproposed topic, any other topics?
19:03:00 <achow101> #topic feature_segwit.py's dependency on legacy wallet (achow101)
19:03:00 <core-meetingbot> topic: feature_segwit.py's dependency on legacy wallet (achow101)
19:03:38 <achow101> feature_segwit.py is currently legacy wallet only test
19:03:47 <brunoerg> hi
19:03:55 <achow101> I was wondering if we thought it was important enough to refactor to not depend on the lgacy wallet
19:03:56 <gene> hi
19:04:00 <achow101> or if it was ok to just ditch it
19:04:20 <achow101> it seems like a big chunk of it is testing legacy wallet only things
19:04:32 <achow101> but there is also a part that tests segwit activation and non-wallet things
19:04:34 <sipa> let me see
19:05:15 <michaelfolkson> Is this resolved by extending the functionality of MiniWallet (longer term potentially)?
19:05:28 <achow101> michaelfolkson: no, it's specifically testing weird IsMine behavior
19:05:43 <achow101> and how the legacy wallet deals with imports, adding segwit addresses, etc.
19:06:00 <sipa> as long as we have the legacy wallet, i think it's important that those things get tested
19:06:06 <S3RK> is it covered by other tests?
19:06:12 <sipa> but it should be separate from testing segwit consensus/activation logic
19:06:34 <achow101> sipa: yes, but after legacy wallet removal?
19:06:49 <sipa> what do you mean by removal?
19:07:18 <achow101> sipa: I've been experimenting with removing the legacy wallet outright (except for migration) and seeing what other things will need to be done in order to get that through
19:07:36 <sipa> that's useful i guess to see what unnecessarily depends on it
19:07:57 <sipa> though, i hope we're not planning to actually drop support for legacy wallets anytime soon?
19:08:05 <michaelfolkson> For the benefit of those following along achow101 is in process of deprecating both the legacy wallet and BerkeleyDB https://github.com/bitcoin/bitcoin/issues/20160
19:08:07 <achow101> see #20160
19:08:08 <gribble> https://github.com/bitcoin/bitcoin/issues/20160 | Proposed Timeline for Legacy Wallet and BDB removal · Issue #20160 · bitcoin/bitcoin · GitHub
19:08:28 <achow101> sipa: the plan is for a few years
19:08:28 <michaelfolkson> Legacy wallet is non-descriptor wallet
19:08:49 <michaelfolkson> "The 4 combinations of type and format are: legacy-bdb, legacy-sqlite, descriptor-bdb, and descriptor-sqlite"
19:08:49 <sipa> yeah, ok, a few years from now
19:09:55 <sipa> i think in principle, tests testing the wallet behavior should be separate from tests of other behavior
19:10:52 <achow101> agreed
19:10:56 <sipa> the feature_segwit.py test does seem to mix these
19:11:27 <michaelfolkson> So split feature_segwit.py up into 2+ files?
19:11:44 <sipa> i don't think files matter
19:11:57 <sipa> just cleanly separating the tests would be a nice improvement already
19:13:02 <michaelfolkson> It seems to me that legacy wallet support in the tests has to linger to a limited extent to do migration test etc
19:13:36 <achow101> #23312 puts all of the legacy wallet specific tests of feature_segwit behind --legacy-wallet
19:13:37 <gribble> https://github.com/bitcoin/bitcoin/issues/23312 | tests: reduce feature_segwit.py usage of the legacy wallet by achow101 · Pull Request #23312 · bitcoin/bitcoin · GitHub
19:13:52 <sipa> there is a difference between a test _of non-wallet code_ that happens to use the wallet
19:13:58 <sipa> and a test of the wallet code itself
19:14:15 <achow101> although I'm not sure if the first part that does some stuff with multisig is testing legacy wallet or just incidental
19:15:18 <michaelfolkson> "a test _of non-wallet code_ that happens to use the wallet" should use MiniWallet longer term right?
19:15:30 <sipa> maybe, i haven't paid attention to that
19:15:33 <sipa> but probably, yes
19:15:36 <michaelfolkson> But not "a test of the wallet code itself"
19:17:13 <achow101> ok, I guess I will try to split it up
19:17:32 <sipa> FWIW, i think a bunch of non-wallet tests of script features etc can probably be redone in feature_taproot.py
19:17:46 <sipa> it has a whole framework of randomly constructing errors/transactions/scripts
19:17:57 <michaelfolkson> Hmm I guess a refactor would be better but maybe someone other than achow101 could attempt it (if he has enough to do!)
19:18:33 <michaelfolkson> Interested S3RK? :)
19:18:36 <sipa> and feature_taproot.py itself might be portable to use miniwallet for the wallet side of things (it already mostly does its own utxo management etc)
19:19:15 <S3RK> i can take on it, once i finish the coin selection improvement that I started
19:20:24 <S3RK> i hope it can wait, because with my speed it might take a while :)
19:20:44 <sipa> it's been there for 4 years, it won't run away
19:21:11 <achow101> I have another topic if we're done with this one?
19:21:58 <achow101> #topic having ScriptPubKeyMans fetch keys from a wallet keystore rather than handling keys individually
19:21:58 <core-meetingbot> topic: having ScriptPubKeyMans fetch keys from a wallet keystore rather than handling keys individually
19:23:05 <achow101> one of the issues with setting up multisigs and setting up new descriptors is that keys are tied to ScriptPubKeyMans, so it is difficult to get a "wallet key"
19:23:18 <sipa> (judging based just on the one-line summary) that sounds like a good idea... you could think of it as the integrated "software signing device" of the wallet
19:23:38 <S3RK> do you mean master key, right?
19:24:20 <achow101> sipa: yes, I was thinking of something in the same model of external hardware signer but internal and software
19:24:45 <sipa> i think it makes sense... there is nothing that makes private keys tied to specific SPKmans
19:24:51 <achow101> S3RK: kind of
19:25:10 <sipa> spkmans mostly ought to live in the "deciding what is ours" domain, not in the "what can we sign" domain
19:25:33 <achow101> sipa: but "what can we sign" should align with "what is ours"
19:25:57 <sipa> achow101: well, maybe- there are also external signers, watch-only wallets, multisig you can participate in
19:26:11 <sipa> those are modeled by spkmans, but their signing story is kind of orthogonal
19:26:36 <achow101> sipa: but all of those things would be tied to a spkman that should be in the wallet
19:26:53 <sipa> right, but does it matter _which_ spkman?
19:26:58 <sipa> holds the keys
19:27:17 <sipa> like you can sign for something, or you can't
19:27:47 <achow101> sure
19:27:47 <sipa> if you'd have the same pubkey/xpub in multiple distinct descriptors, would you ever not want to sign with one, because it was linked with another descriptor?
19:27:56 <sipa> you have the key, might as well use it
19:28:04 <achow101> I think that's fine
19:28:13 <achow101> what I want to avoid is signing for things for which we don't have a spkman for
19:28:26 <achow101> e.g. if the wallet only has a wpkh(), then it shouldn't sign for multisigs even if it has the keys
19:28:51 <sipa> i think that's fine; i'm not sure it's necessarily a problem if it's different
19:29:24 <sipa> once an output is created it's "too late" in a sense - if you can sign for it, and it's in a transaction that you agree with, why wouldn't you?
19:30:16 <sipa> of course, in practice, in order to sign you'll likely need access to an SPKMan to pull things like redeemscripts and whatnot
19:30:32 <achow101> if it's in a psbt, not necessarily
19:30:34 <sipa> right
19:30:59 <sipa> in a psbt, i think there is no problem at all with signing whatever you can, independent of whether it's something you consider yours or not
19:31:20 <sipa> but i'm also fine with "that's too hard to make work, and isn't supported"
19:31:44 <achow101> it's probably easier to make that work given the way our signing code is setup
19:32:17 <achow101> in any case, it seems like this change would be a good idea
19:32:38 <S3RK> fwiw, I think it's good
19:33:01 <achow101> it would also make adding new descriptors like tr() easier
19:33:14 <S3RK> and multisig
19:33:21 <achow101> any other topics
19:33:27 <sipa> right, import the same xpub (maybe different branch), and signing will just work?
19:34:23 <achow101> #endmeeting