LtbLightning / bdk-flutter

Bitcoin Development Kit - Flutter Package
MIT License
63 stars 29 forks source link

Add a constructor to DescriptorPublicKey #59

Closed awaik closed 1 year ago

awaik commented 1 year ago

The logic of the wallet

  1. For security reasons, we shouldn't keep DescriptorSecretKey in the app.
  2. So, we use DescriptorPublicKey and the user works with the read-only wallet.
  3. Only in the case when the user sens BTC app asks for confirmation with a password, generate DescriptorPublicKey, and make a transaction.

The problem

So, we create DescriptorPublicKey

    final publicKey = await descriptorSecretKey.asPublic();

    final descriptorPublic = await Descriptor.newBip84Public(
      publicKey: publicKey,
      network: Network.Testnet,
      keychain: KeychainKind.External,
    );

and need to save it between app sessions in the secured_storage. Secured storage can't keep custom object, so we save the String.

publicKey.asString();

And when the app runs again we need to create DescriptorPublicKey without a seed phrase and etc.

It would be great to have DescriptorPublicKey.fromString(String publicKeyString) constructor.

thunderbiscuit commented 1 year ago

I think you might be able to do this already with the Descriptor type directly (without going through building the key every time). Let me confirm your workflow though because I want to make sure I get this right:

  1. You make the wallet with the private key and store that secret in the secure storage. You then use the public descriptor for the daily use wallet.
  2. The app normally only has access to the public descriptor, so it can see and receive, but when you need to send, you pull that secret key (or could be secret descriptor) from the secure storage and sign the tx

If that's the case, you can simply store the secret descriptor string in your secure storage, and then recreate the wallet using that directly. I have example scripts here in Kotlin. The steps would be to create two descriptors (private and public).

Examples:

// private descriptor
wpkh(tprv8ZgxMBicQKsPf2qfrEygW6fdYseJDDrVnDv26PH5BHdvSuG6ecCbHqLVof9yZcMoM31z9ur3tTYbSnr1WBqbGX97CbXcmp5H6qeMpyvx35B/84h/1h/0h/0/*)

// associated public descriptor
wpkh([9122d9e0/84'/1'/0']tpubDCYVtmaSaDzTxcgvoP5AHZNbZKZzrvoNH9KARep88vESc6MxRqAp4LmePc2eeGX6XUxBcdhAmkthWTDqygPz2wLAyHWisD299Lkdrj5egY6/0/*)#zpaanzgu

Once you have the string for the secret descriptor, you can just build it using

final descriptorString = wpkh(tprv8ZgxMBicQKsPf2qfrEygW6fdYseJDDrVnDv26PH5BHdvSuG6ecCbHqLVof9yZcMoM31z9ur3tTYbSnr1WBqbGX97CbXcmp5H6qeMpyvx35B/84h/1h/0h/0/*)
final privateDescriptor = Descriptor(
    descriptorString, 
    network
)

Let me know if I'm off the mark or didn't understand your requirements!

awaik commented 1 year ago

Thanks for the answer.

No, in the app I keep only a public key.

  1. The entropy is generated using a cryptographically secure random number generator. 1.2 It's then encrypted with a key that is securely derived from the password.
  2. Create a wallet, public key.
  3. Write the public key to secured_storage.
  4. App doesn't have a secret key anywhere.
  5. When a user sends BTC, the app asks password, generates a seed phrase, creates a secret key, and makes a transaction.

So, when app is restarted is has only public key.

thunderbiscuit commented 1 year ago

Oh I see I didn't have your workflow right. So are you generating a seed phrase from this password that the user inputs somewhere? Is that process deterministic? I'm not sure I fully understand yet. Happy to jump on a call if you'd like (hit me up on the BDK discord).

Just to be clear, you also don't keep the mnemonic on the device is that correct? And so here when you say "password", you're not speaking of the "26th word, sometimes referred to as the passphrase" in the BIP39 standard, but rather a separate, user-generated password?

notmandatory commented 1 year ago

@awaik it would make sense if your app is doing something like this:

  1. generate 12 seed words + password
  2. make sure user saves 12 seed words offline (ie. on paper or steel plate or whatever) and memorize password
  3. create bip84 private descriptor from 12 seed words + password
  4. create bip84 public descriptor from bip84 private descriptor
  5. save bip84 public descriptor on phone, don't save 12 seed words or password on phone
  6. use bip84 public descriptor to sync wallet and create unsigned transactions
  7. user must enter 12 seed words + password to re-create bip84 private descriptor and sign transactions
awaik commented 1 year ago

@notmandatory Yes, I use the same flow. And the issue is about

6. use bip84 public descriptor to sync wallet and create unsigned transactions

I can't create bip84 public descriptor after the app is restarted from the string that I get before this publicKey.asString();

awaik commented 1 year ago

@thunderbiscuit

In the app I don't keep:

  1. seed phrase
  2. any passwords
  3. secret key

I keep in the app:

  1. publicKey.asString();

So, when the app is restarted I need to create DescriptorPublicKey from the string that I saved in the secured_storage publicKey.asString();

thunderbiscuit commented 1 year ago

Ok cool I see what you mean. Given what you've said here:

I keep in the app publicKey.asString(); So when the app is restarted I need to create DescriptorPublicKey from the string that I saved in the secured_storage publicKey.asString();

I think all you need to do is change your workflow to save the public descriptor in its entirety rather than just the extended public key. Instead of saving this in your secure storage:

tpubDCYVtmaSaDzTxcgvoP5AHZNbZKZzrvoNH9KARep88vESc6MxRqAp4LmePc2eeGX6XUxBcdhAmkthWTDqygPz2wLAyHWisD299Lkdrj5egY6

You should save this (or whatever your full public descriptor looks like):

wpkh([9122d9e0/84'/1'/0']tpubDCYVtmaSaDzTxcgvoP5AHZNbZKZzrvoNH9KARep88vESc6MxRqAp4LmePc2eeGX6XUxBcdhAmkthWTDqygPz2wLAyHWisD299Lkdrj5egY6/0/*)#zpaanzgu

And you'll be able to recreate the watch-only wallet every time without trouble.

BitcoinZavior commented 1 year ago

@awaik Yes along the same lines as @thunderbiscuit just explained. You can serialise and store DescriptorPublicKey and deserialise when required. I think in Dart its done via json encode and json decode.

awaik commented 1 year ago

@BitcoinZavior @thunderbiscuit

Thank you, but still have the problem.

Yes, I saved the full public descriptor

pkh([9122d9e0/84'/1'/0']tpubDCYVtmaSaDzTxcgvoP5AHZNbZKZzrvoNH9KARep88vESc6MxRqAp4LmePc2eeGX6XUxBcdhAmkthWTDqygPz2wLAyHWisD299Lkdrj5egY6/0/*)#zpaanzgu

And after that tried to create a wallet with this code

    final Descriptor descriptorPublic = await Descriptor.create(
      descriptor: descriptorPublicString,
      network: Network.Testnet,
    );

    Wallet wallet = await Wallet.create(
      descriptor: descriptorPublic,
      network: Network.Testnet,
      databaseConfig: const DatabaseConfig.memory(),
    );

And this wallet is not the same one that I created before. And for the DescriptorPublicKey class there is no constructor, so, I can't instantiate it.

BitcoinZavior commented 1 year ago

@awaik How did you check if the wallet is the same or not?

awaik commented 1 year ago

@BitcoinZavior I tried to check a balance and transactions.

If I create it with a secret descriptor - it has data (I made test transactions).

In case I create it with a public descriptor - no data.

BitcoinZavior commented 1 year ago

@awaik I am getting the same wallet.

This is what I did, created a descriptor from a mnemonic. got the string public descriptor by descriptor.asString() Which was "wpkh([9ef9c59f/84'/1'/0']tpubDD5KMi8Ufi5zD25ptmanaSeS8AnNProd4byM4xxh4o56qBCtSShiAHCfEBHU9KQjjU2C3TRqhbwxyfVUvqtTo7K8vUeo19tJcf3hRzR8n7M/0/*)#r9exju72"

Recreated descriptors and recreated wallet.

 final descriptors = <Descriptor>[];
  final descriptor = await Descriptor.create(
      descriptor: "wpkh([9ef9c59f/84'/1'/0']tpubDD5KMi8Ufi5zD25ptmanaSeS8AnNProd4byM4xxh4o56qBCtSShiAHCfEBHU9KQjjU2C3TRqhbwxyfVUvqtTo7K8vUeo19tJcf3hRzR8n7M/0/*)#r9exju72",
      network: Network.Testnet);

  final change = await  Descriptor.create(
      descriptor: "wpkh([9ef9c59f/84'/1'/0']tpubDD5KMi8Ufi5zD25ptmanaSeS8AnNProd4byM4xxh4o56qBCtSShiAHCfEBHU9KQjjU2C3TRqhbwxyfVUvqtTo7K8vUeo19tJcf3hRzR8n7M/1/*)#j3u80fwj",

  network: Network.Testnet);
  descriptors.add(descriptor);
  descriptors.add(change);

 final res = await Wallet.create(
          descriptor: descriptors[0],
          changeDescriptor: descriptors[1],
          network: network,
          databaseConfig: const DatabaseConfig.memory());

wallet created had the same balance, same initial address and same transactions.

awaik commented 1 year ago

@BitcoinZavior Thank you for the example.

(edited)

There are 2 different strings. How did you get the second string? wpkh([9ef9c59f/84'/1'/0']tpubDD5KMi8Ufi5zD25ptmanaSeS8AnNProd4byM4xxh4o56qBCtSShiAHCfEBHU9KQjjU2C3TRqhbwxyfVUvqtTo7K8vUeo19tJcf3hRzR8n7M/1/*)#j3u80fwj

BitcoinZavior commented 1 year ago

@awaik Creating the descriptor with keychainKind.internal and calling descriptor.asString()

Here is the API reference: https://pub.dev/documentation/bdk_flutter/latest/bdk_flutter/Descriptor-class.html

Here is a code example from the quickstart demo wallet: https://github.com/LtbLightning/bdk-flutter-quickstart/blob/master/lib/screens/home.dart#L32-L54

This article has a couple of lines of explanation on the external and internal addresses/descriptors: https://bitcoindevkit.org/tutorials/exploring_bdk_flutter/

So in your case, in this code, you will call asString on the external descriptor and the change descriptor. And keep them both and use them to recreate the wallet. https://github.com/LtbLightning/bdk-flutter-quickstart/blob/master/lib/screens/home.dart#L32-L54

awaik commented 1 year ago

@BitcoinZavior Thank you for the explanations, now it is clear. But, sorry, still wasn't successful get the balance with the public key.

I made a fork and commit an example with public keys - there is an additional button - Create Wallet Public https://github.com/LtbLightning/bdk-flutter-quickstart/compare/master...awaik:bdk-flutter-quickstart:master

The code for creating keys I copy from docs and examples that you gave before.

Can you please check it? (I've hardcoded the seed phrase):

  1. if we create wallet with this seed phrase and secret keys - it has balance (I've transferred)
  2. if we do the same with public keys - no balance
  /// get public descriptors
  Future<List<Descriptor>> getPublicDescriptors(String mnemonicStr) async {
    final descriptors = <Descriptor>[];
    try {
      for (var e in [KeychainKind.External, KeychainKind.Internal]) {
        final mnemonic = await Mnemonic.fromString(mnemonicStr);
        final descriptorSecretKey = await DescriptorSecretKey.create(
          network: Network.Testnet,
          mnemonic: mnemonic,
        );
        final descriptorPublicKey = await descriptorSecretKey.asPublic();
        final descriptor = await Descriptor.newBip84Public(
          publicKey: descriptorPublicKey,
          network: Network.Testnet,
          keychain: e,
        );
        descriptors.add(descriptor);
      }
      return descriptors;
    } on Exception catch (e) {
      setState(() {
        displayText = "Error : ${e.toString()}";
      });
      rethrow;
    }
  }
Screenshot 2023-02-20 at 14 23 26
awaik commented 1 year ago

@BitcoinZavior @thunderbiscuit Sorry for bothering you, can you please check the previous message? I've checked today again in different combinations and wasn't successful to get the data with public keys.

BitcoinZavior commented 1 year ago

Hi @awaik , Apologies, will take a look shortly.

BitcoinZavior commented 1 year ago

@awaik Please replace this method. I tried to push the code to your repository but couldn't, but its fine, the below method is the only change. I have added comments for the changes, let me know if this resolves your issue.

  /// get public descriptors
  Future<List<Descriptor>> getPublicDescriptors(String mnemonicStr) async {
    final descriptors = <Descriptor>[];
    try {
      for (var e in [KeychainKind.External, KeychainKind.Internal]) {
        final mnemonic = await Mnemonic.fromString(mnemonicStr);
        final descriptorSecretKey = await DescriptorSecretKey.create(
          network: Network.Testnet,
          mnemonic: mnemonic,
        );
        // Once the descriptorSecretKey is created, we can get the public descriptor
        // by calling asString() on it.
        final publicDesctriptor = await descriptorSecretKey.asString();
        // Now we can store the public descriptor in a database or persist in some
        // other way.

        // When we need the descriptor recreated we can use the Descriptor.create()
        // and use the string we got from asString()
        final descriptor = await Descriptor.create(
            descriptor: publicDesctriptor,
            network: Network.Testnet
        );
        descriptors.add(descriptor);
      }
      return descriptors;
    } on Exception catch (e) {
      setState(() {
        displayText = "Error : ${e.toString()}";
      });
      rethrow;
    }
  }
awaik commented 1 year ago

@BitcoinZavior Thank you for the help! Now it is clear. I suggest reflecting it in the docs

// Once the descriptorSecretKey is created, we can get the public descriptor
        // by calling asString() on it.
        final publicDesctriptor = await descriptorSecretKey.asString();

otherwise, it is a little bit confusing. Looking at naming I thought that descriptorSecretKey.asString() creates secret key string, not a public.

Or maybe change the name to the getPublicKeyAsString()

BitcoinZavior commented 1 year ago

@awaik Thanks for the suggestions. I am updating the readme as we speak to add more information about descriptors. Will also consider changing the method name.

Great that it worked out and that you are able to implement your use case  👍

awaik commented 1 year ago

@awaik Please replace this method. I tried to push the code to your repository but couldn't, but its fine, the below method is the only change. I have added comments for the changes, let me know if this resolves your issue.

  /// get public descriptors
  Future<List<Descriptor>> getPublicDescriptors(String mnemonicStr) async {
    final descriptors = <Descriptor>[];
    try {
      for (var e in [KeychainKind.External, KeychainKind.Internal]) {
        final mnemonic = await Mnemonic.fromString(mnemonicStr);
        final descriptorSecretKey = await DescriptorSecretKey.create(
          network: Network.Testnet,
          mnemonic: mnemonic,
        );
        // Once the descriptorSecretKey is created, we can get the public descriptor
        // by calling asString() on it.
        final publicDesctriptor = await descriptorSecretKey.asString();
        // Now we can store the public descriptor in a database or persist in some
        // other way.

        // When we need the descriptor recreated we can use the Descriptor.create()
        // and use the string we got from asString()
        final descriptor = await Descriptor.create(
            descriptor: publicDesctriptor,
            network: Network.Testnet
        );
        descriptors.add(descriptor);
      }
      return descriptors;
    } on Exception catch (e) {
      setState(() {
        displayText = "Error : ${e.toString()}";
      });
      rethrow;
    }
  }

@BitcoinZavior Still wasn't successful, sorry. I made the PR with your code https://github.com/LtbLightning/bdk-flutter-quickstart/pull/2

  1. If you look at your example you will see that we don't use KeychainKind for (var e in [KeychainKind.External, KeychainKind.Internal]) {

  2. The lib contains the method

    ///Returns the public version of this key.
    ///
    /// If the key is an “XPrv”, the hardened derivation steps will be applied before converting it to a public key.
    Future<DescriptorPublicKey> asPublic() async {

    that I used in the first example, but when we got public keys with this method it also doesn't create the same wallet.

If you run the example in PR you can create a wallet, get a balance. After that create a public wallet and check the balance again.

thunderbiscuit commented 1 year ago

Can you post examples of the keys, mnemonic, and descriptors you are getting? I think seeing the strings might help us identify where your code goes wrong.

If you are at all familiar with Kotlin, take a look at this script. You should be able to reproduce the BIP-84 descriptors, both private and public.

Once you have the complete public descriptors (one for your internal and one for your external keychains), you can safely save those public descriptors and reload the wallet using them.

// {
//   "fingerprint": "9122d9e0",
//   "mnemonic": "fire alter tide over object advance diamond pond region select tone pole",
//   "xprv": "tprv8ZgxMBicQKsPf2qfrEygW6fdYseJDDrVnDv26PH5BHdvSuG6ecCbHqLVof9yZcMoM31z9ur3tTYbSnr1WBqbGX97CbXcmp5H6qeMpyvx35B"
// }

val descriptorSecretKey: DescriptorSecretKey = DescriptorSecretKey.fromString(
    "tprv8ZgxMBicQKsPf2qfrEygW6fdYseJDDrVnDv26PH5BHdvSuG6ecCbHqLVof9yZcMoM31z9ur3tTYbSnr1WBqbGX97CbXcmp5H6qeMpyvx35B"
)

val descriptor = Descriptor.newBip84(
    secretKey = descriptorSecretKey,
    keychain = KeychainKind.EXTERNAL,
    network = Network.TESTNET
)

println("The descriptor is ${descriptor.asStringPrivate()}")
// wpkh(tprv8ZgxMBicQKsPf2qfrEygW6fdYseJDDrVnDv26PH5BHdvSuG6ecCbHqLVof9yZcMoM31z9ur3tTYbSnr1WBqbGX97CbXcmp5H6qeMpyvx35B/84'/1'/0'/0/*)#yl0cyza0

println("The public descriptor is ${descriptor.asString()}")
// wpkh([9122d9e0/84'/1'/0']tpubDCYVtmaSaDzTxcgvoP5AHZNbZKZzrvoNH9KARep88vESc6MxRqAp4LmePc2eeGX6XUxBcdhAmkthWTDqygPz2wLAyHWisD299Lkdrj5egY6/0/*)#zpaanzgu
thunderbiscuit commented 1 year ago

I think one of your issues is that you're playing with DescriptorPublicKeys when in fact you might not need to use those at all directly. Upon first creating the descriptors you'll need to create DescriptorSecretKeys, and then use those to build full descriptors. From there simply use the public version of the descriptor. If I understand correctly your workflow, you will not need to interact with the DescriptorPublicKey directly at all.

awaik commented 1 year ago

're playing with DescriptorPublicKeys when in fact you might not need to use those at all directly. Upon first creating t

Safe wallets have to create Public descriptors without Secret. In this case, the user has full control over funds - not the app.

awaik commented 1 year ago

Can you post examples of the keys, mnemonic, and descriptors you are getting? I think seeing the strings might help us identify where your code goes wrong.

If you are at all familiar with Kotlin, take a look at this script. You should be able to reproduce the BIP-84 descriptors, both private and public.

Once you have the complete public descriptors (one for your internal and one for your external keychains), you can safely save those public descriptors and reload the wallet using them.

// {
//   "fingerprint": "9122d9e0",
//   "mnemonic": "fire alter tide over object advance diamond pond region select tone pole",
//   "xprv": "tprv8ZgxMBicQKsPf2qfrEygW6fdYseJDDrVnDv26PH5BHdvSuG6ecCbHqLVof9yZcMoM31z9ur3tTYbSnr1WBqbGX97CbXcmp5H6qeMpyvx35B"
// }

val descriptorSecretKey: DescriptorSecretKey = DescriptorSecretKey.fromString(
    "tprv8ZgxMBicQKsPf2qfrEygW6fdYseJDDrVnDv26PH5BHdvSuG6ecCbHqLVof9yZcMoM31z9ur3tTYbSnr1WBqbGX97CbXcmp5H6qeMpyvx35B"
)

val descriptor = Descriptor.newBip84(
    secretKey = descriptorSecretKey,
    keychain = KeychainKind.EXTERNAL,
    network = Network.TESTNET
)

println("The descriptor is ${descriptor.asStringPrivate()}")
// wpkh(tprv8ZgxMBicQKsPf2qfrEygW6fdYseJDDrVnDv26PH5BHdvSuG6ecCbHqLVof9yZcMoM31z9ur3tTYbSnr1WBqbGX97CbXcmp5H6qeMpyvx35B/84'/1'/0'/0/*)#yl0cyza0

println("The public descriptor is ${descriptor.asString()}")
// wpkh([9122d9e0/84'/1'/0']tpubDCYVtmaSaDzTxcgvoP5AHZNbZKZzrvoNH9KARep88vESc6MxRqAp4LmePc2eeGX6XUxBcdhAmkthWTDqygPz2wLAyHWisD299Lkdrj5egY6/0/*)#zpaanzgu

Thank you for the example. Do I understand correctly that in this example created wallet will have the rights to make BTC transactions?

And the question, can you take the string

println("The public descriptor is ${descriptor.asString()}")
wpkh([9122d9e0/84'/1'/0']tpubDCYVtmaSaDzTxcgvoP5AHZNbZKZzrvoNH9KARep88vESc6MxRqAp4LmePc2eeGX6XUxBcdhAmkthWTDqygPz2wLAyHWisD299Lkdrj5egY6/0/*)#zpaanzgu

and create a read-only wallet? To get transactions and addresses. But without the possibility to make a transition? That's what I'm trying to do and I can't.

awaik commented 1 year ago

OK, the final code is this, it creates a read-only wallet. Guys, thank you for the help, I think we can close it. If you would like I can PR a working example for https://github.com/LtbLightning/bdk-flutter-quickstart/ with some explanations for devs like me ;)

  /// get public descriptors
  Future<List<Descriptor>> getPublicDescriptors(String mnemonicStr) async {
    final descriptors = <Descriptor>[];
    try {
      for (var e in [KeychainKind.External, KeychainKind.Internal]) {
        final mnemonic = await Mnemonic.fromString(mnemonicStr);
        final descriptorSecretKey = await DescriptorSecretKey.create(
          network: Network.Testnet,
          mnemonic: mnemonic,
        );
        final descriptor = await Descriptor.newBip84(
          secretKey: descriptorSecretKey,
          network: Network.Testnet,
          keychain: e,
        );
        final String publicString = await descriptor.asString();
        final descriptorPublic = await Descriptor.create(
          descriptor: publicString,
          network: Network.Testnet,
        );
        descriptors.add(descriptorPublic);
      }
      return descriptors;
    } on Exception catch (e) {
      setState(() {
        displayText = "Error : ${e.toString()}";
      });
      rethrow;
    }
  }
BitcoinZavior commented 1 year ago

Hi @awaik Great that it worked out, I think the creation of the actual descriptor before extracting the public descriptor was missing from the original code in the app. Once added the wallet is able to restore correctly from the backed up public only descriptor, this commit shows the difference: https://github.com/LtbLightning/bdk-flutter-quickstart/commit/b5bf0645989fe65a849f3cfc3d90b9ce0d44ecc0

Yes, you can do a PR if you like with more explanation. I am closing the issue now as the problem is resolved.