Skip to content

Conversation

Sjors
Copy link
Member

@Sjors Sjors commented Sep 17, 2019

This adds 7 commits on top of #16546 (External signer support) to enable multisig functionality.

It introduces a new createmultisigwallet RPC which can also be used without external signer support (although it's only marginally easier than importing descriptors).

It automatically fetches xpubs from connected hardware devices (using HWI), constructs BIP67 multisig descriptors (native and wrapped SegWit) and imports them into a new wallet. For not connected external signers it's also possible to manually provide an xpub.

Use enumeratesigners to get a list of device fingerprints, and then call the new createmultisigwallet :

createmultisigwallet "wallet_name" threshold ["fingerprint","xpub1","xpub2",...] ( avoid_reuse )

Creates and loads a new multisig wallet.
Only native segwit bech32 addresses are supported.
Arguments:
1. wallet_name           (string, required) The name for the new wallet. If this is a path, the wallet will be created at the path location.
2. threshold             (numeric, required) Number of required signatures
3. signers               (json array, required) A json array of signers identified by their BIP32 fingerprint
     [
       "fingerprint",    (string, required) master key fingerprint. Can be obtained using emumeratesigners.
       "xpub1",          (string) the xpub at deriviation path m/48h/0h/0h/1h used for P2SH_SEGWIT, obtained automatically if -signer if configured
       "xpub2",          (string) the xpub at deriviation path m/48h/0h/0h/2h used for native SegWit, obtained automatically if -signer if configured
       ...
     ]
4. avoid_reuse           (boolean, optional, default=false) Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind.

Result:
{
  "name" :    <wallet_name>,        (string) The wallet name if created successfully. If the wallet was created using a full path, the wallet_name will be the full path.
  "warning" : <warning>,            (string) Warning message if wallet was not loaded cleanly.
}

Examples:
> bitcoin-cli createmultisigwallet "ManualMultisigWallet" 2 '[{"fingerprint": "d34db33f", "xpub2": "xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY"}, {"fingerprint": "3442193e", "xpub1": "xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8"}]'
> bitcoin-cli createmultisigwallet "AutomaticMultisigWallet" 2 '[{"fingerprint": "d34db33f"}, {"fingerprint": "3442193e"}]'
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "createmultisigwallet", "params": ["ManualMultisigWallet", 2, '[{"fingerprint": "d34db33f", "xpub2": "xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY"}, {"fingerprint": "3442193e", "xpub1": "xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8"}]'] }' -H 'content-type: text/plain;' http://127.0.0.1:8332/
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "createmultisigwallet", "params": ["AutomaticMultisigWallet", 2, '[{"fingerprint": "d34db33f"}, {"fingerprint": "3442193e"}]'] }' -H 'content-type: text/plain;' http://127.0.0.1:8332/

It uses the getxpub HWI method instead of getdescriptors, and constructs the descriptor locally.

TODO:

  • displayaddress support for all connected devices (only ColdCard can handle this afaik, but requires producing a Multisig.txt file)
  • wait for improved change address detection support, or add strong warning

See also: Junction or Specter Desktop for similar workflows.

Change detection

Note that due to limitations in HWI, hardware wallet firmware and the PSBT format, the current multisig flow is pretty unsafe. This is because change detection generally doesn't work.

On ColdCard you can put a multisig.txt file to enable change detection:

Name: My-2-of-2
Policy: 2 of 2
Derivation: m/48h/0h/0h/2h

A1A1A1A1: xpub...
B1B1B1B1: xpub...

@Sjors
Copy link
Member Author

Sjors commented Sep 17, 2019

cc @justinmoon @stepansnigirev thoughts on derivation paths?

@stepansnigirev
Copy link
Contributor

Using m/48h/0h/0h/1h derivation path by default is ok, but I would like to be able to change the derivation path in options. Also it would be nice to provide xpub without fingerprint, then fingerprint of the xpub and m/ as derivation path could be used, but not sure if it makes sense.

[
       "fingerprint",   (string, required) master key fingerprint. Can be obtained using emumeratesigners.
       "derivation",    (string) deriviation path for the key, m/48h/0h/0h/1h by default
       "xpub",          (string) the xpub, obtained automatically if -signer if configured
]

@Sjors
Copy link
Member Author

Sjors commented Oct 29, 2019

It now uses sortedmulti() from #17056 and creates both native segwit and P2SH wrapped SegWit descriptors. This should match the derivation paths used by Electrum and ColdCard: spesmilo/electrum#4465

You can test this with HWI, a ColdCard simulator plus one other wallet, e.g. Trezor (Simulator) or Ledger. After you called createmultisigwallet, you need to create a multisig text file for the ColdCard simulator in work/MicroSD/multisig.txt, using fingerprints and xpubs used in the wallet. You can see those in the descriptor, by generating an address and calling getaddressinfo.

Name: My-2-of-2
Policy: 2 of 2
Derivation: m/48h/0h/0h/2h

A1A1A1A1: tpub...
B1B1B1B1: tpub...

Now you can send and receive, even with the GUI. Note that HWI automatically clicks Yes on the ColdCard simulator screen to sign....

@Sjors Sjors force-pushed the 2019/09/hww-multisig branch 2 times, most recently from 7108856 to 2c5c803 Compare October 29, 2019 20:02
@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 30, 2019

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #18052 (Remove false positive GCC warning by hebasto)
  • #18034 (Get the OutputType for a descriptor by achow101)
  • #18032 (Output a descriptor in createmultisig and addmultisigaddress by achow101)
  • #18027 ("PSBT Operations" dialog by gwillen)
  • #17877 (qt, refactor: Make enums in BitcoinUnits class scoped by hebasto)
  • #17786 (refactor: Nuke policy/fees->mempool circular dependencies by hebasto)
  • #17681 (wallet: Keep inactive seeds after sethdseed and derive keys from them as needed by achow101)
  • #17566 (Switch to weight units for all feerates computation by darosior)
  • #16722 (build: Disable warnings for leveldb subtree by default by hebasto)
  • #16681 (Tests: Use self.chain instead of 'regtest' in all current tests by jtimon)
  • #16528 (Native Descriptor Wallets using DescriptorScriptPubKeyMan by achow101)
  • #16440 (BIP-322: Generic signed message format by kallewoof)
  • #16432 (qt: Add privacy to the Overview page by hebasto)
  • #16367 (Multiprocess build support by ryanofsky)
  • #16365 (Log RPC parameters (arguments) if -debug=rpcparams by LarryRuane)
  • #16224 (gui: Bilingual GUI error messages by hebasto)
  • #15590 (Descriptor: add GetAddressType() and IsSegWit() by Sjors)
  • #15382 (util: add runCommandParseJSON by Sjors)
  • #15112 (build: Optionally enable -Wzero-as-null-pointer-constant by Empact)
  • #14920 (Build: enable -Wdocumentation via isystem by Empact)
  • #11413 ([wallet] [rpc] sendtoaddress/sendmany: Add explicit feerate option by kallewoof)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@Sjors Sjors force-pushed the 2019/09/hww-multisig branch 2 times, most recently from 87d8b35 to b58c124 Compare October 31, 2019 10:47
@Sjors Sjors force-pushed the 2019/09/hww-multisig branch from b58c124 to bb1c542 Compare November 7, 2019 18:46
@Rspigler Rspigler mentioned this pull request Nov 8, 2019
2 tasks
@Sjors Sjors force-pushed the 2019/09/hww-multisig branch from bb1c542 to c12613e Compare December 9, 2019 16:58
@Sjors Sjors force-pushed the 2019/09/hww-multisig branch from 1fa08cb to 3fda68e Compare January 30, 2020 18:06
@Sjors
Copy link
Member Author

Sjors commented Feb 19, 2020

I'll look into this again when there's more progress on a standard for multisig wallet coordination, see #18142.

@Rspigler
Copy link
Contributor

Hi @Sjors - see BSMS for a standard for multisig wallet coordination

@Rspigler
Copy link
Contributor

Should this be reopened?

@bitcoin bitcoin deleted a comment from gremen1918 Jun 26, 2021
@Sjors
Copy link
Member Author

Sjors commented Jun 26, 2021

I have some ideas on how to tackle this, but no need to reopen this PR. First step is #22341

@Sjors Sjors deleted the 2019/09/hww-multisig branch November 18, 2021 15:35
@bitcoin bitcoin locked and limited conversation to collaborators Nov 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants