Merge dashpay#6467: feat: mnauth always use basic scheme

09058e0 feat: mnauth always use basic scheme (pasta) Pull request description: ## Issue being fixed or feature implemented Currently, mnauth has to check status of v19 hard fork. While this isn't soo terrible, it's not needed anymore. On mainnet or testnet, any mn you possible connect to will have it's TIP past v19 HF, meaning in practice it will only ever send you basic scheme anyhow. Let's just harden it. I initially guarded this behind a new protocol version, but I do not think that is needed. ## What was done? ## How Has This Been Tested? ## Breaking Changes This is potentially a breaking change for devnets, which are moving past the v19 hard fork, but on develop v19 activates at block 2 for devnets sooooo, this shouldn't be noticed. ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 09058e0 knst: utACK 09058e0 Tree-SHA512: 45483904135c90a507f2a1f61c4b06915eb7c02a29eeda254938624a2593aec86540fa360aea498428285b3c0e954c676510dcb128f4ec7062302293c134d517
PastaPastaPasta · Dec 10, 2024 · b197ffb · b197ffb
2 parents 8ec789f + 09058e0
commit b197ffb
Show file tree

Hide file tree

Showing 7 changed files with 13 additions and 38 deletions.
diff --git a/src/bls/bls.h b/src/bls/bls.h
@@ -309,21 +309,6 @@ class CBLSPublicKey : public CBLSWrapper<bls::G1Element, BLS_CURVE_PUBKEY_SIZE,
 
 };
 
-class ConstCBLSPublicKeyVersionWrapper {
-private:
-    const CBLSPublicKey& obj;
-    bool legacy;
-public:
-    ConstCBLSPublicKeyVersionWrapper(const CBLSPublicKey& obj, bool legacy)
-            : obj(obj)
-            , legacy(legacy)
-    {}
-    template <typename Stream>
-    inline void Serialize(Stream& s) const {
-        obj.Serialize(s, legacy);
-    }
-};
-
 class CBLSPublicKeyVersionWrapper {
 private:
     CBLSPublicKey& obj;

diff --git a/src/evo/mnauth.cpp b/src/evo/mnauth.cpp
@@ -7,7 +7,6 @@
 #include <bls/bls.h>
 #include <chain.h>
 #include <chainparams.h>
-#include <deploymentstatus.h>
 #include <evo/deterministicmns.h>
 #include <llmq/utils.h>
 #include <masternode/meta.h>
@@ -19,8 +18,7 @@
 #include <util/time.h>
 #include <validation.h>
 
-void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CActiveMasternodeManager& mn_activeman,
-                         const CBlockIndex* tip)
+void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CActiveMasternodeManager& mn_activeman)
 {
     CMNAuth mnauth;
     if (mn_activeman.GetProTxHash().IsNull()) {
@@ -41,9 +39,8 @@ void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CActiveMasternode
     if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) {
         nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION);
     }
-    const bool is_basic_scheme_active{DeploymentActiveAfter(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)};
     auto pk = mn_activeman.GetPubKey();
-    const CBLSPublicKeyVersionWrapper pubKey(pk, !is_basic_scheme_active);
+    const CBLSPublicKey pubKey(pk);
     uint256 signHash = [&]() {
         if (peer.nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) {
             return ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn()));
@@ -61,7 +58,7 @@ void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CActiveMasternode
 }
 
 PeerMsgRet CMNAuth::ProcessMessage(CNode& peer, ServiceFlags node_services, CConnman& connman, CMasternodeMetaMan& mn_metaman, const CActiveMasternodeManager* const mn_activeman,
-                                   const CChain& active_chain, const CMasternodeSync& mn_sync, const CDeterministicMNList& tip_mn_list,
+                                   const CMasternodeSync& mn_sync, const CDeterministicMNList& tip_mn_list,
                                    std::string_view msg_type, CDataStream& vRecv)
 {
     assert(mn_metaman.IsValid());
@@ -106,9 +103,7 @@ PeerMsgRet CMNAuth::ProcessMessage(CNode& peer, ServiceFlags node_services, CCon
     if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) {
         nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION);
     }
-    const CBlockIndex* tip = active_chain.Tip();
-    const bool is_basic_scheme_active{DeploymentActiveAfter(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)};
-    ConstCBLSPublicKeyVersionWrapper pubKey(dmn->pdmnState->pubKeyOperator.Get(), !is_basic_scheme_active);
+    const CBLSPublicKey pubKey(dmn->pdmnState->pubKeyOperator.Get());
     // See comment in PushMNAUTH (fInbound is negated here as we're on the other side of the connection)
     if (peer.nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) {
         signHash = ::SerializeHash(std::make_tuple(pubKey, peer.GetSentMNAuthChallenge(), !peer.IsInboundConn()));

diff --git a/src/evo/mnauth.h b/src/evo/mnauth.h
@@ -11,7 +11,6 @@
 
 class CActiveMasternodeManager;
 class CBlockIndex;
-class CChain;
 class CConnman;
 class CDataStream;
 class CDeterministicMNList;
@@ -50,15 +49,14 @@ class CMNAuth
         READWRITE(obj.proRegTxHash, obj.sig);
     }
 
