refactor: reduce #ifdef ENABLE_EXTERNAL_SIGNER usage

In particular this make the node interface independent on whether external signer support is compiled.
bitcoin-core · Jun 16, 2021 · 4455145 · 4455145
1 parent 5be90c9
commit 4455145
Show file tree

Hide file tree

Showing 12 changed files with 15 additions and 45 deletions.
diff --git a/src/external_signer.cpp b/src/external_signer.cpp
@@ -13,8 +13,6 @@
 #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
@@ -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
@@ -11,8 +11,6 @@
 #include <string>
 #include <vector>
 
-#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
@@ -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<ExternalSigner> externalSigners() = 0;
-#endif
 
     //! Get total bytes recv.
     virtual int64_t getTotalBytesRecv() = 0;

diff --git 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<ExternalSigner> externalSigners() override
     {
+#ifdef ENABLE_EXTERNAL_SIGNER
         std::vector<ExternalSigner> 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<std::vector>
+        // (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
@@ -112,7 +112,6 @@ CreateWalletDialog::~CreateWalletDialog()
     delete ui;
 }
 
-#ifdef ENABLE_EXTERNAL_SIGNER
 void CreateWalletDialog::setSigners(std::vector<ExternalSigner>& signers)
 {
     if (!signers.empty()) {
@@ -132,7 +131,6 @@ void CreateWalletDialog::setSigners(std::vector<ExternalSigner>& signers)
         ui->external_signer_checkbox->setEnabled(false);
     }
 }
-#endif
 
 QString CreateWalletDialog::walletName() const
 {

diff --git a/src/qt/createwalletdialog.h b/src/qt/createwalletdialog.h
@@ -7,11 +7,8 @@
 
 #include <QDialog>
 
-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<ExternalSigner>& signers);
-#endif
 
     QString walletName() const;
     bool isEncryptWalletChecked() const;

diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp
@@ -296,15 +296,13 @@ void CreateWalletActivity::create()
 {
     m_create_wallet_dialog = new CreateWalletDialog(m_parent_widget);
 
-#ifdef ENABLE_EXTERNAL_SIGNER
     std::vector<ExternalSigner> signers;
     try {
         signers = node().externalSigners();
     } catch (const std::runtime_error& e) {
         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
@@ -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
@@ -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
@@ -13,8 +13,6 @@
 #include <utility>
 #include <vector>
 
-#ifdef ENABLE_EXTERNAL_SIGNER
-
 bool ExternalSignerScriptPubKeyMan::SetupDescriptor(std::unique_ptr<Descriptor> 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
@@ -5,7 +5,6 @@
 #ifndef BITCOIN_WALLET_EXTERNAL_SIGNER_SCRIPTPUBKEYMAN_H
 #define BITCOIN_WALLET_EXTERNAL_SIGNER_SCRIPTPUBKEYMAN_H
 
-#ifdef ENABLE_EXTERNAL_SIGNER
 #include <wallet/scriptpubkeyman.h>
 
 #include <memory>
@@ -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
@@ -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<ScriptPubKeyMan>(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<ScriptPubKeyMan>(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
     }
 }