Adding support for importing non-master extended private key

[deepsimulation]

Hi,

I was trying to import multisig wallet using output descriptors & I realized Sparrow Wallet doesn't natively support BIP32 non-master extended private keys.

If i try to provide non-master tpriv as master private key & 'm' as the path, sparrow wallet shows "incorrect" tpub.

For example: tpub associated with tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK (generated via http://bip32.org/ for this purpose) is tpubDAenfwNu5GyCJWv8oqRAckdKMSUoZjgVF5p8WvQwHQeXjDhAHmGrPa4a4y2Fn7HF2nfCLefJanHV3ny1UY25MRVogizB2zRUdAo7Tr9XAjm

but Sparrow wallet shows tpub as tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B

Upon quick search this could be the issue: https://github.com/sparrowwallet/sparrow/blob/6eefd3f1829cc6b7b5ec947c41311badf27469da/src/main/java/com/sparrowwallet/sparrow/io/Bip32.java#L30

[craigraw]

Indeed, this is not currently supported - the import is specifically named Master Private Key (BIP32). Can you describe the need for this use case?

[deepsimulation]

Yes, our product allow users to create multiple multisig wallets, one of them could be software key. If the key is a hardware device, Sparrow wallet doesn’t have problem signing the psbt. However, for software key, they would import derived xpriv corresponding to their wallet.

After the output descriptor is imported, they would import derived xpriv corresponding to xpub entry in the descriptor string to sign transactions. This allows them keep their other multisig wallets private & secure.

[craigraw]

In the software key situation, what software is deriving the xpriv from the seed? It seems overly complicated to require two applications for one key, and less secure as well since the attack surface is broader.

[deepsimulation]

Some background: We've built multi-account collaborative multisig solution (theya.us). We've designed the software in a way, that gives privacy, security & flexibility for the user to manage multiple multisig wallets. Let's say Alice sets up 2 multisig wallets with Theya:

1. Personal (1 software key owned by Alice + 1 hardware key owned by Alice + 1 Theya key)
2. Shared (1 software key owned by Alice + 1 software/hardware key owned by Bob + 1 Theya key)

The software keys for both accounts are not same. They are however derived from same seed using separate account_index. So, if Alice wants to export their Personal wallet via Sparrow (at this point their Personal wallet is not secure anymore, we'd recommend them to move funds to another wallet they own), they should do it via their derived xpriv instead of mnemonic or master xpriv, latter is not secure because it impacts Shared wallet.

[deepsimulation]

@craigraw is this something you are okay supporting? I’m happy to raise a PR.

[craigraw]

TBH I'm still struggling to understand this.

at this point their Personal wallet is not secure anymore

Why is it not secure anymore? Is the software key compromised in some way? If so, why is the derived private key compromised, and not the master private key?

Making this change to Sparrow would require fairly extensive changes all the way from the UI through to the persistence layer, for a use case that seems poorly defined to me.

[icy-ux]

As discussed in #1208, this should be extended to include importing and exporting all private key based descriptors. This would allow seamless compatibility between Sparrow and other descriptor based wallets. Right now, working with bdk (which uses private key based descriptors) wallets in Sparrow is a giant pain.

If someone wants to import a non-watch-only-wallet using a descriptor, how are they supposed to do that right now? Import the descriptor and then add the seed phrase manually?

If someone wants to export a non-watch-only-wallet to a descriptor-based wallet (e.g anything using BDK)... how are they supposed to do that right now? Export the descriptor, and then -- break down and cry because they only got a watch-only wallet, and that's the only option?

As it stands building a descriptor wallet that only handles public keys seems fundamentally incomplete, as if Michaelangelo had sculpted David but left off one leg.

[craigraw]

@icy-ux I appreciate the use case of working with BDK, but I'm conservative when it comes to exporting private key material. The understanding of protecting seed words is somewhat understood, even if there are constant attempts by scammers to have users enter seed words on many fake Sparrow wallet websites. Offering an export of private key based descriptors will open up another attack vector, but this time with key material that is poorly understood and looks much like public key descriptors. Interoperability is an important goal (and I'll note here that BDK does support importing seed words) but security of funds is critical.

While the current approach seems incomplete to you, it is not by omission - rather it is specifically designed to make it more difficult to lose funds.

That said I'm somewhat okay with this if the wallet was specifically created by pasting in a private key descriptor, since one would assume this would only be done by a technically savvy user. That said, it does create related problems. Such a wallet would not have access to the master secret, and therefore would be a new kind of limited software wallet. It could not, for example, calculate a BIP47 notification address, payment code, or act as a Whirlpool mixing wallet. This increases the support burden, since now should a user experience difficulty with any of the above we must first check whether they have created such a limited wallet. In addition, the software itself must be more complex to handle this situation across a broad range of functionality, which increases the maintenance burden.

All this to point out that there are tradeoffs here, and I need to consider the needs and responsibilities of many before making what is a fairly significant change - creating a new type of software wallet.

[icy-ux]

On Wed, Jan 03, 2024 at 11:30:13PM -0800, craigraw wrote:

@icy-ux I appreciate the use case of working with BDK, but I'm conservative when it comes to exporting private key material. The understanding of protecting seed words is somewhat understood, even if there are constant attempts by scammers to have users enter seed words on many fake Sparrow wallet websites. Offering an export of private key based descriptors will open up another attack vector, but this time with key material that is poorly understood and looks much like public key descriptors. Interoperability is an important goal (and I'll note here that BDK does support importing seed words) but security of funds is critical.

