Adding support for importing non-master extended private key #1137
Comments
|
Indeed, this is not currently supported - the import is specifically named Master Private Key (BIP32). Can you describe the need for this use case? |
|
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. |
|
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. |
|
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:
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. |
|
@craigraw is this something you are okay supporting? I’m happy to raise a PR. |
|
TBH I'm still struggling to understand this.
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. |
|
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 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. |
|
@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. |
|
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?"
|
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.
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.
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.
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. |
|
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).
|
Electrum wallet exports are now encrypted as of b5196d1 where a wallet password is present.
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 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.
Can you provide an example of such an application? I'd like to try it to understand this issue better. |
|
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 b5196d1 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.
|
|
Although it does not address the subject of this issue, as of be0ac52 Sparrow supports importing descriptors containing master private extended keys. This solves the use case for importing a wallet created using the 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. |
|
On Tue, Jan 09, 2024 at 01:46:11AM -0800, craigraw wrote:
Although it does not address the subject of this issue, as of be0ac52 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.
|
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.
What makes you want to deviate from the standard practice of using a BIP39 seed or master xprv for this? |
|
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.
|
|
Note it's possible as of be0ac52 to import master xprvs with specific derivation paths. |
|
Very coool, thanks!
…On Sun, Jan 14, 2024 at 10:29:57PM -0800, craigraw wrote:
Note it's possible as of be0ac52 to import master xprvs with [specific derivation paths](https://github.com/sparrowwallet/drongo/blob/42f279e5e7cfdcf0de80f60f65857d26db8580ad/src/test/java/com/sparrowwallet/drongo/OutputDescriptorTest.java#L149).
|
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:
sparrow/src/main/java/com/sparrowwallet/sparrow/io/Bip32.java
Line 30 in 6eefd3f
The text was updated successfully, but these errors were encountered: