fix: undefined behaviour in evo RPC calls of RPCHelpMan

The implementation of RPC has been refactored in bitcoin significantly with using RPCHelpMan, see multiple PR, such as bitcoin#1853, bitcoin#19528, etc Backporting them and appliying same refactoring to Dash Core caused undefined behaviour, particularly dashpay#6072 For example, in this code the local variable `use_legacy` will be used after `protx_update_registrar_wrapper`, when executor will be called. static RPCHelpMan protx_update_registrar_wrapper(const bool use_legacy) { std::string rpc_name = use_legacy ? "update_registrar_legacy" : "update_registrar"; std::string rpc_full_name = std::string("protx ").append(rpc_name); std::string pubkey_operator = use_legacy ? "\"0532646990082f4fd639f90387b1551f2c7c39d37392cb9055a06a7e85c1d23692db8f87f827886310bccc1e29db9aee\"" : "\"8532646990082f4fd639f90387b1551f2c7c39d37392cb9055a06a7e85c1d23692db8f87f827886310bccc1e29db9aee\""; std::string rpc_example = rpc_name.append(" \"0123456701234567012345670123456701234567012345670123456701234567\" ").append(pubkey_operator).append(" \"" + EXAMPLE_ADDRESS[1] + "\""); return RPCHelpMan{rpc_full_name, <...> RPCResult{ RPCResult::Type::STR_HEX, "txid", "The transaction id" }, RPCExamples{ HelpExampleCli("protx", rpc_example) }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { <...> ptx.nVersion = specific_legacy_bls_scheme ? CProUpRegTx::LEGACY_BLS_VERSION : CProUpRegTx::BASIC_BLS_VERSION; <<<<---- THERE IS UB It can be easy tested by adding debug logs to log string, for example rpc_full_name at the moment of call of executor has been during run: 2024-12-26T16:10:15Z rpc full name: 'r,ߧzu\x00\x00��m��istrar_legacy' This PR fixes multiple unexplainable random failures which had happen only for `tsan` build or for only `ubsan` build during debuggin dashpay#6508 and depends only on code revision.
knst · Dec 26, 2024 · 1cabb75 · 1cabb75
1 parent 26ad3d3
commit 1cabb75
Showing 1 changed file with 6 additions and 7 deletions.
diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp
@@ -398,7 +398,7 @@ static RPCHelpMan protx_register_fund_wrapper(const bool legacy)
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    return protx_register_common_wrapper(request, legacy, ProTxRegisterAction::Fund, MnType::Regular);
+    return protx_register_common_wrapper(request, self.m_name == "protx register_fund_legacy", ProTxRegisterAction::Fund, MnType::Regular);
 },
     };
 }
@@ -445,7 +445,7 @@ static RPCHelpMan protx_register_wrapper(bool legacy)
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    return protx_register_common_wrapper(request, legacy, ProTxRegisterAction::External, MnType::Regular);
+    return protx_register_common_wrapper(request, self.m_name == "protx register_legacy", ProTxRegisterAction::External, MnType::Regular);
 },
     };
 }
@@ -493,7 +493,7 @@ static RPCHelpMan protx_register_prepare_wrapper(const bool legacy)
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    return protx_register_common_wrapper(request, legacy, ProTxRegisterAction::Prepare, MnType::Regular);
+    return protx_register_common_wrapper(request, self.m_name == "protx register_prepare_legacy", ProTxRegisterAction::Prepare, MnType::Regular);
 },
     };
 }
@@ -1060,7 +1060,7 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques
     return SignAndSendSpecialTx(request, chain_helper, chainman, tx);
 }
 
-static RPCHelpMan protx_update_registrar_wrapper(bool specific_legacy_bls_scheme)
+static RPCHelpMan protx_update_registrar_wrapper(const bool specific_legacy_bls_scheme)
 {
     std::string rpc_name = specific_legacy_bls_scheme ? "update_registrar_legacy" : "update_registrar";
     std::string rpc_full_name = std::string("protx ").append(rpc_name);
@@ -1073,7 +1073,7 @@ static RPCHelpMan protx_update_registrar_wrapper(bool specific_legacy_bls_scheme
         + HELP_REQUIRING_PASSPHRASE,
         {
             GetRpcArg("proTxHash"),
-            specific_legacy_bls_scheme  ? GetRpcArg("operatorPubKey_update_legacy") : GetRpcArg("operatorPubKey_update"),
+            specific_legacy_bls_scheme ? GetRpcArg("operatorPubKey_update_legacy") : GetRpcArg("operatorPubKey_update"),
             GetRpcArg("votingAddress_update"),
             GetRpcArg("payoutAddress_update"),
             GetRpcArg("feeSourceAddress"),
@@ -1086,6 +1086,7 @@ static RPCHelpMan protx_update_registrar_wrapper(bool specific_legacy_bls_scheme
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
+    const bool use_legacy{self.m_name == "protx update_registrar_legacy"};
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
 
@@ -1107,8 +1108,6 @@ static RPCHelpMan protx_update_registrar_wrapper(bool specific_legacy_bls_scheme
     ptx.keyIDVoting = dmn->pdmnState->keyIDVoting;
     ptx.scriptPayout = dmn->pdmnState->scriptPayout;
 
-    const bool use_legacy = specific_legacy_bls_scheme;
-
     if (request.params[1].get_str() != "") {
         // new pubkey
         ptx.pubKeyOperator.Set(ParseBLSPubKey(request.params[1].get_str(), "operator BLS address", use_legacy), use_legacy);