While I can appreciate the problems caused by the infinite number of monkeys coming up with an infinite number of crypto scams, this argument doesn't make sense in this particular case!

When I export an Electrum wallet, I get private key material as a JSON file!

If the user clicks "Export as Electrum", the resulting JSON file is highly sensitive.

If the user clicks "Export as Sparrow", the resulting file is less sensitive.

How are they supposed to know the difference? Are we really expecting the user to: a) look at the file contents, b) realise the zprv in the Electrum file means they shouldn't share it with others, but c) still be dumb enough to send the Sparrow wallet file to a scammer?

Of course not, there's no way they would. Anyone who falls victim to sending a Sparrow export would also send an Electrum export file.

So right now we are restricting wallet functionality in a critical way, but getting no additional security.

What's more, even if we WERE able to get additional security this way, restricting functionality is the wrong way to do it!

Instead, we should be fixing the problem through UX:

1. The process for exporting wallet secrets (seed phrase, private key descriptor, zprv, etc) should be clearly separated from the process of exporting wallet public keys, and clearly marked as such.

Anyone who goes to export master private key material in any format should be crystal clear that they are doing the exact same thing they would do if they exported a seed phrase. So the resulting key should be treated as if it were a seed phrase.

2. Any action to export private key material should come with the usual flashing lights and warning sirens, regardless of the form in which this private key material is delivered.

3. Risky actions, of the kind that should only be useful to users with a high level of technical sophistication, might require a correspondingly high level of technical sopohistication to access. For example, in Electrum doing certain things requires interacting with the Electrum console.

While the current approach seems incomplete to you, it is not by omission - rather it is specifically designed to make it more difficult to lose funds.

Except it doesn't make it more difficult to lose funds, see above!

That said I'm somewhat okay with this if the wallet was specifically created by pasting in a private key descriptor, since one would assume this would only be done by a technically savvy user. That said, it does create related problems. Such a wallet would not have access to the master secret, and therefore would be a new kind of limited software wallet.

Wouldn't it? As far as I know, a descriptor can contain the master secret, but does not have to.

It could not, for example, calculate a BIP47 notification address, payment code, or act as a Whirlpool mixing wallet. This increases the support burden, since now should a user experience difficulty with any of the above we must first check whether they have created such a limited wallet.

Isn't that solved in a very simple way?

If the user tries to perform an action for which the wallet does not have the necessary key material, we know that is what is going on as soon as the condition happens.

The wallet only needs to notify the user with a suitably informative error message: "Can't generate a BIP47 address because this wallet was created with a descriptor that did not include the master secret".

Even if the user doesn't know what this message means, it will come out when the user is asked the first and most basic tech support question: "What's the error message that you get?"

[craigraw]

If the user clicks "Export as Electrum", the resulting JSON file is highly sensitive.

You're right, but I'd argue that exporting an Electrum file is much more buried in the interface than exporting the output descriptor, which is available directly from the Settings tab. In any case, I should look at encrypting that export, so thanks for pointing it out.

Instead, we should be fixing the problem through UX

