Please unbreak single-key wallets (and potentially remove internal/external distinction entirely)

[stevenroose]

In alpha.9 or something, I could pass None for the internal descriptor. Now it is no longer an Option, so I passed in the same descriptor, but we get an error that they can't be the same.

We have a wallet that is a single key, simple tr(<pubkey>) descriptor where internal and external addresses are identical. This used to work perfectly before.

We are hitting this message: write!(f, "External and internal descriptors are the same")

[storopoli]

This was discussed in #1383

[stevenroose]

Hmm, from reading the convo, it's not very clear that the explicit decision was made to no longer support single-key wallets. What is the difficulty in doing so? I don't fully understand what this change makes internal development easier.

I would like to strongly request to bring back support for single-key wallets. Also, the internal/external distinction is kinda stupid anyway, I don't see a good reason to keep this structure altogether. I think if BDK wants to take a progressive stance, it should do away with the dual descriptors entirely and just have a single descriptor. (AFAIK the difference originated in wallets not being able to remember which addresses were already given out so they would accidentally use an address they gave to someone for change. But since BDK, and more modern wallets for that matter, never allow re-handing out an address, having both is no longer needed. When you need a change address, you can just pull a new address from the key pool.)

[stevenroose]

EDIT: It seems that another motivation for the distinction is to enhance privacy for users that use Electrum-like servers. They could avoid informing the server about their internal addresses. Ever since we have compact block filters, Electrum-like setups are less and less needed and we should definitely think about making progress away from this model and not doubling down on it, IMO.

[notmandatory]

Unfortunately many/most of our users are still using electrum and/or esplora servers so I think this privacy concern still exists. I can see us removing this restriction in the future once CBF is more common, but for now the decision was to be more opinionated with the Wallet apis and to support the more common uses.

If you want to use BDK for less common scenarios you can still construct a custom wallet using the bdk_chain crate, which we have examples for.

[stevenroose]

You are deciding to provide a worse experience for users that have better wallet practices? All wallets that use bitcoind, cbf, their own personal electrum server or anything else that is not a public blockchain index, are now suffering that they need to deal with two different descriptors, potentially write these down, etc etc, all because a lot of other users do use Electrum?

Also, entirely dropping support for single-key wallets is also a very big feature cut IMO that should be discussed independently of the int/ext discussion.

[notmandatory]

Maybe I don't understand your use case for TR single key wallets, would these always produce the same SPK? are they needed for LN taproot channels or some other type of TR user?

[notmandatory]

I put this on the discussion tab for our alpha milestone to get feed back from others on the team on if we really need to revert #1390.

[ValuedMammal]

It shouldn't be too difficult to build in support for single descriptor wallets. Long term we might consider doing away with KeychainKind enum and supporting wallets with one or more keychains.

[notmandatory]

@stevenroose have you taken a look at the example_bitcoind_rpc_polling cli wallet ? it's an example that is built on just the bdk_chain crate and doesn't use the bdk_wallet at all. This would be the best way to use BDK for a custom on-chain app with a single keychain or any other custom behavior. The example isn't fully fleshed out but we'll be doing that once we get the first beta release out.

[LLFourn]

I think trying to build a single descriptor wallet is a fine idea. But that's not Wallet. Wallet was trying to internally map the internal keychain to the external keychain if the internal didn't exist. I don't think that's the "modern" approach you are after @stevenroose. The correct way to approach this is to create a SingleDescriptorWallet that only takes one descriptor. The architect for this will have to figure out how mark which derivation indices are being used for change, which have been revealed for receiving funds, how and whether to persist these designations and how to figure out confirmed/unconfirmed balance etc etc.

[stevenroose]

will have to figure out how mark which derivation indices are being used for change, which have been revealed for receiving funds

My argument is that this distinction only matters for wallets using a public chain index. So maybe a wallet that cares about keeping track of internal and external addresses separately should be called ElectrumWallet or PublicIndexWallet and wallets that don't care about this just Wallet? Also, the privacy gain from separating them before sending to a public index service is marginal: you send them all your incoming funds and doing the further graph analysis is often trivial. So even for those, I am not sure the complexity of managing two sets of addresses is merited.

