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

Implement segwit multisig for bip39/hw wallets #4352

Closed
SomberNight opened this issue May 9, 2018 · 7 comments
Closed

Implement segwit multisig for bip39/hw wallets #4352

SomberNight opened this issue May 9, 2018 · 7 comments
Labels
enhancement ✨ topic-wallet 👛 related to wallet.py, or maybe address_synchronizer.py/coinchooser.py

Comments

@SomberNight
Copy link
Member

SomberNight commented May 9, 2018

We currently use m/45'/0 as derivation path for legacy multisig for hardware wallets. This path is based on BIP45 but we don't actually implement the full spec of that (the "0" in the path is a hardcoded "cosigner index" that all cosigners use).

electrum/lib/base_wizard.py

Lines 271 to 274 in 7c6364c

if self.wallet_type=='multisig':
# There is no general standard for HD multisig.
# This is partially compatible with BIP45; assumes index=0
self.on_hw_derivation(name, device_info, "m/45'/0")

A new derivation path would be needed for p2wsh-p2sh multisig, and another for p2wsh multisig, to avoid key reuse.

One approach could be to write two new BIPs, as was done with BIP49 and BIP84, that each standardise a BIP43 purpose, one for p2wsh-p2sh multisig, and one for p2wsh multisig. These BIPs could refer back to BIP45 and their sole purpose would essentially just be what the purpose of BIP49/84 is, to get a new constant for "purpose".
So e.g. p2wsh-p2sh multisig could use m/XX'/0 and p2wsh multisig could use m/YY'/0.

Alternatively, given BIP45 does not use the BIP44 HD structure anyway, there could a be a single BIP that adds a new level script_type', to cover p2wsh-p2sh multisig and p2wsh multisig both.
So the BIP45 structure could be adjusted to m / purpose' / script_type' / cosigner_index / change / address_index.
And e.g. p2wsh-p2sh multisig could use m/XX'/0'/0 and p2wsh multisig could use m/XX'/1'/0.
The advantage would be that when new script types come later (e.g. Schnorr-like sigs), no new BIP would be needed.

To be clear, something like BIP45 would be nice, but as with the legacy scripts currently, Electrum does not plan to support the full features of BIP45, so what would be needed is just a "standardised" derivation path.

Also, are there any wallets currently that support segwit multisig for hardware wallets? Of course we should then look at what they do. Bitpay wrote BIP45 but AFAICT Copay still does not support segwit https://github.com/bitpay/copay/issues/4020, and in fact they might no longer even be using BIP45 themselves: https://github.com/bitpay/copay/issues/3729#issuecomment-372091967.

@SomberNight
Copy link
Member Author

Bitgo has segwit multisig. Their paths are not following BIP43:

BitGo wallets currently are hard-coded with their root at m/0/0 across all 3 keychains (however, older legacy wallets may use different key paths). Below the root, the wallet supports four chains of addresses, 0 and 1, as well as 10 and 11. The 0-chain is for external receiving addresses, while the 1-chain is for internal (change) addresses. The 10-chain is for external SegWit receiving addresses, while the 11-chain is for internal SegWit (change) addresses.
The first receiving address of a wallet is at the BIP32 path m/0/0/0/0, which is also the ID used to refer to a wallet in BitGo’s system. The first change address of a wallet is at m/0/0/1/0. Correspondingly, the first SegWit receiving address of a wallet is at m/0/0/10/0, and the first SegWit change address of a wallet is at m/0/0/11/0.

@SomberNight
Copy link
Member Author

Maybe there should be a level for "coin" too, as in BIP44, so that testnet and mainnet use different keys.

@jhoenicke
Copy link
Contributor

Hi @SomberNight,

if you have a new BIP standard, please add the "coin" derivation step like in bip-44 or the unpublished bip-48. Preferably at the second position to keep it uniform. You want to have different keys for different coins.

I think one reason BIP-49 and BIP-84 didn't use a script_type field was that the author wanted that every bip-49 compatible wallet can always autodetect all accounts. For multisig accounts, there is no way to autodetect any accounts, so I think this reason doesn't apply. Also you can always say it is bip-xx compatible with all segwit script types or with Schnorr signatures.

Using the chain id for different scripts is worth a consideration, if you want to have several script types in the same wallet (e.g. have bech32 and non-segwit addresses in the same wallet and if someone doesn't understand the bech32 address you can give him the legacy address instead). We thought about this for bip49, but the huge drawback is that if the wallet doesn't support all script types, it would recover only part of the funds without giving the user any warning.

@spesmilo spesmilo deleted a comment from CJorwinJ May 10, 2018
@SomberNight
Copy link
Member Author

I was not aware of "bip-48", thanks.

https://github.com/bitpay/copay/blob/fb0a5ca237e86d8a695758eca3ea968a3e3b2b99/README.md#wallet-export-format

Since version 1.5, Copay uses the root m/48' for hardware multisignature wallets. This was coordinated with Ledger and Trezor teams. While the derivation path format is still similar to BIP44, the root of what is in the process of doing so are not discoverable. Address generation for multisignature wallets requires the other copayers extended public keys.

I guess we could do the same; have two new BIPs to allocate the purpose constants and use the bip44 structure. Although if there was a level/step for cosigner_index, that would leave the future possibility of implementing that (as in bip45) in a backwards compatible way.

@SomberNight
Copy link
Member Author

I think one reason BIP-49 and BIP-84 didn't use a script_type field was that the author wanted that every bip-49 compatible wallet can always autodetect all accounts. For multisig accounts, there is no way to autodetect any accounts, so I think this reason doesn't apply.

the unpublished bip-48

Given bip-48 is an unpublished spec for a derivation path for legacy multisig accounts; and as you mentioned, auto-detection of accounts is not possible anyway; and considering there does not seem to be many projects interested in bip39 multisig; one option to consider is to just piggyback this on top of bip-48:

instead of

m / purpose' / coin_type' / account' / change / address_index

we could do

m / purpose' / coin_type' / account' / script_type' / change / address_index

(with purpose=48)

@ecdsa

@SomberNight
Copy link
Member Author

@benma: was talking about this ^ today

@SomberNight
Copy link
Member Author

#4465 was merged which lets the user choose both derivation path and script type; with the default path for segwit multisig using the scheme from #4352 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ topic-wallet 👛 related to wallet.py, or maybe address_synchronizer.py/coinchooser.py
Projects
None yet
Development

No branches or pull requests

2 participants