-    static void PushMNAUTH(CNode& peer, CConnman& connman, const CActiveMasternodeManager& mn_activeman,
-                           const CBlockIndex* tip);
+    static void PushMNAUTH(CNode& peer, CConnman& connman, const CActiveMasternodeManager& mn_activeman);
 
     /**
      * @pre CMasternodeMetaMan's database must be successfully loaded before
      *      attempting to call this function regardless of sync state
      */
     static PeerMsgRet ProcessMessage(CNode& peer, ServiceFlags node_services, CConnman& connman, CMasternodeMetaMan& mn_metaman, const CActiveMasternodeManager* const mn_activeman,
-                                     const CChain& active_chain, const CMasternodeSync& mn_sync, const CDeterministicMNList& tip_mn_list,
+                                     const CMasternodeSync& mn_sync, const CDeterministicMNList& tip_mn_list,
                                      std::string_view msg_type, CDataStream& vRecv);
     static void NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff, CConnman& connman);
 };

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -3784,7 +3784,7 @@ void PeerManagerImpl::ProcessMessage(
         }
 
         if (is_masternode && !pfrom.m_masternode_probe_connection) {
-            CMNAuth::PushMNAUTH(pfrom, m_connman, *m_mn_activeman, m_chainman.ActiveChain().Tip());
+            CMNAuth::PushMNAUTH(pfrom, m_connman, *m_mn_activeman);
         }
 
         // Tell our peer we prefer to receive headers rather than inv's
@@ -5260,7 +5260,7 @@ void PeerManagerImpl::ProcessMessage(
         ProcessPeerMsgRet(m_sporkman.ProcessMessage(pfrom, m_connman, *this, msg_type, vRecv), pfrom);
         m_mn_sync.ProcessMessage(pfrom, msg_type, vRecv);
         ProcessPeerMsgRet(m_govman.ProcessMessage(pfrom, m_connman, *this, msg_type, vRecv), pfrom);
-        ProcessPeerMsgRet(CMNAuth::ProcessMessage(pfrom, peer->m_their_services, m_connman, m_mn_metaman, m_mn_activeman, m_chainman.ActiveChain(), m_mn_sync, m_dmnman->GetListAtChainTip(), msg_type, vRecv), pfrom);
+        ProcessPeerMsgRet(CMNAuth::ProcessMessage(pfrom, peer->m_their_services, m_connman, m_mn_metaman, m_mn_activeman, m_mn_sync, m_dmnman->GetListAtChainTip(), msg_type, vRecv), pfrom);
         PostProcessMessage(m_llmq_ctx->quorum_block_processor->ProcessMessage(pfrom, msg_type, vRecv), pfrom.GetId());
         ProcessPeerMsgRet(m_llmq_ctx->qdkgsman->ProcessMessage(pfrom, *this, is_masternode, msg_type, vRecv), pfrom);
         ProcessPeerMsgRet(m_llmq_ctx->qman->ProcessMessage(pfrom, m_connman, msg_type, vRecv), pfrom);

diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
@@ -7,7 +7,6 @@
 #include <addressindex.h>
 #include <chainparams.h>
 #include <consensus/consensus.h>
-#include <deploymentstatus.h>
 #include <evo/mnauth.h>
 #include <httpserver.h>
 #include <index/blockfilterindex.h>
@@ -633,11 +632,8 @@ static RPCHelpMan mnauth()
         throw JSONRPCError(RPC_INVALID_PARAMETER, "proTxHash invalid");
     }
 
-    ChainstateManager& chainman = EnsureAnyChainman(request.context);
-
     CBLSPublicKey publicKey;
-    const bool bls_legacy_scheme{!DeploymentActiveAfter(chainman.ActiveChain().Tip(), Params().GetConsensus(), Consensus::DEPLOYMENT_V19)};
-    publicKey.SetHexStr(request.params[2].get_str(), bls_legacy_scheme);
+    publicKey.SetHexStr(request.params[2].get_str(), /*bls_legacy_scheme=*/false);
     if (!publicKey.IsValid()) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "publicKey invalid");
     }

diff --git a/test/functional/p2p_quorum_data.py b/test/functional/p2p_quorum_data.py
@@ -34,9 +34,9 @@
 
 # Used to overwrite MNAUTH for mininode connections
 fake_mnauth_1 = ["cecf37bf0ec05d2d22cb8227f88074bb882b94cd2081ba318a5a444b1b15b9fd",
-                 "087ba00bf61135f3860c4944a0debabe186ef82628fbe4ceaed1ad51d672c58dde14ea4b321efe0b89257a40322bc972"]
+                 "8e7afdb849e5e2a085b035b62e21c0940c753f2d4501325743894c37162f287bccaffbedd60c36581dabbf127a22e43f"]
 fake_mnauth_2 = ["6ad7ed7a2d6c2c1db30fc364114602b36b2730a9aa96d8f11f1871a9cee37378",
-                 "122463411a86362966a5161805f24cf6a0eef08a586b8e00c4f0ad0b084c5bb3f5c9a60ee5ffc78db2313897e3ab2223"]
+                 "ad38860c03c3d1d875771f41b8a9b933415f72929c21a4276c101d8f0268f6fcdfeed46507c16c00e74f26ce1181e69f"]
 
 # Used to distinguish mininode connections
 uacomment_m3_1 = "MN3_1"
@@ -400,6 +400,7 @@ def test_rpc_quorum_getdata_protx_hash():
                                     mn1.node.quorum, "getdata", 0, 100, quorum_hash, 0x03,
                                     "0000000000000000000000000000000000000000000000000000000000000000")
 
+        self.activate_v19(expected_activation_height=900)
         # Enable DKG and disable ChainLocks
         self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
         self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", 4070908800)

diff --git a/test/functional/rpc_mnauth.py b/test/functional/rpc_mnauth.py
@@ -20,12 +20,12 @@ def set_test_params(self):
         self.set_dash_test_params(2, 1)
 
     def run_test(self):
+        self.activate_v19(expected_activation_height=900)
 
         masternode = self.mninfo[0]
         masternode.node.add_p2p_connection(P2PInterface())
 
         protx_hash = masternode.proTxHash
-        #TODO: Fix that with basic BLS
         public_key = masternode.pubKeyOperator
 
         # The peerinfo should not yet contain verified_proregtx_hash/verified_pubkey_hash