IMO refactoring to make KeychainKind a second class citizen that is only enabled for users explicitly requesting so, would simplify the API for regular users. Having them a first class citizen also feels like it encourages bad behavior of using public index services.

I've had many conversations with Evan about ideas of making compact block filters more ergonomic to use and how BDK is perfectly positioned to use those (with something like Neutrum, public servers of blocks and block filters, wallets can also do full sync without needed p2p or chain data but also without sending their addresses to a third party). I believe this is the way forward instead of doubling down on the Electrum model.

[notmandatory]

These are all sound technical arguments for supporting a Wallet with only a single descriptor, but the overriding non-technical arguments for keeping our current API for the 1.0 are:

1. The primary goal for the 1.0 release was to support async for Wallet persistence and blockchain syncing, this has been accomplished.
2. Most of our existing users use simple descriptors with extended keys and change descriptors.
3. Most of our users use the electrum or esplora clients by the simple fact that we don't have a rust CBF client ready for production use. We may have some RPC client users but they are much less common right now.
4. Getting to a BDK 1.0 stable API has taken more than a year longer than expected and we have users anxiously waiting for it. We hope to wrap this up in a week and don't have time to incorporate any major new changes.

The good news is 1.0 is our first, but won't be our last stable API release. If we can incorporate single descriptors in a non-breaking way in a 1.1 release (eg. some new Wallet constructor methods) we can get it in there. Or worst case we target this for a 2.0 milestone, where we've already discussed supporting "multi descriptor" (ie. 1..n descriptor) wallets.

[notmandatory]

I did a little experimenting and we might be able to support single descriptor wallets in with the existing Wallet, but just not for our 1.0 beta milestone. I did a quick experiment with #1513 and it looks like we can do it as a non-breaking change (new constructors) so could get it in a 1.1 release.

[cryptoquick]

It would be really helpful if we could get this in for 1.0. For compatibility purposes, could we just make it so if the same descriptor is passed as both arguments, it's just treated as a single descriptor for now?

[ValuedMammal]

I can work on this. In order to not break the current (beta) API, we may just add a way to construct CreateParams with one descriptor instead of two. Some of the immediate consequences are

• Attempting to reveal script pubkeys/addresses for an empty keychain would cause a panic
• Methods involving the ChangeSpendPolicy type would be less significant

[storopoli]

• Attempting to reveal script pubkeys/addresses for an empty keychain would cause a panic

Yes, we should abort at any potential loss of funds. Agreed!

[matthiasdebernardini]

I've asked on X/Twitter why have two descriptors instead of one. I got some great answers from the community. From my perspective, it comes down to privacy/backup restoration UX.

Optech writeup

Bitcoin Core related to the PR

For our use case, we have two descriptors, but our wallet can guarantee metadata persistence (we can add labels to the addresses and mark them as change and always track which addresses are given out) which would take care of privacy/backup concerns. Therefore we could have one instead of two. This change would simplify the code for creating the descriptors for our users. Despite this, its such a small value add and would make our descriptors incompatible with wallets expecting a second descriptor that staying with two is the best way forward.

I'm adding these links here to add some color to the discussion.

[Rob1Ham]

I'm in support of single descriptor wallets as a use case. In addition to use cases brought up so far here, descriptors allow for multiple paths within them. The standard change/receive functionality is used with <0;1> syntax for descriptors for most hardware wallets at this point.

If this was enabled for BDK, I could see many multi descriptor wallets falling under the single descriptor path, and using this syntax to clean up code, and bring about uniform interoperability between HWW descriptors and BDK descriptors.

As of now we treat BDK external/internal descriptors as first class citizens, and do conversions to <0;1> for HWWs at the last mile, but it adds additional maintenance in our codebase to do this.

[notmandatory]

If this was enabled for BDK, I could see many multi descriptor wallets falling under the single descriptor path, and using this syntax to clean up code, and bring about uniform interoperability between HWW descriptors and BDK descriptors.

I agree, and now that we've added back support for a single descriptor when creating a wallet we should be able to do #1021 as a non-breaking change in a 1.x version.