You are clearly a technically knowledgeable user. Unfortunately I think you overestimate most users understanding of the difference between public and private key material. Many (most?) users do not read, so the solution of "flashing lights and warning sirens" just irritates technical users while doing little to protect less technical ones. I have plenty of experienced evidence of this.

Wouldn't it? As far as I know, a descriptor can contain the master secret, but does not have to.

In general it does not. If you have the master xprv, you can already import it into Sparrow, so this issue is about importing non-master xprvs.

If the user tries to perform an action for which the wallet does not have the necessary key material, we know that is what is going on as soon as the condition happens. The wallet only needs to notify the user with a suitably informative error message.

Some actions (for example, scanning a BIP47 notification address for transactions) happen without user interaction, making this approach potentially confusing.

In general your approach seems to be "let's solve it by showing more dialogs to the user and relying on support to explain things". I don't mean to criticise - for some applications it may be entirely appropriate - but this philosophy runs pretty much directly counter to the approach I've taken, which is to reduce dialogs and reliance on external tech support.

Rather than debate UX philosophy though, I'd like to hear about specific use cases for this functionality. Substantial changes should be accompanied by substantial motivation, and I've still to hear that in this issue.

[icy-ux]

On Thu, Jan 04, 2024 at 11:11:29PM -0800, craigraw wrote:

If the user clicks "Export as Electrum", the resulting JSON file is highly sensitive.

You're right, but I'd argue that exporting an Electrum file is much more buried in the interface than exporting the output descriptor, which is available directly from the Settings tab. In any case, I should look at encrypting that export, so thanks for pointing it out.

There are multiple ways to export the descriptor. One of them is directly in the settings tab. Another one is through the Export dialog, in the same place where people export the wallet in Electrum format.

If you want to export the private key information ONLY via the Export dialog, and not in the Settings tab, that might be a reasonable compromise.

You are clearly a technically knowledgeable user. Unfortunately I think you overestimate most users understanding of the difference between public and private key material. Many (most?) users do not read, so the solution of "flashing lights and warning sirens" just irritates technical users while doing little to protect less technical ones. I have plenty of experienced evidence of this.

Even if it doesn't solve the problem 100%, being able to reduce the problem 50% or 30% is still helpful. Combined with other measures (the export flow philosphy described above) it may get the problem down to manageable levels.

At the end of the day, you cannot solve this issue 100% ... "if you try to make something idiot-proof, someone will come along and make a better idiot."

Wouldn't it? As far as I know, a descriptor can contain the master secret, but does not have to.

In general it does not. If you have the master xprv, you can already import it into Sparrow, so this issue is about importing non-master xprvs.

That wasn't true when I tried it a few days ago -- I imported a descriptor containing an xprv and got a watch-only wallet. Maybe I did something wrong, in which case the solution is probably to improve the UX around this point.

In general your approach seems to be "let's solve it by showing more dialogs to the user and relying on support to explain things". I don't mean to criticise - for some applications it may be entirely appropriate - but this philosophy runs pretty much directly counter to the approach I've taken, which is to reduce dialogs and reliance on external tech support.

Well, no, only part of my suggestions are about dialogs or support. Working with the UX of the application to make it inherently clear to the user what they are doing ("you're at the same place where you would go to export the seed phrase") is just as important, for example.

In any case, my support-oriented suggestion was NOT about "relying on support to explain things" ... my suggestion was about reducing the support workload and closing/resolving issues faster for those cases where support has to deal with something.

If support can resolve the issue by asking for the error message, that's much faster than going back and trying to figure out what the user did wrong.

Rather than debate UX philosophy though, I'd like to hear about specific use cases for this functionality. Substantial changes should be accompanied by substantial motivation, and I've still to hear that in this issue.

Compatibility with any and all BitcoinDevKit-based applications isn't enough of a reason?

bdk has become something of a standard for building Bitcoin-based applications in Rust, Python, Android, and others. bdk is an inherently descriptor-based library, so bdk applications tend to import and export wallet keys (public and private) using descriptors.

If I have an application that uses bdk and I want to import the wallet into Sparrow, right now I get a watch-only wallet.

If I have a Sparrow wallet and I want to use the wallet in a bdk based application, that application only gets a watch-only wallet (because there is no way to export an xprv descriptor).

[craigraw]

When I export an Electrum wallet, I get private key material as a JSON file!

Electrum wallet exports are now encrypted as of b5196d1a where a wallet password is present.

