bips icon indicating copy to clipboard operation
bips copied to clipboard

BIP 388: Wallet Policies for Descriptor Wallets

Open bigspider opened this issue 3 years ago • 22 comments

Initial version posted to bitcoin-dev in May.

Wallet policies are implemented in the Ledger bitcoin app (since version 2.1.0).

The following PR experimenting with an HWI integration might provide more context and an end-to-end demo:

  • https://github.com/bitcoin-core/HWI/pull/647

bigspider avatar Nov 16 '22 15:11 bigspider

In dfa26453fb1975c0c8b7909e8237a5ac55f4dd8c, I added the possibility of using unhardened derivation steps in the key origin information (while keeping the restriction for change/address_index in the key placeholders). While most wallets use hardened derivations for all but the last two steps, there are deployed use cases with unhardened derivations, and this only adds negligible implementation complexity to wallet policies.

bigspider avatar Jan 24 '23 09:01 bigspider

Thanks @jgriffiths and @bucko13 for the reviews! I addressed most of the comments in da3e117d075f4428f14d4120124d840682e5587d.

In bb98f8017a883262e03127ab718514abf4a5e5f9 I deleted the examples coming from HTLC miniscripts; they don't quite make sense as wallet policies.

bigspider avatar Feb 21 '23 08:02 bigspider

@luke-jr can this be assigned a BIP number?

darosior avatar Aug 09 '23 16:08 darosior

After @benma suggestion I propose to merge my recent proposal https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-September/021946.html with this standard.

Specifically, I think we can remove unnecessary remaining ambiguity, when multiple keys need to repeat the same data (BIP44 purpose, coin, change derivation path segments) and make descriptors even shorter, more readable and standards, removing key list as a separate structure.

What can be done in this regard:

  • Descriptor is equipped with prefix and suffix containing information to reconstruct all shared key components: wsh/testnet(...)/<0;1>/*#checksum
  • Keys do not list shared components, skipping them: [f149e757//0h]xpub (where 0h is the account number and "terminal derivation" from the suffix is appended to all keys, such that they all follow uniform derivation);
  • Instead of a separate key list the keys go into the descriptor referenced in full only the first time they appear;
  • Each key has an alphanumeric alias, separated using @: alice@[f149e757//0h]xpub; the same key can be referred to in other descriptor places as @alice;
  • Keys in different spending paths, when necessary (since in Taproot it is not necessary) are distinguished by an additional derivation path segment called "branch". It is a non-hardened index going before the change index: alice@[f149e757//0h]xpub/1 (which after expansion with prefix and suffix becomes [f149e757/89h/1h/0h]xpub/1/<0;1>/*;

Overall, with these changes, the descriptors will look like

wsh/test(or(
    and(alice@[fe569a81//1']xpub1..., bob@[8871bad9//1']xpub2..., carol@[beafcafe//1']xpub3...), 
    and(older(1000), thresh(2, @alice, @bob, @carol))
))/<0;1>/*

The benefits of the proposal are:

  • Remove separate key list;
  • Use human-readable aliaces;
  • Avoid conflicting descriptors using different BIP44 purposes, networks or terminal indexes;
  • Separate from previous descriptor standard using new BIP44 purpose value;
  • Do not assign new meaning for the change index and make the standard compatible with RGB protocol (which uses change index for specifying derivations containing assigned client-side state or tapret commitments). This also makes wallet implementation easier, helping to clearly understand which indexes to use when deriving external or internal (change) addresses.

I can work on a PR to this PR to put this proposals into the text.

dr-orlovsky avatar Sep 11 '23 06:09 dr-orlovsky

Remove separate key list;

The separate key list is very nice though. On the hardware wallets, being able to verify the descriptor without all the xpub/fingerprint/path-prefix noise is much better UX and I think the main purpose of this BIP.

benma avatar Sep 11 '23 07:09 benma

  • Descriptor is equipped with prefix and suffix containing information to reconstruct all shared key components: wsh/testnet(...)/<0;1>/*#checksum

In my mind including the network identifier outside of the xpub is good from a global pov but bad on the UX pov: you asking all signer vendors/user to change they way of export xpubs.....

  • Instead of a separate key list the keys go into the descriptor referenced in full only the first time they appear;

I personally feel that a separate list of keys is more clear/clean to read/use

  • Each key has an alphanumeric alias, separated using @: alice@[f149e757//0h]xpub; the same key can be referred to in other descriptor places as @alice;

In my mind the key should have a numeric alias, this numeric alias can be replaced by some kind of alphanumeric 'mnemonic' by the hardware that handles the display

  • Keys in different spending paths, when necessary (since in Taproot it is not necessary) are distinguished by an additional derivation path segment called "branch". It is a non-hardened index going before the change index: alice@[f149e757//0h]xpub/1 (which after expansion with prefix and suffix becomes [f149e757/89h/1h/0h]xpub/1/<0;1>/*;

The 'branch' should be allowed with @, it's needed for actual miniscripts.

Looks like you forgot to talk about not limit (or increase limit) the number of change segments to allow you to use <0;1;9;10> with RGB

pythcoiner avatar Sep 11 '23 07:09 pythcoiner

@benma:

The separate key list is very nice though. On the hardware wallets, being able to verify the descriptor without all the xpub/fingerprint/path-prefix noise is much better UX and I think the main purpose of this BIP.

You are right that displaying that in the UI is terrible. Instead, my understanding is that the descriptor will be parsed by the wallet and the keys and policy will be present in the UI in separate form

@pythcoiner:

I personally feel that a separate list of keys is more clear/clean to read/use

I am not insisting on merging them; however, to import/export descriptors and pass them between software wallets it will be desirable to have a joined form.

In my mind including the network identifier outside of the xpub is good from a global pov but bad on the UX pov: you asking all signer vendors/user to change they way of export xpubs.....

No, the xpubs are exported the same way. However, when they are used in the descriptor, their shared parts are moved to prefix/suffix, ensuring they can be combined into the same descriptor (i.e. do not belong to different networks, which is hard to check otherwise).

In my mind the key should have a numeric alias, this numeric alias can be replaced by some kind of alphanumeric 'mnemonic' by the hardware that handles the display

Putting additional requirements on having ints and having them in a strictly incremental manner just bloats validation code with no clear benefits

The 'branch' should be allowed with @, it's needed for actual miniscripts.

Optionally. If not provided with a branch, then each time the key appears the branch number is automatically incremented. This reduces the visual load and size of the descriptor validation code (you can't write it in the wrong way).

dr-orlovsky avatar Sep 11 '23 08:09 dr-orlovsky

No, the xpubs are exported the same way. However, when they are used in the descriptor, their shared parts are moved to prefix/suffix, ensuring they can be combined into the same descriptor (i.e. do not belong to different networks, which is hard to check otherwise).

so it's mean the key/xpub a user supply to a wallet/coordinator will look differently than the one in the descriptor, how they can check accurately?

Putting additional requirements on having ints and having them in a strictly incremental manner just bloats validation code with no clear benefits

should we allow arabic/chinese/japanese/koreans/cyrillic/.... alphabets? the benefit i found is just KISS, let the fancy stuff optionnaly on the display side, not on the communication side.

Optionally. If not provided with a branch, then each time the key appears the branch number is automatically incremented. This reduces the visual load and size of the descriptor validation code (you can't write it in the wrong way).

i wonder if the order the key are passed to the miniscript 'compiler_that_is_not_compiler' are the same order in the output descriptor?

pythcoiner avatar Sep 11 '23 09:09 pythcoiner

@dr-orlovsky I feel like your suggestion would be better as a separate BIP as it appears to have slightly different goals and tradeoffs than this one. I don't think it would be feasible to merge these two proposals without morphing one into the other. If you do this and link it here I'd be happy to provide feedback on that issue or PR.

jgriffiths avatar Sep 11 '23 21:09 jgriffiths

  • Descriptor is equipped with prefix and suffix containing information to reconstruct all shared key components: wsh/testnet(...)/<0;1>/*#checksum
  • Keys do not list shared components, skipping them: [f149e757//0h]xpub (where 0h is the account number and "terminal derivation" from the suffix is appended to all keys, such that they all follow uniform derivation);
  • Instead of a separate key list the keys go into the descriptor referenced in full only the first time they appear;
  • Each key has an alphanumeric alias, separated using @: alice@[f149e757//0h]xpub; the same key can be referred to in other descriptor places as @alice;
  • Keys in different spending paths, when necessary (since in Taproot it is not necessary) are distinguished by an additional derivation path segment called "branch". It is a non-hardened index going before the change index: alice@[f149e757//0h]xpub/1 (which after expansion with prefix and suffix becomes [f149e757/89h/1h/0h]xpub/1/<0;1>/*;

The proposed changes undo most of the design choices of this BIP proposal (like separating the actual keys from the "descriptor template" − something I consider a core feature and benefit!), while making the resulting language a lot more incompatible with output descriptor; moreover, it would make parsing a lot more complicated, as parsing a key expression is no longer context-free, because aliases are back-references to the previously-parsed part of the string.

In wallet policies, aliases for keys could easily be introduced as additional metadata associated to the elements of the keys information vector.

This BIP proposal wants to model in the most minimal way the object that software/hardware wallets (and their users) think of as "accounts". Application-specific use cases could be implemented either by building on top of it (if there is a desire / need to stay compatible), or with completely independent approaches.

bigspider avatar Sep 12 '23 08:09 bigspider

@jgriffiths:

I feel like your suggestion would be better as a separate BIP as it appears to have slightly different goals and tradeoffs than this one.

That was what I began with:

After @benma suggestion I propose to merge my recent proposal https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-September/021946.html with this standard.

and was suggested to merge into this standard

Anyway, I see that it doesn't fit the hardware wallet devs, so I rest my case. The only part which is essential is to remember that RGB and other client-side-validation may require more derivation indexes in the change segment.

dr-orlovsky avatar Sep 13 '23 17:09 dr-orlovsky

RGB and other client-side-validation may require more derivation indexes in the change segment.

If this doesn't fit the existing patterns or the single derivation case (for example if the solved cardinality of such descriptors is > 2, and/or they contain more than one multi-path expression), I think this can be added as an optional extension as the single derivation case (non-bip44-style) was. As long as every key expression in a policy has the same solved cardinality this should work (disclaimer: I'm not following RGB development).

jgriffiths avatar Sep 25 '23 20:09 jgriffiths

@bigspider Policy support for the Jade HWW is now released as of firmware version 1.0.24 (https://github.com/Blockstream/Jade/releases/tag/1.0.24), if you'd like to update the Reference Implementation section.

Jade support is implemented via libwally-core (C/C++/Python/Java/JS) v1.0.0 (https://github.com/ElementsProject/libwally-core/releases/tag/release_1.0.0) if you want to link a general purpose implementation. Both Jade and wally implement the single-path derivation /* extension for non-bip44 wallets.

From my POV review is otherwise complete, ACK for this to receive a BIP number and be merged.

jgriffiths avatar Nov 20 '23 10:11 jgriffiths

Thanks for the update, @jgriffiths! I added Jade/libwally support in 3c97885, and did a couple of small improvements to the python demo.

bigspider avatar Nov 21 '23 10:11 bigspider

@luke-jr, please let me know if this can be assigned a BIP number, or if something is missing.

There are now three hardware signers already using these specs in production.

bigspider avatar Nov 21 '23 10:11 bigspider

I squashed the history, rebased and updated BIP number and type in 3bb2cfe4751313470b98c9aaaff9322078fb8e3c d4c650bad3ac190c9106d059b1ed5b110f2660bb.

bigspider avatar Jan 08 '24 10:01 bigspider

@luke-jr, is there any other further change that I missed?

Otherwise, it would be great to have this merged so I can start linking to it in docs.

bigspider avatar Feb 05 '24 16:02 bigspider

@luke-jr could you merge this when you've got a minute?

darosior avatar Apr 02 '24 13:04 darosior

I also noticed that this BIP does not have a Rationale section, although it seems to me that the rationale is sufficiently covered in Motivation and Specification (and therefore no change is necessary in that regard).

murchandamus avatar Apr 22 '24 21:04 murchandamus

Yes, it's nice that the check is working now. Here's the entry it suggests:

+> | [[bip-0388.mediawiki|388]]
+> | Applications
+> | Wallet Policies for Descriptor Wallets
+> | Salvatore Ingala
+> | Standard
+> | Draft
+> |-

jonatack avatar May 06 '24 15:05 jonatack

Rebased, and addressed the final comments (thank you!). Please let me know if you prefer me to squash the new commits.

bigspider avatar May 07 '24 09:05 bigspider

Thanks @murchandamus for the thorough suggestions! I accepted almost all directly, and slightly modified the remaining ones in 7d0c08e38acac3ef14095d0e8664c7332b7be381.

bigspider avatar May 07 '24 20:05 bigspider