Merge #6115: backport: v21.0.x rc.2

db82817 Merge #6106: feat: create new composite quorum-command platformsign (pasta)
a45e6df Merge #6104: fix: adjust incorrect parameter description that says there is a default that doesn't exist (pasta)
7330982 Merge #6100: feat: make whitelist works with composite commands for platform needs (pasta)
9998ffd Merge #6096: feat: split type of error in submitchainlock - return enum in CL verifying code (pasta)
cdf7a25 Merge #6095: fix: createwallet to require 'load_on_startup' for descriptor wallets (pasta)
c1c2c55 Merge #6092: fix: mixing for partially unlocked descriptor wallets (pasta)
1175486 Merge #6073: feat: add logging for RPC HTTP requests: command, user, http-code, time of running (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Backports a set of 6 PRs needed in rc.2

  ## What was done?
  Backported PRs with labels

  ## How Has This Been Tested?

  ## Breaking Changes

  ## 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:
  kwvg:
    LGTM, utACK db82817
  UdjinM6:
    utACK db82817

Tree-SHA512: 1b242c5db04bd5873ef622543bc2a25e29567f15962c677ea51ff05cb784291968d18f419bf611c206b912e8f15d687208ae75af33aab89038b6f0167d99c4bf

Author: PastaPastaPasta

doc/release-notes-16528.md

@@ -35,6 +35,7 @@ Descriptor Wallet should be created.
 
 Without those options being set, a Legacy Wallet will be created instead.
 
+
 #### `IsMine` Semantics
 
 `IsMine` refers to the function used to determine whether a script belongs to the wallet.
@@ -117,3 +118,4 @@ descriptors with private keys for now as explained earlier.
 ## RPC changes
  - `createwallet` has changed list of arguments: `createwallet "wallet_name" ( disable_private_keys blank "passphrase" avoid_reuse descriptors load_on_startup )`
 `load_on_startup` used to be an argument 5 but now has a number 6.
+ - `createwallet` requires specifying the `load_on_startup` flag when creating descriptor wallets due to breaking changes in v21.

doc/release-notes-6100.md

@@ -0,0 +1,9 @@
+## Remote Procedure Call (RPC) Changes
+
+### Improved support of composite commands
+
+Dash Core's composite commands such as `quorum list` or `bls generate` now are compatible with a whitelist feature.
+
+For example, whitelist `getblockcount,quorumlist` will let to call commands `getblockcount`, `quorum list`, but not `quorum sign`
+
+Note, that adding simple `quorum` in whitelist will allow to use all kind of `quorum...` commands, such as `quorum`, `quorum list`, `quorum sign`, etc

doc/release-notes-6106.md

@@ -0,0 +1,6 @@
+## Remote Procedure Call (RPC) Changes
+
+### The new RPCs are:
+
+- `quorum signplatform` This RPC is added for Platform needs. This composite command let to limit quorum type for signing by platform. It is equivalent of `quorum sign <platform type>`.
+

src/chainparamsbase.cpp