That wasn't true when I tried it a few days ago -- I imported a descriptor containing an xprv and got a watch-only wallet.

It's possible to do this via creating a new wallet, selecting Software Wallet, and using the Master Private Key (BIP32) option to input the xprv.

Extending this functionality to support importing the master private key via a descriptor should be possible as well - it's never come up before, so I haven't implemented it. I will look into this.

bdk applications tend to import and export wallet keys (public and private) using descriptors.

Can you provide an example of such an application? I'd like to try it to understand this issue better.

[icy-ux]

On Sun, Jan 07, 2024 at 11:25:04PM -0800, craigraw wrote:

When I export an Electrum wallet, I get private key material as a JSON file!

Electrum wallet exports are now encrypted as of b5196d1a where a wallet password is present.

Fair, but not all wallets use a password -- most of mine don't, simply because I have dozens of them and the hassle of putting a password on a project-specific wallet outweighs the benefits.

bdk applications tend to import and export wallet keys (public and private) using descriptors.

Can you provide an example of such an application? I'd like to try it to understand this issue better.

Sure, here's a list of projects that use bdk: https://bitcoindevkit.org/case-studies/

Some of these are mobile apps. If you want something that's very simple, command-line based, and shows how bdk uses descriptors, try BTC-XMR atomic swaps: https://github.com/comit-network/xmr-btc-swap https://unstoppableswap.net/

For example, download the swap binary here: https://github.com/comit-network/xmr-btc-swap/releases/latest

then do ./swap export-bitcoin-wallet to get the bdk-generated descriptor.

[craigraw]

Although it does not address the subject of this issue, as of be0ac52a Sparrow supports importing descriptors containing master private extended keys. This solves the use case for importing a wallet created using the swap binary, which exports a master private key.

I'm still looking for specific use cases to motivate the original issue. As noted previously, substantial changes require substantial motivation, especially when they involve importing and exporting of private key material.

[icy-ux]

On Tue, Jan 09, 2024 at 01:46:11AM -0800, craigraw wrote:

Although it does not address the subject of this issue, as of be0ac52a Sparrow supports importing descriptors containing master private extended keys. This solves the use case for importing a wallet created using the swap binary, which exports a master private key.

Great!

I'm still looking for specific use cases to motivate the original issue. As noted previously, substantial changes require substantial motivation, especially when they involve importing and exporting of private key material.

The swap binary example was just the simplest way to show how BDK wallets use descriptors. swap is export-only, the wallet is always generated by the swap binary. Other BDK-based wallet applications (see the link!) also allow importing wallets.

The use case that inspired this is a bdk-based wallet project I am currently working on that has not yet been released -- I need a way to generate a wallet in Sparrow, before importing the descriptor into the final application.

[craigraw]

Other BDK-based wallet applications (see the link!) also allow importing wallets.

Obviously I am aware of many of the applications at that link. They don't to my knowledge export wallets using descriptors with xprvs though, at least not exclusively.

I need a way to generate a wallet in Sparrow, before importing the descriptor into the final application.

What makes you want to deviate from the standard practice of using a BIP39 seed or master xprv for this?

[icy-ux]

On Tue, Jan 09, 2024 at 09:45:49PM -0800, craigraw wrote:

I need a way to generate a wallet in Sparrow, before importing the descriptor into the final application.

What makes you want to deviate from the standard practice of using a BIP39 seed or master xprv for this?

Because seeds or master xprvs don't include the other (important) metadata that a descriptor does.

If I have a descriptor, I know that the wallet I import will be the wallet I exported.

If I have to manually enter derivation paths or other parameters, I may make a mistake or fall victim to different applications making different choices about how to parse and structure things.

This is doubly important when the wallet is:

1. exported for long term archiving (e.g as a QR code paper wallet),
2. multisig, or
3. imported by a different (or non-technical) person than the person who exported it.

[craigraw]

Note it's possible as of https://github.com/sparrowwallet/sparrow/commit/be0ac52a09234dafaaeb1a564b41d58434ce7022 to import master xprvs with specific derivation paths.

[icy-ux]

Very coool, thanks!

On Sun, Jan 14, 2024 at 10:29:57PM -0800, craigraw wrote:

Note it's possible as of https://github.com/sparrowwallet/sparrow/commit/be0ac52a09234dafaaeb1a564b41d58434ce7022 to import master xprvs with specific derivation paths.