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

Add bip-psbt-descriptors #1548

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Conversation

seedhammer
Copy link

A BIP 174 PSBT may contain an extended key for deriving input and output
addresses. This document proposes an additional field for PSBTs to represent
arbitrary BIP 380 output script descriptors.

To support transfer of output descriptors outside signing flows, the proposal
makes the unsigned transaction optional.

@achow101
Copy link
Member

achow101 commented Jan 31, 2024

I don't think PSBT is the right way to be doing this. Descriptors are not necessary in PSBT, and relaxing the invariant that a PSBT represents a transaction (by allowing a PSBT to include no transaction data) would break many parsers and also just is antithetical to PSBT. It's not a general purpose format for sending data around - you could use protobuf if you wanted that.

If the goal is to have a machine parsable representation of descriptors, then by all means, do that. But it doesn't need to (nor do I think it should) interact with PSBT, other than having a different header that identifies it.

It's also unnecessary to have a descriptor in a PSBT. A PSBT can contain all of the data to derive it from the scriptPubKey at a later time. If an updater has the descriptor in order to put it in the PSBT, then it can also put the rest of the data in the PSBT so that the descriptor can be inferred. I just don't see how this proposal is at all useful.

Also, descriptors are a textual format, and encoding text in binary is just not very efficient. There are better ways to do that if you really wanted to.

@seedhammer
Copy link
Author

I don't think PSBT is the right way to be doing this. Descriptors are not necessary in PSBT, and relaxing the invariant that a PSBT represents a transaction (by allowing a PSBT to include no transaction data) would break many parsers and also just is antithetical to PSBT. It's not a general purpose format for sending data around - you could use protobuf if you wanted that.

If the goal is to have a machine parsable representation of descriptors, then by all means, do that. But it doesn't need to (nor do I think it should) interact with PSBT, other than having a different header that identifies it.

FWIW, the first draft was indeed PSBT with a different header: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-November/022184.html. https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-November/022186.html suggested we add descriptors to PSBT proper, which seemed like a good idea.

Would you support an alternative BIP that changed the header to make it intentionally different from PSBT files?

It's also unnecessary to have a descriptor in a PSBT. A PSBT can contain all of the data to derive it from the scriptPubKey at a later time. If an updater has the descriptor in order to put it in the PSBT, then it can also put the rest of the data in the PSBT so that the descriptor can be inferred.

How can the scriptPubKey infer a descriptor that contains, say, a key expression that ends in '/*'?

I just don't see how this proposal is at all useful.

The point of the BIP is to transfer descriptors between devices, in particular between a coordinator and (resource constrained) signing devices. It can be viewed as a codec for wallet policies.

Today, there exists a plethora of formats for exchanging descriptors, all with downsides:

  • Raw xpubs are underspecified (missing script type)
  • The BlueWallet textual format is inefficient (base58 xpubs) and doesn't support arbitrary descriptors.
  • Raw BIP-380 descriptors are also inefficient and lack metadata.
  • BCR-2023-010 are efficient but rely on the more complex (d)CBOR encoding.
  • JSON encoded textual descriptors are also inefficient and require a JSON implementation.

This proposal addresses them all:

  • Based on PSBT, signing devices need not spend additional code space on a separate codec.
  • Efficient encoding of extended keys.
  • Metadata (title, birth block) supported.
  • Every current or future bip-380 descriptor can be represented.

Also, descriptors are a textual format, and encoding text in binary is just not very efficient. There are better ways to do that if you really wanted to.

We believe it's hopeless to keep a binary representation of output descriptors up to date with the moving target that is textual bip-380 descriptors (BCR-2020-010 attempted but failed). This BIP (along with wallet policies) address the worst offender by encoding extended keys efficiently.

@achow101
Copy link
Member

achow101 commented Feb 9, 2024

Would you support an alternative BIP that changed the header to make it intentionally different from PSBT files?

That would be fine with me.

How can the scriptPubKey infer a descriptor that contains, say, a key expression that ends in '/*'?

Although /* cannot be directly inferred, you can still make an educated guess. It only affects the ends of derivation paths from xpubs, so the last derivation index of a key expression with an xpub could just be replaced with /*. Although descriptors allow for xpubs without a /*, it seems unlikely to me that that would actually be used in practice.

But I don't understand why you'd need to have the parent descriptor at all. You'll have enough key origin information to derive any private keys and be able to sign.

@0xGRAV3R

This comment was marked as spam.

0xGRAV3R

This comment was marked as spam.

@pythcoiner
Copy link

But I don't understand why you'd need to have the parent descriptor at all. You'll have enough key origin information to derive any private keys and be able to sign.

By example in the current ledger miniscript signing flow, the hardware device produce a Proof of Registration (PoR) against the parent descriptor that is stored by the softwarewallet (in order to keep stateless) the first time the descriptor is checked by user on signer side, then every time the software wallet send a psbt to sign to ledger device, it should also supply also the parent descriptor and PoR.

we previously discuss about this here

@pythcoiner
Copy link

cc @bigspider

@bigspider
Copy link
Contributor

Hardware signing devices need knowledge of the full descriptor/wallet policy in order to:

  • recognize change addresses
  • recognize from which of your different accounts (sometimes called wallets) the inputs are spending from.

(The second could perhaps be considered a strengthening of the traditional guarantees of hardware signers, not sure if it was ever formalized before wallet policies; I think it's essential in practice for anything that is not single-user)

While it's true that PSBTs today have enough info to sign, they do not have enough info to do it securely - the descriptor is also needed. Therefore, I agree that it would be a great improvement to be able to pass this info via PSBTs; currently, the Ledger app's signing flow is sign(PSBT, wallet_policy), and quite some work in the app goes into figuring out change/address_index for each input and change output based on the derivations in the PSBT.

However, I think that's a separate discussion, and it's unclear that recycling PSBTs for a purpose other than representing transactions brings any benefit over than directly designing a space-efficient encoding specifically for descriptors or wallet policies – if the <30% space saving that can be obtained with a re-encoding is deemed important in practice.

@jonatack
Copy link
Member

@seedhammer, do you plan to update here based on the review feedback?

@seedhammer
Copy link
Author

Thanks for the prod, @jonatack . I've updated the proposal so it's no longer a PSBT extension but a separate format merely based on PSBT key-value maps.

I'd like to add proof-of-registration (PoR), but I'm not sure how it's best done. There's some discussion at wizardsardine/liana#539 (comment), where @bigspider suggests a HMAC scheme and in a later post @pythcoiner suggest that the format of PoR field should be vendor defined. I'm still of the position that to be maximally useful and interoperable, the PoR field should have a defined format and be a signature, not a HMAC.

@pythcoiner
Copy link

I'd like to add proof-of-registration (PoR), but I'm not sure how it's best done. There's some discussion at wizardsardine/liana#539 (comment), where @bigspider suggests a HMAC scheme and in a later post @pythcoiner suggest that the format of PoR field should be vendor defined. I'm still of the position that to be maximally useful and interoperable, the PoR field should have a defined format and be a signature, not a HMAC.

As i already said in previous discussion, i do not have strong opinion about wether the PoR should be standardized or not, but i do not think it should be standardized in this BIP.

And anyway if i get it well both sheme (HMAC vs Signature) will be 32 bytes of data?

@pythcoiner
Copy link

it should be interesting to have insight from other signing devices teams.

cc @benma @scgbckbone @stepansnigirev @stepansnigirev @JamieDriver

@seedhammer
Copy link
Author

Also @kdmukai. Maybe others?

However, I think that's a separate discussion, and it's unclear that recycling PSBTs for a purpose other than representing transactions brings any benefit over than directly designing a space-efficient encoding specifically for descriptors or wallet policies – if the <30% space saving that can be obtained with a re-encoding is deemed important in practice.

I want to point out that while saving space is important to us, the ability to add metadata and eventually proof-of-registration is the more interesting feature in general.

@benma
Copy link

benma commented Apr 28, 2024

I am in agreement with @bigspider's comment above.

BitBox02 libraries have functions like sign_psbt(psbt, wallet_policy) (example), and the policy/descriptor is required for anything but simple single-sigs.

It would be helpful if the way the policy is described and transferred to signers was standardized so apps don't have to implement different ways to interface with different signers.

PSBT seems like the obvious location to put this information. Another format that contains both the PSBT and the policy, which is interoperable with all software, seems difficult to achieve and impractical.

@seedhammer
Copy link
Author

Given

  • PSBTs are useful without descriptors, and
  • descriptors are useful without PSBTs (backups, registration), and
  • the combination of a PSBT and descriptor is useful for more secure signing flows,

Perhaps it's feasible to keep the formats separate, but concatenate them at the transport layer (QR code, files), in a backwards compatible way? Say, append the decriptor at the end of the PSBT, where devices without knowledge of descriptors may parse the PSBT and ignore the rest?

Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have only lightly reviewed this document.

It sounds to me that there is still some discussion what solution is exactly to be pursued for this document. I’m going to change this to a draft PR for the moment, please set it ready to merge when the proposal is a bit more mature.

Please note that you will need a Backwards Compatibility section, and without going into the content in detail, it seems to me that the Rationale section could be a bit more extensive.

bip-bod-descriptors.mediawiki Outdated Show resolved Hide resolved
@murchandamus murchandamus marked this pull request as draft April 29, 2024 13:54
@seedhammer
Copy link
Author

Thanks, Murch. I've added the mandatory headers, added a compatibility section, and expanded the rationale section.

It seems to me that the main source of contention is whether the format should be separate or part of PSBT. While convenient to extend PSBT, it's probably best to separate the format if for nothing else than descriptors are useful outside signing flows.

Proof of registration can wait for a future extension BIP, as suggested by @pythcoiner.

@pythcoiner
Copy link

Thanks, Murch. I've added the mandatory headers, added a compatibility section, and expanded the rationale section.

It seems to me that the main source of contention is whether the format should be separate or part of PSBT. While convenient to extend PSBT, it's probably best to separate the format if for nothing else than descriptors are useful outside signing flows.

Proof of registration can wait for a future extension BIP, as suggested by @pythcoiner.

To be more accurate, i think how is processed the PoR should de defined in a separate BIP, but having an optional key-value pair defined in this BIP should be useful imho.

@seedhammer
Copy link
Author

Thanks, Murch. I've added the mandatory headers, added a compatibility section, and expanded the rationale section.
It seems to me that the main source of contention is whether the format should be separate or part of PSBT. While convenient to extend PSBT, it's probably best to separate the format if for nothing else than descriptors are useful outside signing flows.
Proof of registration can wait for a future extension BIP, as suggested by @pythcoiner.

To be more accurate, i think how is processed the PoR should de defined in a separate BIP, but having an optional key-value pair defined in this BIP should be useful imho.

Good point. Done.

@scgbckbone
Copy link
Contributor

I'm very much PRO descriptors directly IN PSBTs. Even tho I understand @achow101 comments.

Regardless of the format here (which i haven't studied) it is useful to have descriptor (or at least policy) in PSBT. Example from Coldcard is when you want to have multiple miniscripts registered on device that have identical keys in miniscript but policy/script is different. Or same keys same policy but order of keys in policy is different. Here you just cannot infer solely from PSBT which wallet to use because keys match for all wallets...

why not just use PSBT_GLOBAL_PROPRIETARY key and send descriptor in there?

@seedhammer
Copy link
Author

seedhammer commented May 1, 2024

I'm very much PRO descriptors directly IN PSBTs. Even tho I understand @achow101 comments.

Descriptors in PSBTs are useful, but descriptors are also useful outside PSBTs: transferring descriptors between wallets, (long term) backups. It would be unfortunate to specify a descriptor format that only solved the signing use-cases.

why not just use PSBT_GLOBAL_PROPRIETARY key and send descriptor in there?

That would hijack PSBT_GLOBAL_PROPRIETARY, leaving it no longer available for vendor-specific uses.

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few review suggestions follow for the BIP text.

bip-bod-descriptors.mediawiki Outdated Show resolved Hide resolved
bip-bod-descriptors.mediawiki Outdated Show resolved Hide resolved
bip-bod-descriptors.mediawiki Outdated Show resolved Hide resolved
bip-bod-descriptors.mediawiki Show resolved Hide resolved
bip-bod-descriptors.mediawiki Outdated Show resolved Hide resolved
bip-bod-descriptors.mediawiki Outdated Show resolved Hide resolved
bip-bod-descriptors.mediawiki Outdated Show resolved Hide resolved
bip-bod-descriptors.mediawiki Outdated Show resolved Hide resolved
bip-bod-descriptors.mediawiki Outdated Show resolved Hide resolved
bip-bod-descriptors.mediawiki Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.