From 4455145e266450397b45acd7286686966edd072b Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 13 May 2021 15:23:19 +0200 Subject: [PATCH] refactor: reduce #ifdef ENABLE_EXTERNAL_SIGNER usage In particular this make the node interface independent on whether external signer support is compiled. --- src/external_signer.cpp | 4 ---- src/external_signer.h | 4 ---- src/interfaces/node.h | 2 -- src/node/interfaces.cpp | 12 ++++++++++-- src/qt/createwalletdialog.cpp | 2 -- src/qt/createwalletdialog.h | 7 +------ src/qt/walletcontroller.cpp | 2 -- src/util/system.cpp | 6 ++++-- src/util/system.h | 2 -- src/wallet/external_signer_scriptpubkeyman.cpp | 4 ---- src/wallet/external_signer_scriptpubkeyman.h | 3 --- src/wallet/wallet.cpp | 12 ------------ 12 files changed, 15 insertions(+), 45 deletions(-) diff --git a/src/external_signer.cpp b/src/external_signer.cpp index f16d21fa60c..b2bd7cb261a 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -13,8 +13,6 @@ #include #include -#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 @@ -116,5 +114,3 @@ bool ExternalSigner::SignTransaction(PartiallySignedTransaction& psbtx, std::str return true; } - -#endif // ENABLE_EXTERNAL_SIGNER diff --git a/src/external_signer.h b/src/external_signer.h index b3b202091a6..c4f723a1bb8 100644 --- a/src/external_signer.h +++ b/src/external_signer.h @@ -11,8 +11,6 @@ #include #include -#ifdef ENABLE_EXTERNAL_SIGNER - struct PartiallySignedTransaction; //! Enables interaction with an external signing device or service, such as @@ -65,6 +63,4 @@ class ExternalSigner bool SignTransaction(PartiallySignedTransaction& psbt, std::string& error); }; -#endif // ENABLE_EXTERNAL_SIGNER - #endif // BITCOIN_EXTERNAL_SIGNER_H diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 35b6160ceac..77129423db9 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -111,10 +111,8 @@ class Node //! Disconnect node by id. virtual bool disconnectById(NodeId id) = 0; -#ifdef ENABLE_EXTERNAL_SIGNER //! List external signers virtual std::vector externalSigners() = 0; -#endif //! Get total bytes recv. virtual int64_t getTotalBytesRecv() = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 37a5fe4b383..fce3c1809c3 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -171,16 +171,24 @@ class NodeImpl : public Node } return false; } -#ifdef ENABLE_EXTERNAL_SIGNER std::vector externalSigners() override { +#ifdef ENABLE_EXTERNAL_SIGNER std::vector signers = {}; const std::string command = gArgs.GetArg("-signer", ""); if (command == "") return signers; ExternalSigner::Enumerate(command, signers, Params().NetworkIDString()); return signers; +#else + // This result is undistinguisable from a succesful call that returns + // no signers. For the current GUI this doesn't matter, because the wallet + // creation dialog disables the external signer checkbox in both + // cases. The return type could be changed to std::optional + // (or something that also includes error messages) if this distinction + // becomes important. + return {}; +#endif // ENABLE_EXTERNAL_SIGNER } -#endif int64_t getTotalBytesRecv() override { return m_context->connman ? m_context->connman->GetTotalBytesRecv() : 0; } int64_t getTotalBytesSent() override { return m_context->connman ? m_context->connman->GetTotalBytesSent() : 0; } size_t getMempoolSize() override { return m_context->mempool ? m_context->mempool->size() : 0; } diff --git a/src/qt/createwalletdialog.cpp b/src/qt/createwalletdialog.cpp index e593697b46d..cea41a94aef 100644 --- a/src/qt/createwalletdialog.cpp +++ b/src/qt/createwalletdialog.cpp @@ -112,7 +112,6 @@ CreateWalletDialog::~CreateWalletDialog() delete ui; } -#ifdef ENABLE_EXTERNAL_SIGNER void CreateWalletDialog::setSigners(std::vector& signers) { if (!signers.empty()) { @@ -132,7 +131,6 @@ void CreateWalletDialog::setSigners(std::vector& signers) ui->external_signer_checkbox->setEnabled(false); } } -#endif QString CreateWalletDialog::walletName() const { diff --git a/src/qt/createwalletdialog.h b/src/qt/createwalletdialog.h index 585b1461f7e..3f6a32548eb 100644 --- a/src/qt/createwalletdialog.h +++ b/src/qt/createwalletdialog.h @@ -7,11 +7,8 @@ #include -class WalletModel; - -#ifdef ENABLE_EXTERNAL_SIGNER class ExternalSigner; -#endif +class WalletModel; namespace Ui { class CreateWalletDialog; @@ -27,9 +24,7 @@ class CreateWalletDialog : public QDialog explicit CreateWalletDialog(QWidget* parent); virtual ~CreateWalletDialog(); -#ifdef ENABLE_EXTERNAL_SIGNER void setSigners(std::vector& signers); -#endif QString walletName() const; bool isEncryptWalletChecked() const; diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index 24f73a2ea07..3cceb5ca5a4 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -296,7 +296,6 @@ void CreateWalletActivity::create() { m_create_wallet_dialog = new CreateWalletDialog(m_parent_widget); -#ifdef ENABLE_EXTERNAL_SIGNER std::vector signers; try { signers = node().externalSigners(); @@ -304,7 +303,6 @@ void CreateWalletActivity::create() QMessageBox::critical(nullptr, tr("Can't list signers"), e.what()); } m_create_wallet_dialog->setSigners(signers); -#endif m_create_wallet_dialog->setWindowModality(Qt::ApplicationModal); m_create_wallet_dialog->show(); diff --git a/src/util/system.cpp b/src/util/system.cpp index 13ccf7463ee..5ea01392755 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -1243,9 +1243,9 @@ void runCommand(const std::string& strCommand) } #endif -#ifdef ENABLE_EXTERNAL_SIGNER UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) { +#ifdef ENABLE_EXTERNAL_SIGNER namespace bp = boost::process; UniValue result_json; @@ -1277,8 +1277,10 @@ UniValue RunCommandParseJSON(const std::string& str_command, const std::string& if (!result_json.read(result)) throw std::runtime_error("Unable to parse JSON: " + result); return result_json; -} +#else + throw std::runtime_error("Compiled without external signing support (required for external signing)."); #endif // ENABLE_EXTERNAL_SIGNER +} void SetupEnvironment() { diff --git a/src/util/system.h b/src/util/system.h index c4317c62d02..ea9870a343e 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -102,7 +102,6 @@ std::string ShellEscape(const std::string& arg); #if HAVE_SYSTEM void runCommand(const std::string& strCommand); #endif -#ifdef ENABLE_EXTERNAL_SIGNER /** * Execute a command which returns JSON, and parse the result. * @@ -111,7 +110,6 @@ void runCommand(const std::string& strCommand); * @return parsed JSON */ UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in=""); -#endif // ENABLE_EXTERNAL_SIGNER /** * Most paths passed as configuration arguments are treated as relative to diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index fe2c810afa7..e5464c5b89c 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -13,8 +13,6 @@ #include #include -#ifdef ENABLE_EXTERNAL_SIGNER - bool ExternalSignerScriptPubKeyMan::SetupDescriptor(std::unique_ptr desc) { LOCK(cs_desc_man); @@ -84,5 +82,3 @@ TransactionError ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransact FinalizePSBT(psbt); // This won't work in a multisig setup return TransactionError::OK; } - -#endif diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index 1786958912c..5df1e6d9395 100644 --- a/src/wallet/external_signer_scriptpubkeyman.h +++ b/src/wallet/external_signer_scriptpubkeyman.h @@ -5,7 +5,6 @@ #ifndef BITCOIN_WALLET_EXTERNAL_SIGNER_SCRIPTPUBKEYMAN_H #define BITCOIN_WALLET_EXTERNAL_SIGNER_SCRIPTPUBKEYMAN_H -#ifdef ENABLE_EXTERNAL_SIGNER #include #include @@ -31,6 +30,4 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override; }; -#endif - #endif // BITCOIN_WALLET_EXTERNAL_SIGNER_SCRIPTPUBKEYMAN_H diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 98fab260300..073d5e269be 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2216,7 +2216,6 @@ void ReserveDestination::ReturnDestination() bool CWallet::DisplayAddress(const CTxDestination& dest) { -#ifdef ENABLE_EXTERNAL_SIGNER CScript scriptPubKey = GetScriptForDestination(dest); const auto spk_man = GetScriptPubKeyMan(scriptPubKey); if (spk_man == nullptr) { @@ -2228,9 +2227,6 @@ bool CWallet::DisplayAddress(const CTxDestination& dest) } ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner(); return signer_spk_man->DisplayAddress(scriptPubKey, signer); -#else - return false; -#endif } void CWallet::LockCoin(const COutPoint& output) @@ -3064,12 +3060,8 @@ void CWallet::ConnectScriptPubKeyManNotifiers() void CWallet::LoadDescriptorScriptPubKeyMan(uint256 id, WalletDescriptor& desc) { if (IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) { -#ifdef ENABLE_EXTERNAL_SIGNER auto spk_manager = std::unique_ptr(new ExternalSignerScriptPubKeyMan(*this, desc)); m_spk_managers[id] = std::move(spk_manager); -#else - throw std::runtime_error(std::string(__func__) + ": Compiled without external signing support (required for external signing)"); -#endif } else { auto spk_manager = std::unique_ptr(new DescriptorScriptPubKeyMan(*this, desc)); m_spk_managers[id] = std::move(spk_manager); @@ -3109,7 +3101,6 @@ void CWallet::SetupDescriptorScriptPubKeyMans() } } } else { -#ifdef ENABLE_EXTERNAL_SIGNER ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner(); // TODO: add account parameter @@ -3136,9 +3127,6 @@ void CWallet::SetupDescriptorScriptPubKeyMans() AddActiveScriptPubKeyMan(id, t, internal); } } -#else - throw std::runtime_error(std::string(__func__) + ": Compiled without external signing support (required for external signing)"); -#endif // ENABLE_EXTERNAL_SIGNER } }