Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a type safe descriptor builder #165

Closed
notmandatory opened this issue Jun 14, 2022 · 12 comments · Fixed by #260
Closed

Create a type safe descriptor builder #165

notmandatory opened this issue Jun 14, 2022 · 12 comments · Fixed by #260
Labels
discussion Issue requires some discussion first enhancement New feature or request

Comments

@notmandatory
Copy link
Member

notmandatory commented Jun 14, 2022

As discussed in the team chat with kirillzhukov, the bdk lib uses rust macros to provide a type safe way to construct valid descriptors. Since the macro approach won't work for bdk-ffi language bindings we should try to build a simple library, possibly as a new repo, that uses simple structs and enums to accomplish something similar to the bdk::descriptor::dsl module.

@notmandatory notmandatory moved this to Todo in BDK-Bindings Jun 14, 2022
@notmandatory notmandatory added enhancement New feature or request discussion Issue requires some discussion first labels Jun 14, 2022
@thunderbiscuit
Copy link
Member

Related to #98.

@notmandatory
Copy link
Member Author

Yes it's related to the static templates, they both should result in some Descriptor structure that can be used when constructing a Wallet.

@notmandatory
Copy link
Member Author

notmandatory commented Oct 19, 2022

Had a related discussion on Discord bdk-users today and think a simple descriptor template api could look like below. These represent simple single key descriptors according to BIP44,BIP49,BIP84, and BIP86.

There's no BIP yet for multisig templates, but we could create a simple one that takes a given key threshold and array of keys, would also need to a DescriptorKey enum.

[Enum]
interface DescriptorKey {
  Public(DescriptorPublicKey key);
  Secret(DescriptorSecretKey key);
};

interface Descriptor {
  [Throws=Error]
  constructor(string descriptor);

  [Name=new_bip44]
  constructor(DescriptorSecretKey secret_key, KeychainKind keychain , u32 account);

  [Name=new_bip44_public]
  constructor(DescriptorPublicKey public_key, string fingerprint, KeychainKind keychain , u32 account);

  [Name=new_bip49]
  constructor(DescriptorSecretKey secret_key, KeychainKind keychain , u32 account);

  [Name=new_bip49_public]
  constructor(DescriptorPublicKey public_key, string fingerprint, KeychainKind keychain , u32 account);
  
  [Name=new_bip84]
  constructor(DescriptorSecretKey secret_key, KeychainKind keychain , u32 account);
  
  [Name=new_bip84_public]
  constructor(DescriptorPublicKey public_key, string fingerprint, KeychainKind keychain , u32 account);
  
  [Name=new_bip86]
  constructor(DescriptorSecretKey secret_key, KeychainKind keychain , u32 account);
  
  [Name=new_bip86_public]
  constructor(DescriptorPublicKey public_key, string fingerprint, KeychainKind keychain , u32 account);

  [Name=new_multisig]
  constructor(sequence<DescriptorKey> keys, u32 threshold);

  string to_string();
};

@notmandatory
Copy link
Member Author

Related link with the types of descriptors other wallets use: https://github.com/nvk/walletsrecovery.org

@BitcoinZavior
Copy link
Contributor

This looks great 👍
Apart from the standard templates, we can have a custom descriptor option as well, so that a path and script type can be specified and the descriptor will be created for it.

  • This will take care of existing wallets created with non standard path.
  • It will also allow users to create wallets based on instructions from existing wallets which may not specify or inform the users of a descriptor nor a standard template but provide a path to use.
  • API to create a single XPub descriptor or a single key descriptor has been requested many times by users and having a custom descriptor option will take care of the request.

@notmandatory
Copy link
Member Author

notmandatory commented Oct 24, 2022

This looks great 👍 Apart from the standard templates, we can have a custom descriptor option as well, so that a path and script type can be specified and the descriptor will be created for it.

That's a great idea! But let's do it in a separate PR once we have the standard templates working ok? I'll think I'll move the multisig into it's own PR too since it might require more API discussion also.

@BitcoinZavior
Copy link
Contributor

That's a great idea! But let's do it in a separate PR once we have the standard templates working ok? I'll think I'll move the multisig into it's own PR too since it might require more API discussion also.

Sounds good 👍

@thunderbiscuit
Copy link
Member

@notmandatory mind if I take this on?

@thunderbiscuit
Copy link
Member

Now that I'm looking at the templates from BDK I see they only build for account 0 (if you want/need a different account you must build the descriptor manually).

@notmandatory
Copy link
Member Author

notmandatory commented Nov 21, 2022

Now that I'm looking at the templates from BDK I see they only build for account 0 (if you want/need a different account you must build the descriptor manually).

Good point, I agree for the initial api let's stick with just what the bdk templates give us, so account 0 only. Before we support different accounts I want to get that supported in thebdk templates.

@danielnordh
Copy link

Only here to say that I support this effort of adding 'ready to go' descriptor types.
It was one of the hurdles for me in getting started, I had to go ask in discord to figure it out.

In my own Swift code I only got to describe two so far:

public enum DescriptorType {
    case singleKey_wpkh84 // Native Segwit
    case singleKey_tr86 // Taproot
}

You have more detailed knowledge about a wider range of types so I don't have much to add, other than please name them in an approachable and descriptive way. (Or at least add comments in the code).

@notmandatory
Copy link
Member Author

notmandatory commented Nov 22, 2022

This is a good point about the naming. In BDK the templates are named by the bipxx + public added if they are for creating public key watch only descriptors, so that's what I am proposing for bdk-ffi. But we'll certainly need to add docs for anyone who's unfamiliar with the bips.

Repository owner moved this from Todo to Done in BDK-Bindings Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Issue requires some discussion first enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants