refactor: make bls{Pub}KeyOperator member variables instead of pointers

Using unique_ptr is a relic of activeMasternodeInfo, as it was accessible
independent of CActiveMasternodeManager. As it is now only accessible
if CActiveMasternodeManager is initialized, we can make it part of its
constructor and make them const.

Removes the assertion and sanity checks to see if key pair is valid.

src/init.cpp

@@ -1853,9 +1853,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
         fMasternodeMode = true;
         {
             // Create and register activeMasternodeManager, will init later in ThreadImport
-            ::activeMasternodeManager = std::make_unique<CActiveMasternodeManager>(*node.connman, ::deterministicMNManager);
+            ::activeMasternodeManager = std::make_unique<CActiveMasternodeManager>(keyOperator, *node.connman, ::deterministicMNManager);
-            ::activeMasternodeManager->InitKeys(keyOperator);
-
             RegisterValidationInterface(::activeMasternodeManager.get());
         }
     }

src/masternode/node.cpp

@@ -18,15 +18,15 @@
 // Keep track of the active Masternode
 std::unique_ptr<CActiveMasternodeManager> activeMasternodeManager;
 
-CActiveMasternodeManager::~CActiveMasternodeManager()
+CActiveMasternodeManager::CActiveMasternodeManager(const CBLSSecretKey& sk, CConnman& _connman, const std::unique_ptr<CDeterministicMNManager>& dmnman) :
+    m_info(sk, sk.GetPublicKey()),
+    connman(_connman),
+    m_dmnman(dmnman)
 {
-    // Make sure to clean up BLS keys before global destructors are called
+    assert(sk.IsValid()); /* We can assume pk is valid if sk is valid */
-    // (they have been allocated from the secure memory pool)
+    LogPrintf("MASTERNODE:\n  blsPubKeyOperator legacy: %s\n  blsPubKeyOperator basic: %s\n",
-    {
+            m_info.blsPubKeyOperator.ToString(/*specificLegacyScheme=*/ true),
-        LOCK(cs);
+            m_info.blsPubKeyOperator.ToString(/*specificLegacyScheme=*/ false));
-        m_info.blsKeyOperator.reset();
-        m_info.blsPubKeyOperator.reset();
-    }
 }
 
 std::string CActiveMasternodeManager::GetStateString() const
@@ -97,7 +97,7 @@ void CActiveMasternodeManager::Init(const CBlockIndex* pindex)
 
     CDeterministicMNList mnList = Assert(m_dmnman)->GetListForBlock(pindex);
 
-    CDeterministicMNCPtr dmn = mnList.GetMNByOperatorKey(*m_info.blsPubKeyOperator);
+    CDeterministicMNCPtr dmn = mnList.GetMNByOperatorKey(m_info.blsPubKeyOperator);
     if (!dmn) {
         // MN not appeared on the chain yet
         return;
@@ -146,21 +146,6 @@ void CActiveMasternodeManager::Init(const CBlockIndex* pindex)
     state = MASTERNODE_READY;
 }
 
-void CActiveMasternodeManager::InitKeys(const CBLSSecretKey& sk)
-{
-    AssertLockNotHeld(cs);
-
-    LOCK(cs);
-    assert(m_info.blsKeyOperator == nullptr);
-    assert(m_info.blsPubKeyOperator == nullptr);
-    m_info.blsKeyOperator = std::make_unique<CBLSSecretKey>(sk);
-    m_info.blsPubKeyOperator = std::make_unique<CBLSPublicKey>(sk.GetPublicKey());
-    // We don't know the actual scheme at this point, print both
-    LogPrintf("MASTERNODE:\n  blsPubKeyOperator legacy: %s\n  blsPubKeyOperator basic: %s\n",
-            m_info.blsPubKeyOperator->ToString(/*specificLegacyScheme=*/ true),
-            m_info.blsPubKeyOperator->ToString(/*specificLegacyScheme=*/ false));
-}
-
 void CActiveMasternodeManager::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload)
 {
     LOCK2(::cs_main, cs);
@@ -258,7 +243,7 @@ template <template <typename> class EncryptedObj, typename Obj>
                                                      int version) const
 {
     AssertLockNotHeld(cs);
-    return WITH_LOCK(cs, return obj.Decrypt(idx, *Assert(m_info.blsKeyOperator), ret_obj, version));
+    return WITH_LOCK(cs, return obj.Decrypt(idx, m_info.blsKeyOperator, ret_obj, version));
 }
 template bool CActiveMasternodeManager::Decrypt(const CBLSIESEncryptedObject<CBLSSecretKey>& obj, size_t idx,
                                                 CBLSSecretKey& ret_obj, int version) const;
@@ -268,18 +253,18 @@ template bool CActiveMasternodeManager::Decrypt(const CBLSIESMultiRecipientObjec
 [[nodiscard]] CBLSSignature CActiveMasternodeManager::Sign(const uint256& hash) const
 {
     AssertLockNotHeld(cs);
-    return WITH_LOCK(cs, return Assert(m_info.blsKeyOperator)->Sign(hash));
+    return WITH_LOCK(cs, return m_info.blsKeyOperator.Sign(hash));
 }
 
 [[nodiscard]] CBLSSignature CActiveMasternodeManager::Sign(const uint256& hash, const bool is_legacy) const
 {
     AssertLockNotHeld(cs);
-    return WITH_LOCK(cs, return Assert(m_info.blsKeyOperator)->Sign(hash, is_legacy));
+    return WITH_LOCK(cs, return m_info.blsKeyOperator.Sign(hash, is_legacy));
 }
 
 // We need to pass a copy as opposed to a const ref because CBLSPublicKeyVersionWrapper
 // does not accept a const ref in its construction args
 [[nodiscard]] CBLSPublicKey CActiveMasternodeManager::GetPubKey() const
 {
-    return *Assert(m_info.blsPubKeyOperator);
+    return m_info.blsPubKeyOperator;
 }

src/masternode/node.h

@@ -5,26 +5,27 @@
 #ifndef BITCOIN_MASTERNODE_NODE_H
 #define BITCOIN_MASTERNODE_NODE_H
 
+#include <bls/bls.h>
 #include <netaddress.h>
 #include <primitives/transaction.h>
 #include <threadsafety.h>
 #include <validationinterface.h>
 
-class CBLSPublicKey;
-class CBLSSecretKey;
-class CBLSSignature;
 class CDeterministicMNManager;
 
 struct CActiveMasternodeInfo {
     // Keys for the active Masternode
-    std::unique_ptr<CBLSPublicKey> blsPubKeyOperator;
+    const CBLSSecretKey blsKeyOperator;
-    std::unique_ptr<CBLSSecretKey> blsKeyOperator;
+    const CBLSPublicKey blsPubKeyOperator;
 
     // Initialized while registering Masternode
     uint256 proTxHash;
     COutPoint outpoint;
     CService service;
     bool legacy{true};
+
+    CActiveMasternodeInfo(const CBLSSecretKey& blsKeyOperator, const CBLSPublicKey& blsPubKeyOperator) :
+        blsKeyOperator(blsKeyOperator), blsPubKeyOperator(blsPubKeyOperator) {};
 };
 
 class CActiveMasternodeManager final : public CValidationInterface
@@ -51,14 +52,11 @@ private:
     const std::unique_ptr<CDeterministicMNManager>& m_dmnman;
 
 public:
-    explicit CActiveMasternodeManager(CConnman& _connman, const std::unique_ptr<CDeterministicMNManager>& dmnman) :
+    explicit CActiveMasternodeManager(const CBLSSecretKey& sk, CConnman& _connman, const std::unique_ptr<CDeterministicMNManager>& dmnman);
-        connman(_connman), m_dmnman(dmnman) {};
-    ~CActiveMasternodeManager();
 
     void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) override;
 
     void Init(const CBlockIndex* pindex);
-    void InitKeys(const CBLSSecretKey& sk) LOCKS_EXCLUDED(cs);
 
     std::string GetStateString() const;
     std::string GetStatus() const;

src/rpc/governance.cpp

@@ -322,7 +322,7 @@ static UniValue gobject_submit(const JSONRPCRequest& request)
         fMnFound = mnList.HasValidMNByCollateral(::activeMasternodeManager->GetOutPoint());
 
         LogPrint(BCLog::GOBJECT, "gobject_submit -- pubKeyOperator = %s, outpoint = %s, params.size() = %lld, fMnFound = %d\n",
-                (::activeMasternodeManager->GetPubKey().IsValid() ? ::activeMasternodeManager->GetPubKey().ToString(::activeMasternodeManager->IsLegacy()) : "N/A"),
+                ::activeMasternodeManager->GetPubKey().ToString(::activeMasternodeManager->IsLegacy()),
                 ::activeMasternodeManager->GetOutPoint().ToStringShort(), request.params.size(), fMnFound);
     } else {
         LogPrint(BCLog::GOBJECT, "gobject_submit -- pubKeyOperator = N/A, outpoint = N/A, params.size() = %lld, fMnFound = %d\n",