Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 26 additions & 19 deletions doc/external-signer.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ Replace `<address>` with the result of `getnewaddress`.
Under the hood this uses a [PSBT (Partially Signed Bitcoin Transaction)](psbt.md).

```sh
bitcoin-cli -rpcwallet=<walletname> sendtoaddress <address> <amount>
bitcoin rpc -rpcwallet=<walletname> send outputs='{"<address>": <amount>}'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder why send and sendall is marked EXPERIMENTAL in the help, but used in the normal docs here.

Either it should be hidden, and EXPERIMENTAL, and not be referred to in the docs. Or it should be included normally, not be EXPERIMENTAL and can be referred to in the docs.

Somehow promoting an RPC call for use, but at the same discouraging it to be used seems contradictory.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe it's time to promote them to non-experimental at this point.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Proposed in #35601.

```

This constructs a PSBT and prompts your external signer to sign (will fail if it's not connected). If successful, Bitcoin Core finalizes and broadcasts the transaction.
Expand All @@ -84,13 +84,17 @@ Prerequisite knowledge:
* [Output Descriptors](descriptors.md)
* Partially Signed Bitcoin Transaction ([PSBT](psbt.md))

### Flag `--chain <name>` (required)
Comment thread
fanquake marked this conversation as resolved.
### Flag `--chain <name>` (required except for `enumerate`)

With `<name>` one of `main`, `test`, `signet`, `regtest`, `testnet4`.

Bitcoin Core passes this flag to commands that operate on a chain-specific signer, for example `getdescriptors`, `displayaddress` and `signtx`.

### Flag `--stdin` (required)

Indicate that (sub)command should be received over stdin and results returned in response to that. `--stdin` is a global flag, it is not used for all subcommands.
Indicate that (sub)command should be received over stdin and results returned in response to that. `--stdin` is a global flag, it is not specific to any subcommand.

Bitcoin Core currently uses this flag for `signtx`.

All subcommands SHOULD support both
- being called as commandline arguments; or
Expand All @@ -107,11 +111,11 @@ Usage:

Note: remember that shell-expansion is not available on _stdin_. Consequently, commands such as `signtx`, may write their arguments in either quoted or unquoted form.

### Flag `--fingerprint <fingerprint>` (required)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks

### Flag `--fingerprint <fingerprint>` (required except for `enumerate`)

With `<fingerprint>` being the hexadecimal 8-symbol identifier for a wallet.

Commands will specify a fingerprint as an identifier for external-signer wallet.
Bitcoin Core passes this flag to commands that operate on a specific external-signer wallet, for example `getdescriptors`, `displayaddress` and `signtx`.

### `enumerate` (required)

Expand All @@ -125,13 +129,15 @@ Usage:
[
{
"fingerprint": "00000000",
"name": "trezor_t"
"model": "trezor_t"
}
]
```

The command MUST return an array, possibly empty, of entries that contain at least a `fingerprint` field.

If present, the optional `model` field is used as the device name in the `enumeratesigners` RPC result.

A future extension could add an optional return field with device capabilities. Perhaps a descriptor with wildcards. For example: `["pkh("44'/0'/$'/{0,1}/*"), sh(wpkh("49'/0'/$'/{0,1}/*")), wpkh("84'/0'/$'/{0,1}/*")]`. This would indicate the device supports legacy, wrapped SegWit and native SegWit. In addition it restricts the derivation paths that can used for those, to maintain compatibility with other wallet software. It also indicates the device, or the driver, doesn't support multisig.

A future extension could add an optional return field `reachable`, in case `<cmd>` knows a signer exists but can't currently reach it.
Expand All @@ -153,27 +159,28 @@ The command MAY complain if `--chain` is set to a test-network, but any of the B
Usage:

```sh
<cmd> --fingerprint=<fingerprint> getdescriptors <account>
<xpub>
<cmd> --fingerprint <fingerprint> --chain <name> getdescriptors --account <account>
```

Returns descriptors supported by the device. Example:

```sh
<cmd> --fingerprint=00000000 getdescriptors
<cmd> --fingerprint 00000000 --chain main getdescriptors --account 0
```

```
{
"receive": [
"pkh([00000000/44h/0h/0h]xpub6C.../0/*)#fn95jwmg",
"sh(wpkh([00000000/49h/0h/0h]xpub6B..../0/*))#j4r9hntt",
"wpkh([00000000/84h/0h/0h]xpub6C.../0/*)#qw72dxa9"
"wpkh([00000000/84h/0h/0h]xpub6C.../0/*)#qw72dxa9",
"tr([00000000/86h/0h/0h]xpub6C.../0/*)#4d8tq2ns"
],
"internal": [
"pkh([00000000/44h/0h/0h]xpub6C.../1/*)#c8q40mts",
"sh(wpkh([00000000/49h/0h/0h]xpub6B..../1/*))#85dn0v75",
"wpkh([00000000/84h/0h/0h]xpub6C..../1/*)#36mtsnda"
"wpkh([00000000/84h/0h/0h]xpub6C..../1/*)#36mtsnda",
"tr([00000000/86h/0h/0h]xpub6C.../1/*)#d63h6jpt"
]
}
```
Expand All @@ -182,13 +189,13 @@ Returns descriptors supported by the device. Example:

Usage:
```sh
<cmd> --fingerprint=<fingerprint> displayaddress --desc <descriptor>
<cmd> --fingerprint <fingerprint> --chain <name> displayaddress --desc <descriptor>
```

Example, display the first native SegWit receive address on Testnet:

```sh
<cmd> --chain test --fingerprint=00000000 displayaddress --desc "wpkh([00000000/84h/1h/0h]tpubDDUZ..../0/0)"
<cmd> --fingerprint 00000000 --chain test displayaddress --desc "wpkh([00000000/84h/1h/0h]tpubDDUZ..../0/0)"
```

The command MUST be able to figure out the address type from the descriptor.
Expand All @@ -204,16 +211,16 @@ The command MAY complain if `--chain` is set to a test-network, but the BIP32 co

## How Bitcoin Core uses the Signer API

The `enumeratesigners` RPC simply calls `<cmd> enumerate`.
The `enumeratesigners` RPC calls `<cmd> enumerate`, skips duplicate entries with the same `fingerprint`, and maps the optional `model` field to the RPC `name` field.

Wallet operations that need a signer (`createwallet`, `walletdisplayaddress` and spending) also call `<cmd> enumerate` and fail unless exactly one signer is found, so only one device should be connected at a time.

The `createwallet` RPC calls:

* `<cmd> --chain <name> --fingerprint=00000000 getdescriptors --account 0`
* `<cmd> --fingerprint 00000000 --chain <name> getdescriptors --account 0`

It then imports descriptors for all supported address types, in a BIP44/49/84/86 compatible manner.

The `walletdisplayaddress` RPC reuses some code from `getaddressinfo` on the provided address and obtains the inferred descriptor. It then calls `<cmd> --fingerprint=00000000 displayaddress --desc=<descriptor>`.

For external-signer wallets, spending uses `send` or `sendall`. Bitcoin Core builds a PSBT, calls the signer via stdin with `signtx`, and if signatures are sufficient, finalizes and broadcasts the transaction. If the signer is not connected or cancels, the call fails with an error. For fee-bumping on such wallets, use `psbtbumpfee` to involve an external signer.
The `walletdisplayaddress` RPC obtains the inferred descriptor for the provided address. It then calls `<cmd> --fingerprint 00000000 --chain <name> displayaddress --desc <descriptor>`.

`sendtoaddress` and `sendmany` check `inputs->bip32_derivs` to see if any inputs have the same `master_fingerprint` as the signer. If so, it calls `<cmd> --fingerprint=00000000 signtransaction <psbt>`. It waits for the device to return a (partially) signed psbt, tries to finalize it and broadcasts the transaction.
For external-signer wallets, spending uses `send` or `sendall`, and fee-bumping uses `bumpfee`. Bitcoin Core builds a PSBT, adds key origin information, checks whether any input key origin fingerprint matches the signer, calls `<cmd> --stdin --fingerprint 00000000 --chain <name>`, and sends `signtx <psbt>` over stdin. If signatures are sufficient, it finalizes the transaction and, for broadcasting RPCs, broadcasts it. If signing cannot complete, the call fails with an error. For manual fee-bumping, use `psbtbumpfee` to obtain a PSBT for signing.
13 changes: 8 additions & 5 deletions src/external_signer.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class ExternalSigner
public:
//! @param[in] command the command which handles interaction with the external signer
//! @param[in] fingerprint master key fingerprint of the signer
//! @param[in] chain "main", "test", "regtest" or "signet"
//! @param[in] chain "main", "test", "signet", "regtest" or "testnet4"
//! @param[in] name device name
ExternalSigner(std::vector<std::string> command, std::string chain, std::string fingerprint, std::string name);

Expand All @@ -42,23 +42,26 @@ class ExternalSigner
//! Obtain a list of signers. Calls `<command> enumerate`.
//! @param[in] command the command which handles interaction with the external signer
//! @param[in,out] signers vector to which new signers (with a unique master key fingerprint) are added
//! @param chain "main", "test", "regtest" or "signet"
//! @param chain "main", "test", "signet", "regtest" or "testnet4"
//! @returns success
static bool Enumerate(const std::string& command, std::vector<ExternalSigner>& signers, const std::string& chain);

//! Display address on the device. Calls `<command> displayaddress --desc <descriptor>`.
//! Display address on the device. Calls `<command> --fingerprint <fingerprint> --chain <chain>
//! displayaddress --desc <descriptor>`.
//! @param[in] descriptor Descriptor specifying which address to display.
//! Must include a public key or xpub, as well as key origin.
UniValue DisplayAddress(const std::string& descriptor) const;

//! Get receive and change Descriptor(s) from device for a given account.
//! Calls `<command> getdescriptors --account <account>`
//! Calls `<command> --fingerprint <fingerprint> --chain <chain> getdescriptors
//! --account <account>`.
//! @param[in] account which BIP32 account to use (e.g. `m/44'/0'/account'`)
//! @returns see doc/external-signer.md
UniValue GetDescriptors(int account);

//! Sign PartiallySignedTransaction on the device.
//! Calls `<command> signtransaction` and passes the PSBT via stdin.
//! Calls `<command> --stdin --fingerprint <fingerprint> --chain <chain>` and passes the
//! `signtx` command and PSBT via stdin.
//! @param[in,out] psbt PartiallySignedTransaction to be signed
bool SignTransaction(PartiallySignedTransaction& psbt, std::string& error);
};
Expand Down
4 changes: 2 additions & 2 deletions src/rpc/external_signer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
static RPCMethod enumeratesigners()
{
return RPCMethod{"enumeratesigners",
"Returns a list of external signers from -signer.",
"Returns a list of external signers from -signer. Signers with duplicate master key fingerprints are skipped.",
{},
RPCResult{
RPCResult::Type::OBJ, "", "",
Expand All @@ -30,7 +30,7 @@ static RPCMethod enumeratesigners()
{RPCResult::Type::OBJ, "", "",
{
{RPCResult::Type::STR_HEX, "fingerprint", "Master key fingerprint"},
{RPCResult::Type::STR, "name", "Device name"},
{RPCResult::Type::STR, "name", "Device name, the model returned by the signer"},
}},
},
}
Expand Down
6 changes: 5 additions & 1 deletion src/wallet/rpc/spend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,11 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto
EnsureWalletIsUnlocked(wallet);

// This function is only used by sendtoaddress and sendmany.
// This should always try to sign, if we don't have private keys, don't try to do anything here.
// This should always try to sign, if we don't have (all) private keys, don't
// try to do anything here.
if (wallet.IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error: sendtoaddress and sendmany are not supported for wallets with external signers; use send instead");
}
if (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet");
}
Expand Down
15 changes: 15 additions & 0 deletions test/functional/wallet_signer.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,21 @@ def test_valid_signer(self):
result = hww.walletdisplayaddress(address)
assert_equal(result, {"address": address})

assert_raises_rpc_error(
-4,
"Error: sendtoaddress and sendmany are not supported for wallets with external signers; use send instead",
hww.sendtoaddress,
self.nodes[0].getnewaddress(),
0.01,
)
assert_raises_rpc_error(
-4,
"Error: sendtoaddress and sendmany are not supported for wallets with external signers; use send instead",
hww.sendmany,
"",
{self.nodes[0].getnewaddress(): 0.01},
)

# Handle error thrown by script
self.set_mock_result(self.nodes[1], "2")
assert_raises_rpc_error(-1, 'RunCommandParseJSON error',
Expand Down
Loading