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

HD Multisig derivation standard #1072

Merged
merged 25 commits into from Jul 20, 2021
Merged

HD Multisig derivation standard #1072

merged 25 commits into from Jul 20, 2021

Conversation

Fonta1n3
Copy link
Contributor

This PR is a proposal for a new BIP to define a standard on m/48' derivation paths used in modern day hierarchical deterministic multi-sig wallets.

@Fonta1n3 Fonta1n3 marked this pull request as draft February 24, 2021 13:41
@Fonta1n3 Fonta1n3 marked this pull request as ready for review February 24, 2021 14:14
@devrandom
Copy link
Contributor

stray file .DS_Store

@@ -84,7 +84,7 @@ Hardened derivation is used at this level.
===Script===

This level splits the key space into two separate <code>script_type</code>(s). To provide
backward and forward compatibility.
backward compatibility.
Copy link
Contributor

Choose a reason for hiding this comment

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

I have originally added the script_type level for forward compatibility. Specifically to avoid a BIP44/49/84 scenario, that is, to avoid creating a new BIP every time we wanted to use new type of scripts -- instead I thought it's easier to maintain a list of script types.

from spesmilo/electrum#4352 (comment) :

The advantage would be that when new script types come later (e.g. Schnorr-like sigs), no new BIP would be needed.

There is another solution, which is to e.g. hash a descriptor or similar of the script type template to arrive at a derivation path level; then a single BIP would be enough; though that might make disaster-recovery problematic -- but this too requires its own script_type level.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, arguably there is no backward compatibility provided at all. What exactly is this backward compatible with?
I guess you mean the wallets that are using this same spec before it was formally specced (here), but as you don't add anything new, just formalise it, that is just self-referential.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, I have added some simple text around future script types and extensibility in 8a3a8bd, does that work?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, thanks. I personally like this approach better; though I guess it might be subjective. (re new BIP every time vs maintaining list of script types.) I prefer the wording to express the original intention, to explain why it was done like this.

@Rspigler
Copy link
Contributor

I don't understand the purpose of this BIP. The stated goal is to not need a BIP for new script types, but you then still need to maintain a list of script types for the script_type path, so it's not actually reducing maintainability, and in doing so, the derivation path would then have the script type included, instead of just being in the descriptor, which is unnecessarily messy IMO. It seems to be a standard for describing current bad practices instead of a standard of how wallets should behave

@Fonta1n3
Copy link
Contributor Author

Fonta1n3 commented Mar 1, 2021

I don't understand the purpose of this BIP. The stated goal is to not need a BIP for new script types, but you then still need to maintain a list of script types for the script_type path, so it's not actually reducing maintainability, and in doing so, the derivation path would then have the script type included, instead of just being in the descriptor, which is unnecessarily messy IMO. It seems to be a standard for describing current bad practices instead of a standard of how wallets should behave

Hi Robert,

The stated goal is:

This PR is a proposal for a new BIP to define a standard on m/48' derivation paths used in modern day hierarchical deterministic multi-sig wallets.
The motivation of this BIP is to define the existing industry wide practice of utilizing m/48' derivation paths in hierarchical deterministic multi-sig wallets so that other developers may benefit from a standard. This BIP allows for future script types to easily be appended to the specification so that a new BIP is not required for every future script type.

The hierarchy proposed in this paper is quite comprehensive. It allows the handling of multiple accounts, external and internal chains per account, multiple script types and millions of addresses per chain.

This paper was inspired from BIP44.

It is absolutely for the purpose of describing existing standard that are in use across all modern segwit HD multisig wallets and also provides future extensibility.

@SomberNight is the original creator of the m/48' derivation. He had commented here so I amended the BIP to stay true to the original vision of allowing for future script types.

You can absolutely "just include a descriptor" (in your application) as descriptors include the derivation path. This BIP gives devs a standard to follow for which derivations they should use in their output descriptors for segwit (currently) multi-sig wallets. Similarly to what BIP44, 84, 49 achieve for single-sig.

@Rspigler
Copy link
Contributor

Rspigler commented Mar 1, 2021

Multisig wallets should be created using descriptors, which contain the script type, (sorted) multi, key origin identification, xpubs, and checksum. The descriptor should be created using parameters like 'M', 'N', script type, and by gathering keys from co-signers in the descriptor format of master key fingerprint, BIP44/49/84/Future_Tapscript derivation path, xpub. There's no reason to use BIP45, or to create a 'future proof' BIP45 ie BIP48.

@Fonta1n3
Copy link
Contributor Author

Fonta1n3 commented Mar 1, 2021

Multisig wallets should be created using descriptors, which contain the script type, (sorted) multi, key origin identification, xpubs, and checksum. The descriptor should be created using parameters like 'M', 'N', script type, and by gathering keys from co-signers in the descriptor format of master key fingerprint, BIP44/49/84/Future_Tapscript derivation path, xpub. There's no reason to use BIP45, or to create a 'future proof' BIP45 ie BIP48.

Other then the fact that many wallets do use bip45 and m/48' and yet we have do not have a standard for m/48' which is the purpose of this BIP.

Specter, Sparrow, Electrum, Coldcard, Trezor, Ledger, Fully Noded, Gordian Wallet, Nunchuk (and others) all use the m/48' derivation as it is the only derivation path we have for HD segwit multi-sig wallets. If you are using bip44/84/49 in your multisig descriptors then you are doing it wrong as those are single-sig derivs.

@Rspigler
Copy link
Contributor

Rspigler commented Mar 1, 2021

If you are using bip44/84/49 in your multisig descriptors then you are doing it wrong as those are single-sig derivs.

Yes, I meant in the same way BIP44 was done - with a BIP per script. I don't see why we should be encouraging mixing scripts and keys by creating a BIP45 with a script_type. There's also no reason to have a cosigner_index, since that requires that each cosigner send to each other their purpose' public key before being able to create their full derivation path of the key, which merely adds unnecessary communication rounds when the coordinator can use sortedmulti in the descriptor.

BIP48 doesn't have a cosigner_index, but if we want to create a HD Multisig derivation standard of how wallets should behave, rather than how they currently behave, we should remove script_type as well (and rename it to something different from BIP48).

@Fonta1n3
Copy link
Contributor Author

Fonta1n3 commented Mar 1, 2021

If you are using bip44/84/49 in your multisig descriptors then you are doing it wrong as those are single-sig derivs.

Yes, I meant in the same way BIP44 was done - with a BIP per script. I don't see why we should be encouraging mixing scripts and keys by creating a BIP45 with a script_type. There's also no reason to have a cosigner_index, since that requires that each cosigner send to each other their purpose' public key before being able to create their full derivation path of the key, which merely adds unnecessary communication rounds when the coordinator can use sortedmulti in the descriptor.

BIP48 doesn't have a cosigner_index, but if we want to create a HD Multisig derivation standard of how wallets should behave, rather than how they currently behave, we should remove script_type as well (and rename it to something different from BIP48).

As I've made extremely clear this is to define an existing standard that is already in use and also provides extensibility in case the community wants to extend upon it. Removingscript_type will cause "loss of funds" and break compatibility with existing multisig wallets.

If you want to define a standard for what "should" be used in HD multisig wallets and remove script_type then a separate BIP would be appropriate.

@doc-hex
Copy link

doc-hex commented Apr 19, 2021

Datapoint: Coldcard Wallet supports this BIP (always has) and as of v4.0.3 will support varied account numbers.

Copy link
Member

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

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

Please:

  1. drop the .DS_Store file.
  2. add a section on backward compatibility, especially with current use of purpose 48.
  3. follow the format described in BIP 2's "BIP format and structure".
  4. choose a copyright license and add the relevant metadata.

It might also be a good idea to document script type 0.

- Replace BIP number with ?
- Reduce title to less then 44 characters
- Add MIT Licence and copyright section
- Add specification section
- Add backwards compatibility section
@Fonta1n3
Copy link
Contributor Author

Please:

  1. drop the .DS_Store file.
  2. add a section on backward compatibility, especially with current use of purpose 48.
  3. follow the format described in BIP 2's "BIP format and structure".
  4. choose a copyright license and add the relevant metadata.

It might also be a good idea to document script type 0.

Hi Luke,

I went ahead and made the requested changes. If I did not do it correctly just let me know.

afaik script type 0 is not actually used as per @SomberNight (original creator of this derivation scheme), instead wallets use BIP45 for legacy multi-sig scripts.

@Rspigler
Copy link
Contributor

Strong NACK - Bitcoin is moving away from this (layer violations) and towards descriptors. This is also redundant with BIP 87

@craigraw
Copy link
Contributor

Strong NACK - Bitcoin is moving away from this (layer violations) and towards descriptors. This is also redundant with BIP 87

As pointed out above, this is simply documenting the existing system which is already widely used. Also, I think you misunderstand the process. For a BIP number to be assigned, the BIP proposal merely needs to be formatted correctly and technically sound (as in could be implemented). Clearly the latter is already true, therefore it is just a matter of the former.

@Rspigler
Copy link
Contributor

I understand the BIP process (for example, see BIP-101 and BIP-102). Anyone can make a proposal that won't be implemented in any project in a couple years. I just believe it's a better use of time to work forwards rather than backwards.

@luke-jr luke-jr merged commit ed35a41 into bitcoin:master Jul 20, 2021
Copy link

@Hbutlercapone Hbutlercapone left a comment

Choose a reason for hiding this comment

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

Approve merge request #1072

Copy link

@Hbutlercapone Hbutlercapone left a comment

Choose a reason for hiding this comment

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

Looks great,
Approve #1072

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet