19:01:34 <achow101> #startmeeting 
19:01:35 <core-meetingbot> Meeting started Fri Aug 27 19:01:34 2021 UTC.  The chair is achow101. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
19:01:35 <core-meetingbot> Available commands: action commands idea info link nick
19:01:49 <michaelfolkson> hi
19:01:53 <S3RK> hi
19:01:53 <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:54 <achow101> phantomcircuit promag provoostenator ryanofsky sdaftuar sipa vasild
19:01:59 <prayank> hi
19:02:06 <jarolrod> hi
19:02:16 <achow101> two proposed topics
19:02:35 <achow101> #topic wallet files should not be reused across chains (prayank)
19:02:35 <core-meetingbot> topic: wallet files should not be reused across chains (prayank)
19:03:29 <sipa> prayank: you have the floor
19:03:46 <prayank> I tried this PR
19:04:11 <prayank> https://github.com/bitcoin/bitcoin/pull/18554
19:04:20 <prayank> Which doesn't fix the issues
19:04:36 <prayank> So I was thinking if we can save this info in db
19:04:54 <prayank> If a wallet was created for testnet or mainnet or signet
19:05:03 <prayank> And check this to give error
19:05:12 <prayank> But this won't work for old wallets
19:05:14 <michaelfolkson> This is the context https://github.com/bitcoin/bitcoin/issues/16107
19:05:30 <S3RK> I remember there was a proposal to check best block record and compare genesis blocks
19:05:40 <sipa> that seems to be what 18554 is doing
19:06:20 <achow101> comparing bestblock only works if it is in the mainchain. If it's a stale block, and the wallet is loaded on a node that doesn't have it, it won't work
19:06:24 <prayank> Yes it does something with blocks which doesn't look the right approach or maybe we need to add more things in if statement
19:06:37 <sipa> prayank: if you tested that PR, and found it isn't doing what it is claimed, you should leave that as a review on the PR
19:06:56 <prayank> I commented in PR
19:07:09 <michaelfolkson> sipa: https://github.com/bitcoin/bitcoin/pull/18554#issuecomment-904808840
19:07:35 <sipa> i saw that
19:07:39 <sipa> but it doesn't say anything
19:08:36 <prayank> I will add more details in the comment
19:08:36 <michaelfolkson> I think prayank was possibly unsure why it wasn't fixing them
19:09:02 <prayank> Yeah I wasn't even sure if this is the only PR that is working to resolve this issue
19:09:05 <S3RK> achow101: how does rescan works in the case of stale bestblocked absent on the node?
19:09:11 <michaelfolkson> It appears this PR should be closed if it is taking a flawed approach
19:09:52 <achow101> S3RK: I think it rescans from genesis
19:11:10 <sipa> i see it's looking at the block locator record in the wallet
19:11:15 <sipa> i don't see why it would ever not work
19:11:22 <sipa> as the locator always includes the genesis block
19:11:51 <achow101> oh, it's a locator? I thought it was just the hash
19:12:10 <sipa> oh
19:12:11 <sipa> hmm
19:12:58 <sipa> WalletBatch batch(walletInstance->GetDatabase()); CBlockLocator locator; if (batch.ReadBestBlock(locator))
19:13:16 <sipa> what it's doing below seems overly complicated
19:13:19 <sipa> it can just compare the genesis
19:13:52 <S3RK> but there is no protection now, the code now just determines the point to rescan from
19:14:32 <achow101> sipa: is CBlockLocator guaranteed to have the genesis hash?
19:14:41 <michaelfolkson> sipa: Greg said genesis block isn't sufficient as altcoins share genesis blocks https://github.com/bitcoin/bitcoin/pull/14533#issuecomment-431645072
19:15:10 <sipa> achow101: yes
19:15:12 <sipa> michaelfolkson: oh, i see
19:15:34 <sipa> though that's unnecessary if the concern is mixing testnet/mainnet
19:16:02 <sipa> but fair, it's nicer if it can account for that too
19:16:20 <sipa> in any case, i don't think there is much to discuss here; comments can go on the PR
19:17:24 <achow101> probably need to have a network magic record?
19:18:07 <achow101> in any case, I agree this can be discussed in the PR
19:18:31 <achow101> #topic automatically adding record to adress book to prevent #19856 (S3RK)
19:18:31 <core-meetingbot> topic: automatically adding record to adress book to prevent #19856 (S3RK)
19:18:33 <gribble> https://github.com/bitcoin/bitcoin/issues/19856 | Some transactions are not shown in listtransactions output · Issue #19856 · bitcoin/bitcoin · GitHub
19:18:33 <gribble> https://github.com/bitcoin/bitcoin/issues/19856 | Some transactions are not shown in listtransactions output · Issue #19856 · bitcoin/bitcoin · GitHub
19:19:30 <S3RK> descriptor wallets already contain magic afaik
19:19:30 <S3RK> yes. So there is a problem with miscategorizing self-to-self transactions as change
19:19:30 <S3RK> this leads to them missing from listtransactions
19:19:36 <S3RK> it affects cases when restoring wallets with missing metadata
19:19:47 <S3RK> or having same wallet loaded in parallel in two nodes
19:20:31 <S3RK> I created a prototype to fix that by automatically adding such addresses to address book
19:20:36 <S3RK> but I'm not sure that's a way to go
19:20:55 <achow101> send to self should already be in the address book because the address had to be requested
19:21:07 <S3RK> yes, but no in the cases I described
19:21:57 <achow101> the situations this bug occurs in are restored wallets and having another node loaded with the same wallet
19:22:12 <S3RK> even we discard the case with the same wallet loaded in two places
19:22:20 <S3RK> the case with restored wallet seems like a bug
19:23:36 <S3RK> I don't see many downsides to add such addresses automatically to the address book
19:23:46 <S3RK> or should I say any downsides
19:24:05 <achow101> IMO it's not a bug. I think if you are restoring a wallet, you can expect that some metadata will be missing, e.g. whether something is or is not change
19:24:47 <S3RK> whether it's a bug or not is secondary. It's a poor ux and we can reasonably fix that
19:25:43 <achow101> I don't mind if it isn't very invasive
19:25:49 <S3RK> I can't think of any downsides, but I may be missing something
19:26:40 <achow101> the only downside is if we want to change back to using a single key chain rather than a split for receive and change
19:26:56 <S3RK> the prototype is here https://github.com/S3RK/bitcoin/tree/fix_19856
19:27:02 <achow101> since afaict, you need the split in order to correctly determine whether an output is send to self or change
19:28:21 <S3RK> not exaclty, even without descriptors you can get it from keypool records
19:28:54 <achow101> only for legacy wallets
19:29:34 <S3RK> agree, one chain makes it much more complicated or even impossible
19:30:19 <achow101> in any case, if you open a PR, we can discuss further there
19:30:43 <S3RK> ok. I wasn't sure if it makes sense to invest more time. Will open a PR
19:31:17 <achow101> any other topics to discuss?
19:31:34 <S3RK> I have a question
19:32:20 <S3RK> do you have any updates on the upgrading wallets to tr descriptors?
19:32:43 <achow101> No, I thought it might be something we should discuss at coredev
19:32:55 <S3RK> thumbs up
19:33:04 <michaelfolkson> +1
19:33:35 <michaelfolkson> Latest Twitch streams have been on multipath descriptors
19:33:47 <S3RK> there is a gist with topic ideas for coredev
19:34:11 <achow101> I'll comment on it (don't post that here)
19:34:35 <achow101> anything else?
19:34:42 <michaelfolkson> It hasn't clicked with me why multipath descriptors are important but I didn't do sufficient reading
19:34:49 <michaelfolkson> So let me do that first
19:34:53 <michaelfolkson> No nothing from me
19:34:56 <S3RK> one more small thing
19:35:03 <S3RK> I'm reviewing #22067
19:35:05 <gribble> https://github.com/bitcoin/bitcoin/issues/22067 | Test and document a basic M-of-N multisig using descriptor wallets and PSBTs by mjdietzx · Pull Request #22067 · bitcoin/bitcoin · GitHub
19:35:25 <S3RK> and I wonder why can't we use one wallet for this multisig setup
19:36:07 <achow101> I think it's to demonstrate the use of combinepsbt
19:36:20 <achow101> and generally the passing around of PSBTs
19:36:32 <S3RK> no, I mean each participant have two wallets
19:36:47 <S3RK> one to signer and one watch-only multisig
19:36:54 <S3RK> can't we have both descriptors in one wallet?
19:37:05 <achow101> oh, pure watchonly can't be imported into a wallet with private keys
19:37:16 <S3RK> yes, but you can replace your xpub with xprv
19:38:18 <achow101> sure
19:38:25 <achow101> that's something you can ask in the pr
19:38:39 <S3RK> will do. Just checking if I'm missing something obvious
19:39:07 <S3RK> that's all from me
19:39:30 <achow101> anything else?
19:39:46 <S3RK> thank you for your time :)
19:40:02 <michaelfolkson> Yeah thanks achow101
19:40:03 <achow101> #endmeeting