foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00
Code Issues Activity

Merge #21666: Miscellaneous external signer changes

Browse source 
c8f469c6d5 external_signer: remove ExternalSignerException (fanquake)
9e0b199b97 external_signer: use const where appropriate (fanquake)
aaa4e5a45b wallet: remove CWallet::GetExternalSigner() (fanquake)
06a0673351 external_signer: remove ignore_errors from Enumerate() (fanquake)
8fdbb899b8 refactor: unify external wallet runtime errors (fanquake)
f4652bf125 refactor: add missing includes to external signer code (fanquake)
54569cc6d6 refactor: move all signer code inside ENABLE_EXTERNAL_SIGNER #ifdefs (fanquake)

Pull request description:

  These are a few followups after #21467.

ACKs for top commit:
  Sjors:
    tACK c8f469c6d5
  instagibbs:
    utACK c8f469c6d5

Tree-SHA512: 3d5ac5df81680075e71e0e4a7595c520d746c3e37f016cf168c1e10da15541ebb1595aecaf2c08575636e9ff77d499644cae53180232b7049cfae0b923106e4e
This commit is contained in:
fanquake 2021-04-14 09:36:41 +08:00
commit e7af2f35af
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
8 changed files with 49 additions and 55 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
src/external_signer.cpp
View file

@ -9,43 +9,44 @@
#include <util/system.h>
#include <external_signer.h>
ExternalSigner::ExternalSigner(const std::string& command, const std::string& fingerprint, std::string chain, std::string name): m_command(command), m_fingerprint(fingerprint), m_chain(chain), m_name(name) {}
#include <stdexcept>
#include <string>
#include <vector>
#ifdef ENABLE_EXTERNAL_SIGNER
ExternalSigner::ExternalSigner(const std::string& command, const std::string& fingerprint, const std::string chain, const std::string name): m_command(command), m_fingerprint(fingerprint), m_chain(chain), m_name(name) {}
const std::string ExternalSigner::NetworkArg() const
{
return " --chain " + m_chain;
}
#ifdef ENABLE_EXTERNAL_SIGNER
bool ExternalSigner::Enumerate(const std::string& command, std::vector<ExternalSigner>& signers, std::string chain, bool ignore_errors)
bool ExternalSigner::Enumerate(const std::string& command, std::vector<ExternalSigner>& signers, const std::string chain)
{
// Call <command> enumerate
const UniValue result = RunCommandParseJSON(command + " enumerate");
if (!result.isArray()) {
if (ignore_errors) return false;
throw ExternalSignerException(strprintf("'%s' received invalid response, expected array of signers", command));
throw std::runtime_error(strprintf("'%s' received invalid response, expected array of signers", command));
}
for (UniValue signer : result.getValues()) {
// Check for error
const UniValue& error = find_value(signer, "error");
if (!error.isNull()) {
if (ignore_errors) return false;
if (!error.isStr()) {
throw ExternalSignerException(strprintf("'%s' error", command));
throw std::runtime_error(strprintf("'%s' error", command));
}
throw ExternalSignerException(strprintf("'%s' error: %s", command, error.getValStr()));
throw std::runtime_error(strprintf("'%s' error: %s", command, error.getValStr()));
}
// Check if fingerprint is present
const UniValue& fingerprint = find_value(signer, "fingerprint");
if (fingerprint.isNull()) {
if (ignore_errors) return false;
throw ExternalSignerException(strprintf("'%s' received invalid response, missing signer fingerprint", command));
throw std::runtime_error(strprintf("'%s' received invalid response, missing signer fingerprint", command));
}
std::string fingerprintStr = fingerprint.get_str();
const std::string fingerprintStr = fingerprint.get_str();
// Skip duplicate signer
bool duplicate = false;
for (ExternalSigner signer : signers) {
for (const ExternalSigner& signer : signers) {
if (signer.m_fingerprint.compare(fingerprintStr) == 0) duplicate = true;
}
if (duplicate) break;
@ -64,7 +65,7 @@ UniValue ExternalSigner::DisplayAddress(const std::string& descriptor) const
return RunCommandParseJSON(m_command + " --fingerprint \"" + m_fingerprint + "\"" + NetworkArg() + " displayaddress --desc \"" + descriptor + "\"");
}
UniValue ExternalSigner::GetDescriptors(int account)
UniValue ExternalSigner::GetDescriptors(const int account)
{
return RunCommandParseJSON(m_command + " --fingerprint \"" + m_fingerprint + "\"" + NetworkArg() + " getdescriptors --account " + strprintf("%d", account));
}
@ -79,7 +80,7 @@ bool ExternalSigner::SignTransaction(PartiallySignedTransaction& psbtx, std::str
bool match = false;
for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) {
const PSBTInput& input = psbtx.inputs[i];
for (auto entry : input.hd_keypaths) {
for (const auto& entry : input.hd_keypaths) {
if (m_fingerprint == strprintf("%08x", ReadBE32(entry.second.fingerprint))) match = true;
}
}
@ -89,8 +90,8 @@ bool ExternalSigner::SignTransaction(PartiallySignedTransaction& psbtx, std::str
return false;
}
std::string command = m_command + " --stdin --fingerprint \"" + m_fingerprint + "\"" + NetworkArg();
std::string stdinStr = "signtx \"" + EncodeBase64(ssTx.str()) + "\"";
const std::string command = m_command + " --stdin --fingerprint \"" + m_fingerprint + "\"" + NetworkArg();
const std::string stdinStr = "signtx \"" + EncodeBase64(ssTx.str()) + "\"";
const UniValue signer_result = RunCommandParseJSON(command, stdinStr);
@ -116,4 +117,4 @@ bool ExternalSigner::SignTransaction(PartiallySignedTransaction& psbtx, std::str
return true;
}
#endif
#endif // ENABLE_EXTERNAL_SIGNER

23
src/external_signer.h
View file

@ -5,17 +5,15 @@
#ifndef BITCOIN_EXTERNAL_SIGNER_H
#define BITCOIN_EXTERNAL_SIGNER_H
#include <stdexcept>
#include <string>
#include <univalue.h>
#include <util/system.h>
struct PartiallySignedTransaction;
#include <string>
#include <vector>
class ExternalSignerException : public std::runtime_error {
public:
using std::runtime_error::runtime_error;
};
#ifdef ENABLE_EXTERNAL_SIGNER
struct PartiallySignedTransaction;
//! Enables interaction with an external signing device or service, such as
//! a hardware wallet. See doc/external-signer.md
@ -30,7 +28,7 @@ public:
//! @param[in] fingerprint master key fingerprint of the signer
//! @param[in] chain "main", "test", "regtest" or "signet"
//! @param[in] name device name
ExternalSigner(const std::string& command, const std::string& fingerprint, std::string chain, std::string name);
ExternalSigner(const std::string& command, const std::string& fingerprint, const std::string chain, const std::string name);
//! Master key fingerprint of the signer
std::string m_fingerprint;
@ -43,13 +41,12 @@ public:
const std::string NetworkArg() const;
#ifdef ENABLE_EXTERNAL_SIGNER
//! 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"
//! @returns success
static bool Enumerate(const std::string& command, std::vector<ExternalSigner>& signers, std::string chain, bool ignore_errors = false);
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>`.
//! @param[in] descriptor Descriptor specifying which address to display.
@ -60,14 +57,14 @@ public:
//! Calls `<command> 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);
UniValue GetDescriptors(const int account);
//! Sign PartiallySignedTransaction on the device.
//! Calls `<command> signtransaction` and passes the PSBT via stdin.
//! @param[in,out] psbt PartiallySignedTransaction to be signed
bool SignTransaction(PartiallySignedTransaction& psbt, std::string& error);
#endif
};
#endif // ENABLE_EXTERNAL_SIGNER
#endif // BITCOIN_EXTERNAL_SIGNER_H

9
src/rpc/external_signer.cpp
View file

@ -9,6 +9,9 @@
#include <util/strencodings.h>
#include <rpc/protocol.h>
#include <string>
#include <vector>
#ifdef ENABLE_EXTERNAL_SIGNER
static RPCHelpMan enumeratesigners()
@ -35,18 +38,18 @@ static RPCHelpMan enumeratesigners()
{
const std::string command = gArgs.GetArg("-signer", "");
if (command == "") throw JSONRPCError(RPC_MISC_ERROR, "Error: restart bitcoind with -signer=<cmd>");
std::string chain = gArgs.GetChainName();
const std::string chain = gArgs.GetChainName();
UniValue signers_res = UniValue::VARR;
try {
std::vector<ExternalSigner> signers;
ExternalSigner::Enumerate(command, signers, chain);
for (ExternalSigner signer : signers) {
for (const ExternalSigner& signer : signers) {
UniValue signer_res = UniValue::VOBJ;
signer_res.pushKV("fingerprint", signer.m_fingerprint);
signer_res.pushKV("name", signer.m_name);
signers_res.push_back(signer_res);
}
} catch (const ExternalSignerException& e) {
} catch (const std::exception& e) {
throw JSONRPCError(RPC_MISC_ERROR, e.what());
}
UniValue result(UniValue::VOBJ);

7
src/wallet/external_signer_scriptpubkeyman.cpp
View file

@ -6,6 +6,13 @@
#include <external_signer.h>
#include <wallet/external_signer_scriptpubkeyman.h>
#include <iostream>
#include <memory>
#include <stdexcept>
#include <string>
#include <utility>
#include <vector>
#ifdef ENABLE_EXTERNAL_SIGNER
bool ExternalSignerScriptPubKeyMan::SetupDescriptor(std::unique_ptr<Descriptor> desc)

2
src/wallet/external_signer_scriptpubkeyman.h
View file

@ -8,6 +8,8 @@
#ifdef ENABLE_EXTERNAL_SIGNER
#include <wallet/scriptpubkeyman.h>
#include <memory>
class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan
{
public:

2
src/wallet/rpcwallet.cpp
View file

@ -2750,7 +2750,7 @@ static RPCHelpMan createwallet()
#ifdef ENABLE_EXTERNAL_SIGNER
flags |= WALLET_FLAG_EXTERNAL_SIGNER;
#else
throw JSONRPCError(RPC_WALLET_ERROR, "Configure with --enable-external-signer to use this");
throw JSONRPCError(RPC_WALLET_ERROR, "Compiled without external signing support (required for external signing)");
#endif
}

21
src/wallet/wallet.cpp
View file

@ -3594,19 +3594,6 @@ void ReserveDestination::ReturnDestination()
address = CNoDestination();
}
#ifdef ENABLE_EXTERNAL_SIGNER
ExternalSigner CWallet::GetExternalSigner()
{
const std::string command = gArgs.GetArg("-signer", "");
if (command == "") throw std::runtime_error(std::string(__func__) + ": restart bitcoind with -signer=<cmd>");
std::vector<ExternalSigner> signers;
ExternalSigner::Enumerate(command, signers, Params().NetworkIDString());
if (signers.empty()) throw std::runtime_error(std::string(__func__) + ": No external signers found");
// TODO: add fingerprint argument in case of multiple signers
return signers[0];
}
#endif
bool CWallet::DisplayAddress(const CTxDestination& dest)
{
#ifdef ENABLE_EXTERNAL_SIGNER
@ -3619,7 +3606,7 @@ bool CWallet::DisplayAddress(const CTxDestination& dest)
if (signer_spk_man == nullptr) {
return false;
}
ExternalSigner signer = GetExternalSigner(); // TODO: move signer in spk_man
ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner();
return signer_spk_man->DisplayAddress(scriptPubKey, signer);
#else
return false;
@ -4516,7 +4503,7 @@ void CWallet::LoadDescriptorScriptPubKeyMan(uint256 id, WalletDescriptor& desc)
auto spk_manager = std::unique_ptr<ScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(*this, desc));
m_spk_managers[id] = std::move(spk_manager);
#else
throw std::runtime_error(std::string(__func__) + ": Configure with --enable-external-signer to use external signer wallets");
throw std::runtime_error(std::string(__func__) + ": Compiled without external signing support (required for external signing)");
#endif
} else {
auto spk_manager = std::unique_ptr<ScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, desc));
@ -4585,8 +4572,8 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
}
}
#else
throw std::runtime_error(std::string(__func__) + ": Wallets with external signers require Boost::Process library.");
#endif
throw std::runtime_error(std::string(__func__) + ": Compiled without external signing support (required for external signing)");
#endif // ENABLE_EXTERNAL_SIGNER
}
}

3
src/wallet/wallet.h
View file

@ -845,9 +845,6 @@ public:
std::vector<OutputGroup> GroupOutputs(const std::vector<COutput>& outputs, bool separate_coins, const CFeeRate& effective_feerate, const CFeeRate& long_term_feerate, const CoinEligibilityFilter& filter, bool positive_only) const;
#ifdef ENABLE_EXTERNAL_SIGNER
ExternalSigner GetExternalSigner() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
#endif
/** Display address on an external signer. Returns false if external signer support is not compiled */
bool DisplayAddress(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);