@@ -25,7 +25,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman)
     argsman.AddArg("-highsubsidyblocks=<n>", "The number of blocks with a higher than normal subsidy to mine at the start of a chain. Block after that height will have fixed subsidy base. (default: 0, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-highsubsidyfactor=<n>", "The factor to multiply the normal block subsidy by while in the highsubsidyblocks window of a chain (default: 1, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-llmqchainlocks=<quorum name>", "Override the default LLMQ type used for ChainLocks. Allows using ChainLocks with smaller LLMQs. (default: llmq_devnet, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
-    argsman.AddArg("-llmqdevnetparams=<size>:<threshold>", "Override the default LLMQ size for the LLMQ_DEVNET quorum (default: 3:2, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
+    argsman.AddArg("-llmqdevnetparams=<size>:<threshold>", "Override the default LLMQ size for the LLMQ_DEVNET quorum (devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-llmqinstantsenddip0024=<quorum name>", "Override the default LLMQ type used for InstantSendDIP0024. (default: llmq_devnet_dip0024, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-llmqplatform=<quorum name>", "Override the default LLMQ type used for Platform. (default: llmq_devnet_platform, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-llmqmnhf=<quorum name>", "Override the default LLMQ type used for EHF. (default: llmq_devnet, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);

src/evo/cbtx.cpp

@@ -374,7 +374,7 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex,
             return true;
         }
         uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash();
-        if (!chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, opt_cbTx->bestCLSignature))) {
+        if (chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, opt_cbTx->bestCLSignature)) != llmq::VerifyRecSigStatus::Valid) {
             return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-invalid-clsig");
         }
     } else if (opt_cbTx->bestCLHeightDiff != 0) {

src/httprpc.cpp

@@ -72,7 +72,47 @@ static std::vector<std::vector<std::string>> g_rpcauth;
 static std::map<std::string, std::set<std::string>> g_rpc_whitelist;
 static bool g_rpc_whitelist_default = false;
 
-static void JSONErrorReply(HTTPRequest* req, const UniValue& objError, const UniValue& id)
+extern std::vector<std::string> g_external_usernames;
+class RpcHttpRequest
+{
+public:
+    HTTPRequest* m_req;
+    int64_t m_startTime;
+    int m_status{0};
+    std::string user;
+    std::string command;
+
+    RpcHttpRequest(HTTPRequest* req) :
+        m_req{req},
+        m_startTime{GetTimeMicros()}
+    {}
+
+    ~RpcHttpRequest()
+    {
+        const bool is_external = find(g_external_usernames.begin(), g_external_usernames.end(), user) != g_external_usernames.end();
+        LogPrint(BCLog::BENCHMARK, "HTTP RPC request handled: user=%s command=%s external=%s status=%d elapsed_time_ms=%d\n", user, command, is_external, m_status, (GetTimeMicros() - m_startTime) / 1000);
+    }
+
+    bool send_reply(int status, const std::string& response = "")
+    {
+        m_status = status;
+        m_req->WriteReply(status, response);
+        return m_status == HTTP_OK;
+    }
+};
+
+static bool whitelisted(JSONRPCRequest jreq)
+{
+    if (g_rpc_whitelist[jreq.authUser].count(jreq.strMethod)) return true;
+
+    // check for composite command after
+    if (!jreq.params.isArray() || jreq.params.empty()) return false;
+    if (!jreq.params[0].isStr()) return false;
+
+    return g_rpc_whitelist[jreq.authUser].count(jreq.strMethod + jreq.params[0].get_str());
+}
+
+static bool JSONErrorReply(RpcHttpRequest& rpcRequest, const UniValue& objError, const UniValue& id)
 {
     // Send error reply from json-rpc error object
     int nStatus = HTTP_INTERNAL_SERVER_ERROR;
@@ -88,8 +128,8 @@ static void JSONErrorReply(HTTPRequest* req, const UniValue& objError, const Uni
 
     std::string strReply = JSONRPCReply(NullUniValue, objError, id);
 
-    req->WriteHeader("Content-Type", "application/json");
-    req->WriteReply(nStatus, strReply);
+    rpcRequest.m_req->WriteHeader("Content-Type", "application/json");
+    return rpcRequest.send_reply(nStatus, strReply);
 }
 
 //This function checks username and password against -rpcauth
@@ -146,24 +186,25 @@ static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUserna
     return multiUserAuthorized(strUserPass);
 }
 
-static bool HTTPReq_JSONRPC(const CoreContext& context, HTTPRequest* req, bool external = false)
+static bool HTTPReq_JSONRPC(const CoreContext& context, HTTPRequest* req)
 {
+    RpcHttpRequest rpcRequest(req);
+
     // JSONRPC handles only POST
     if (req->GetRequestMethod() != HTTPRequest::POST) {
-        req->WriteReply(HTTP_BAD_METHOD, "JSONRPC server handles only POST requests");
-        return false;
+        return rpcRequest.send_reply(HTTP_BAD_METHOD, "JSONRPC server handles only POST requests");
     }
     // Check authorization
     std::pair<bool, std::string> authHeader = req->GetHeader("authorization");
     if (!authHeader.first) {
         req->WriteHeader("WWW-Authenticate", WWW_AUTH_HEADER_DATA);
-        req->WriteReply(HTTP_UNAUTHORIZED);
-        return false;
+        return rpcRequest.send_reply(HTTP_UNAUTHORIZED);
     }
 
     JSONRPCRequest jreq(context);
+
     jreq.peerAddr = req->GetPeer().ToString();
-    if (!RPCAuthorized(authHeader.second, jreq.authUser)) {
+    if (!RPCAuthorized(authHeader.second, rpcRequest.user)) {
         LogPrintf("ThreadRPCServer incorrect password attempt from %s\n", jreq.peerAddr);
 
         /* Deter brute-forcing
@@ -172,9 +213,9 @@ static bool HTTPReq_JSONRPC(const CoreContext& context, HTTPRequest* req, bool e
         UninterruptibleSleep(std::chrono::milliseconds{250});
 
         req->WriteHeader("WWW-Authenticate", WWW_AUTH_HEADER_DATA);
-        req->WriteReply(HTTP_UNAUTHORIZED);
-        return false;
+        return rpcRequest.send_reply(HTTP_UNAUTHORIZED);
     }
+    jreq.authUser = rpcRequest.user;
 
     try {
         // Parse request
@@ -189,16 +230,16 @@ static bool HTTPReq_JSONRPC(const CoreContext& context, HTTPRequest* req, bool e
         bool user_has_whitelist = g_rpc_whitelist.count(jreq.authUser);
         if (!user_has_whitelist && g_rpc_whitelist_default) {
             LogPrintf("RPC User %s not allowed to call any methods\n", jreq.authUser);
-            req->WriteReply(HTTP_FORBIDDEN);
-            return false;
+            return rpcRequest.send_reply(HTTP_FORBIDDEN);
 
         // singleton request
         } else if (valRequest.isObject()) {
             jreq.parse(valRequest);
-            if (user_has_whitelist && !g_rpc_whitelist[jreq.authUser].count(jreq.strMethod)) {
+            rpcRequest.command = jreq.strMethod;
+
+            if (user_has_whitelist && !whitelisted(jreq)) {
                 LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, jreq.strMethod);
-                req->WriteReply(HTTP_FORBIDDEN);
-                return false;
+                return rpcRequest.send_reply(HTTP_FORBIDDEN);
             }
             UniValue result = tableRPC.execute(jreq);
 
@@ -215,10 +256,9 @@ static bool HTTPReq_JSONRPC(const CoreContext& context, HTTPRequest* req, bool e
                         const UniValue& request = valRequest[reqIdx].get_obj();
                         // Parse method
                         std::string strMethod = find_value(request, "method").get_str();
-                        if (!g_rpc_whitelist[jreq.authUser].count(strMethod)) {
+                        if (!whitelisted(jreq)) {
                             LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, strMethod);
-                            req->WriteReply(HTTP_FORBIDDEN);
-                            return false;
+                            return rpcRequest.send_reply(HTTP_FORBIDDEN);
                         }
                     }
                 }
@@ -229,15 +269,13 @@ static bool HTTPReq_JSONRPC(const CoreContext& context, HTTPRequest* req, bool e
             throw JSONRPCError(RPC_PARSE_ERROR, "Top-level object parse error");
 
         req->WriteHeader("Content-Type", "application/json");
-        req->WriteReply(HTTP_OK, strReply);
+        return rpcRequest.send_reply(HTTP_OK, strReply);
     } catch (const UniValue& objError) {
-        JSONErrorReply(req, objError, jreq.id);
-        return false;
+        return JSONErrorReply(rpcRequest, objError, jreq.id);
     } catch (const std::exception& e) {
-        JSONErrorReply(req, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id);
-        return false;
+        return JSONErrorReply(rpcRequest, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id);
     }
-    return true;
+    assert(false);
 }
 
 static bool InitRPCAuthentication()

src/httpserver.cpp

@@ -154,8 +154,8 @@ static struct evhttp* eventHTTP = nullptr;
 static std::vector<CSubNet> rpc_allow_subnets;
 //! Work queue for handling longer requests off the event loop thread
 static std::unique_ptr<WorkQueue<HTTPClosure>> g_work_queue{nullptr};
-//! List of 'external' RPC users
-static std::vector<std::string> g_external_usernames;
+//! List of 'external' RPC users (global variable, used by httprpc)
+std::vector<std::string> g_external_usernames;
 //! Handlers for (sub)paths
 static std::vector<HTTPPathHandler> pathHandlers;
 //! Bound listening sockets

src/llmq/chainlocks.cpp

@@ -19,6 +19,7 @@
 #include <txmempool.h>
 #include <util/thread.h>
 #include <util/time.h>
+#include <util/underlying.h>
 #include <validation.h>
 #include <validationinterface.h>
 
@@ -130,8 +131,8 @@ PeerMsgRet CChainLocksHandler::ProcessNewChainLock(const NodeId from, const llmq
         }
     }
 
-    if (!VerifyChainLock(clsig)) {
-        LogPrint(BCLog::CHAINLOCKS, "CChainLocksHandler::%s -- invalid CLSIG (%s), peer=%d\n", __func__, clsig.ToString(), from);
+    if (const auto ret = VerifyChainLock(clsig); ret != VerifyRecSigStatus::Valid) {
+        LogPrint(BCLog::CHAINLOCKS, "CChainLocksHandler::%s -- invalid CLSIG (%s), status=%d peer=%d\n", __func__, clsig.ToString(), ToUnderlying(ret), from);
         if (from != -1) {
             return tl::unexpected{10};
         }
@@ -551,10 +552,12 @@ bool CChainLocksHandler::HasChainLock(int nHeight, const uint256& blockHash) con
     return InternalHasChainLock(nHeight, blockHash);
 }
 
-bool CChainLocksHandler::VerifyChainLock(const CChainLockSig& clsig) const
+
+VerifyRecSigStatus CChainLocksHandler::VerifyChainLock(const CChainLockSig& clsig) const
 {
     const auto llmqType = Params().GetConsensus().llmqTypeChainLocks;
     const uint256 nRequestId = ::SerializeHash(std::make_pair(llmq::CLSIG_REQUESTID_PREFIX, clsig.getHeight()));
+
     return llmq::VerifyRecoveredSig(llmqType, m_chainstate.m_chain, qman, clsig.getHeight(), nRequestId, clsig.getBlockHash(), clsig.getSig());
 }
 

src/llmq/chainlocks.h

@@ -9,6 +9,7 @@
 
 #include <crypto/common.h>
 #include <llmq/signing.h>
+#include <llmq/quorums.h>
 #include <net.h>
 #include <net_types.h>
 #include <primitives/block.h>
@@ -114,7 +115,7 @@ class CChainLocksHandler : public CRecoveredSigsListener
 
     bool HasChainLock(int nHeight, const uint256& blockHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
     bool HasConflictingChainLock(int nHeight, const uint256& blockHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
-    bool VerifyChainLock(const CChainLockSig& clsig) const;
+    VerifyRecSigStatus VerifyChainLock(const CChainLockSig& clsig) const;
 
     bool IsTxSafeForMining(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
 

src/llmq/quorums.cpp

@@ -1169,18 +1169,19 @@ CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, con
     }
 }
 
-bool VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain& active_chain, const CQuorumManager& qman,
+VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain& active_chain, const CQuorumManager& qman,
                         int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig,
                         const int signOffset)
 {
     const auto& llmq_params_opt = Params().GetLLMQ(llmqType);
     assert(llmq_params_opt.has_value());
     auto quorum = SelectQuorumForSigning(llmq_params_opt.value(), active_chain, qman, id, signedAtHeight, signOffset);
     if (!quorum) {
-        return false;
+        return VerifyRecSigStatus::NoQuorum;
     }
 
     uint256 signHash = BuildSignHash(llmqType, quorum->qc->quorumHash, id, msgHash);
-    return sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash);
+    const bool ret = sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash);
+    return ret ? VerifyRecSigStatus::Valid : VerifyRecSigStatus::Invalid;
 }
 } // namespace llmq

src/llmq/quorums.h

@@ -36,6 +36,13 @@ using CDeterministicMNCPtr = std::shared_ptr<const CDeterministicMN>;
 
 namespace llmq
 {
+enum class VerifyRecSigStatus
+{
+    NoQuorum,
+    Invalid,
+    Valid,
+};
+
 class CDKGSessionManager;
 class CQuorumBlockProcessor;
 
@@ -298,9 +305,9 @@ CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, con
                                    const uint256& selectionHash, int signHeight = -1 /*chain tip*/, int signOffset = SIGN_HEIGHT_OFFSET);
 
 // Verifies a recovered sig that was signed while the chain tip was at signedAtTip
-bool VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain& active_chain, const CQuorumManager& qman,
-                        int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig,
-                        int signOffset = SIGN_HEIGHT_OFFSET);
+VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain& active_chain, const CQuorumManager& qman,
+                                      int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig,
+                                      int signOffset = SIGN_HEIGHT_OFFSET);
 } // namespace llmq
 
 template<typename T> struct SaltedHasherImpl;

src/llmq/signing.cpp

@@ -578,6 +578,30 @@ PeerMsgRet CSigningManager::ProcessMessage(const CNode& pfrom, const std::string
     return {};
 }
 
+static bool PreVerifyRecoveredSig(const CQuorumManager& quorum_manager, const CRecoveredSig& recoveredSig, bool& retBan)
+{
+    retBan = false;
+
+    auto llmqType = recoveredSig.getLlmqType();
+    if (!Params().GetLLMQ(llmqType).has_value()) {
+        retBan = true;
+        return false;
+    }
+
+    CQuorumCPtr quorum = quorum_manager.GetQuorum(llmqType, recoveredSig.getQuorumHash());
+
+    if (!quorum) {
+        LogPrint(BCLog::LLMQ, "CSigningManager::%s -- quorum %s not found\n", __func__,
+                  recoveredSig.getQuorumHash().ToString());
+        return false;
+    }
+    if (!IsQuorumActive(llmqType, quorum_manager, quorum->qc->quorumHash)) {
+        return false;
+    }
+
+    return true;
+}
+
 PeerMsgRet CSigningManager::ProcessMessageRecoveredSig(const CNode& pfrom, const std::shared_ptr<const CRecoveredSig>& recoveredSig)
 {
     {
@@ -614,30 +638,6 @@ PeerMsgRet CSigningManager::ProcessMessageRecoveredSig(const CNode& pfrom, const
     return {};
 }
 
-bool CSigningManager::PreVerifyRecoveredSig(const CQuorumManager& quorum_manager, const CRecoveredSig& recoveredSig, bool& retBan)
-{
-    retBan = false;
-
-    auto llmqType = recoveredSig.getLlmqType();
-    if (!Params().GetLLMQ(llmqType).has_value()) {
-        retBan = true;
-        return false;
-    }
-
-    CQuorumCPtr quorum = quorum_manager.GetQuorum(llmqType, recoveredSig.getQuorumHash());
-
-    if (!quorum) {
-        LogPrint(BCLog::LLMQ, "CSigningManager::%s -- quorum %s not found\n", __func__,
-                  recoveredSig.getQuorumHash().ToString());
-        return false;
-    }
-    if (!IsQuorumActive(llmqType, quorum_manager, quorum->qc->quorumHash)) {
-        return false;
-    }
-
-    return true;
-}
-
 void CSigningManager::CollectPendingRecoveredSigsToVerify(
         size_t maxUniqueSessions,
         std::unordered_map<NodeId, std::list<std::shared_ptr<const CRecoveredSig>>>& retSigShares,

src/llmq/signing.h

@@ -201,7 +201,6 @@ class CSigningManager
 
 private:
     PeerMsgRet ProcessMessageRecoveredSig(const CNode& pfrom, const std::shared_ptr<const CRecoveredSig>& recoveredSig);
-    static bool PreVerifyRecoveredSig(const CQuorumManager& quorum_manager, const CRecoveredSig& recoveredSig, bool& retBan);
 
     void CollectPendingRecoveredSigsToVerify(size_t maxUniqueSessions,
             std::unordered_map<NodeId, std::list<std::shared_ptr<const CRecoveredSig>>>& retSigShares,