Merge pull request #5488 from vijaydasmp/bp22_1

backport: Merge bitcoin#20408,19836,19851,(partial)21424, 21948, 20056,20432,19337,20448, 21010

src/core_read.cpp

@@ -17,10 +17,10 @@
 
 #include <algorithm>
 
-CScript ParseScript(const std::string& s)
+namespace {
-{
-    CScript result;
 
+opcodetype ParseOpCode(const std::string& s)
+{
     static std::map<std::string, opcodetype> mapOpNames;
 
     if (mapOpNames.empty())
@@ -42,6 +42,17 @@ CScript ParseScript(const std::string& s)
             }
         }
     }
+    auto it = mapOpNames.find(s);
+    if (it == mapOpNames.end()) throw std::runtime_error("script parse error: unknown opcode");
+    return it->second;
+}
+
+} // namespace
+
+CScript ParseScript(const std::string& s)
+{
+    CScript result;
+
 
     std::vector<std::string> words = SplitString(s, " \t\n");
 
@@ -79,14 +90,10 @@ CScript ParseScript(const std::string& s)
             std::vector<unsigned char> value(w->begin()+1, w->end()-1);
             result << value;
         }
-        else if (mapOpNames.count(*w))
-        {
-            // opcode, e.g. OP_ADD or ADD:
-            result << mapOpNames[*w];
-        }
         else
         {
-            throw std::runtime_error("script parse error");
+            // opcode, e.g. OP_ADD or ADD:
+            result << ParseOpCode(*w);
         }
     }
 

src/net.cpp

@@ -676,34 +676,21 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
 }
 #undef X
 
-/**
+bool CNode::ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete)
- * Receive bytes from the buffer and deserialize them into messages.
- *
- * @param[in]   pch         A pointer to the raw data
- * @param[in]   nBytes      Size of the data
- * @param[out]  complete    Set True if at least one message has been
- *                          deserialized and is ready to be processed
- * @return  True if the peer should stay connected,
- *          False if the peer should be disconnected from.
- */
-bool CNode::ReceiveMsgBytes(const char *pch, unsigned int nBytes, bool& complete)
 {
     complete = false;
     int64_t nTimeMicros = GetTimeMicros();
     LOCK(cs_vRecv);
     nLastRecv = nTimeMicros / 1000000;
-    nRecvBytes += nBytes;
+    nRecvBytes += msg_bytes.size();
-    while (nBytes > 0) {
+    while (msg_bytes.size() > 0) {
         // absorb network data
-        int handled = m_deserializer->Read(pch, nBytes);
+        int handled = m_deserializer->Read(msg_bytes);
         if (handled < 0) {
             // Serious header problem, disconnect from the peer.
             return false;
         }
 
-        pch += handled;
-        nBytes -= handled;
-
         if (m_deserializer->Complete()) {
             // decompose a transport agnostic CNetMessage from the deserializer
             uint32_t out_err_raw_size{0};
@@ -760,13 +747,13 @@ int CNode::GetSendVersion() const
     return nSendVersion;
 }
 
-int V1TransportDeserializer::readHeader(const char *pch, unsigned int nBytes)
+int V1TransportDeserializer::readHeader(Span<const uint8_t> msg_bytes)
 {
     // copy data to temporary parsing buffer
     unsigned int nRemaining = 24 - nHdrPos;
-    unsigned int nCopy = std::min(nRemaining, nBytes);
+    unsigned int nCopy = std::min<unsigned int>(nRemaining, msg_bytes.size());
 
-    memcpy(&hdrbuf[nHdrPos], pch, nCopy);
+    memcpy(&hdrbuf[nHdrPos], msg_bytes.data(), nCopy);
     nHdrPos += nCopy;
 
     // if header incomplete, exit
@@ -800,18 +787,18 @@ int V1TransportDeserializer::readHeader(const char *pch, unsigned int nBytes)
     return nCopy;
 }
 
-int V1TransportDeserializer::readData(const char *pch, unsigned int nBytes)
+int V1TransportDeserializer::readData(Span<const uint8_t> msg_bytes)
 {
     unsigned int nRemaining = hdr.nMessageSize - nDataPos;
-    unsigned int nCopy = std::min(nRemaining, nBytes);
+    unsigned int nCopy = std::min<unsigned int>(nRemaining, msg_bytes.size());
 
     if (vRecv.size() < nDataPos + nCopy) {
         // Allocate up to 256 KiB ahead, but never more than the total message size.
         vRecv.resize(std::min(hdr.nMessageSize, nDataPos + nCopy + 256 * 1024));
     }
 
-    hasher.Write({(const unsigned char*)pch, nCopy});
+    hasher.Write(msg_bytes.first(nCopy));
-    memcpy(&vRecv[nDataPos], pch, nCopy);
+    memcpy(&vRecv[nDataPos], msg_bytes.data(), nCopy);
     nDataPos += nCopy;
 
     return nCopy;
@@ -1925,13 +1912,13 @@ void CConnman::SocketHandler()
 size_t CConnman::SocketRecvData(CNode *pnode)
 {
     // typical socket buffer is 8K-64K
-    char pchBuf[0x10000];
+    uint8_t pchBuf[0x10000];
     int nBytes = 0;
     {
         LOCK(pnode->cs_hSocket);
         if (pnode->hSocket == INVALID_SOCKET)
             return 0;
-        nBytes = recv(pnode->hSocket, pchBuf, sizeof(pchBuf), MSG_DONTWAIT);
+        nBytes = recv(pnode->hSocket, (char*)pchBuf, sizeof(pchBuf), MSG_DONTWAIT);
         if (nBytes < (int)sizeof(pchBuf)) {
             pnode->fHasRecvData = false;
         }
@@ -1939,7 +1926,7 @@ size_t CConnman::SocketRecvData(CNode *pnode)
     if (nBytes > 0)
     {
         bool notify = false;
-        if (!pnode->ReceiveMsgBytes(pchBuf, nBytes, notify)) {
+        if (!pnode->ReceiveMsgBytes(Span<const uint8_t>(pchBuf, nBytes), notify)) {
             LOCK(cs_vNodes);
             pnode->CloseSocketDisconnect(this);
         }
@@ -3080,17 +3067,6 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
 {
     Init(connOptions);
 
-    {
-        LOCK(cs_totalBytesRecv);
-        nTotalBytesRecv = 0;
-    }
-    {
-        LOCK(cs_totalBytesSent);
-        nTotalBytesSent = 0;
-        nMaxOutboundTotalBytesSentInCycle = 0;
-        nMaxOutboundCycleStartTime = 0s;
-    }
-
 #ifdef USE_KQUEUE
     if (socketEventsMode == SOCKETEVENTS_KQUEUE) {
         kqueuefd = kqueue();

src/net.h

@@ -591,8 +591,8 @@ private:
     uint64_t nTotalBytesSent GUARDED_BY(cs_totalBytesSent) {0};
 
     // outbound limit & stats
-    uint64_t nMaxOutboundTotalBytesSentInCycle GUARDED_BY(cs_totalBytesSent);
+    uint64_t nMaxOutboundTotalBytesSentInCycle GUARDED_BY(cs_totalBytesSent) {0};
-    std::chrono::seconds nMaxOutboundCycleStartTime GUARDED_BY(cs_totalBytesSent);
+    std::chrono::seconds nMaxOutboundCycleStartTime GUARDED_BY(cs_totalBytesSent) {0};
     uint64_t nMaxOutboundLimit GUARDED_BY(cs_totalBytesSent);
 
     // P2P timeout in seconds
@@ -933,8 +933,8 @@ public:
     virtual bool Complete() const = 0;
     // set the serialization context version
     virtual void SetVersion(int version) = 0;
-    // read and deserialize data
+    /** read and deserialize data, advances msg_bytes data pointer */
-    virtual int Read(const char *data, unsigned int bytes) = 0;
+    virtual int Read(Span<const uint8_t>& msg_bytes) = 0;
     // decomposes a message from the context
     virtual std::optional<CNetMessage> GetMessage(int64_t time, uint32_t& out_err) = 0;
     virtual ~TransportDeserializer() {}
@@ -955,8 +955,8 @@ private:
     unsigned int nDataPos;
 
     const uint256& GetMessageHash() const;
-    int readHeader(const char *pch, unsigned int nBytes);
+    int readHeader(Span<const uint8_t> msg_bytes);
-    int readData(const char *pch, unsigned int nBytes);
+    int readData(Span<const uint8_t> msg_bytes);
 
     void Reset() {
         vRecv.clear();
@@ -990,9 +990,14 @@ public:
         hdrbuf.SetVersion(nVersionIn);
         vRecv.SetVersion(nVersionIn);
     }
-    int Read(const char *pch, unsigned int nBytes) override {
+    int Read(Span<const uint8_t>& msg_bytes) override
-        int ret = in_data ? readData(pch, nBytes) : readHeader(pch, nBytes);
+    {
-        if (ret < 0) Reset();
+        int ret = in_data ? readData(msg_bytes) : readHeader(msg_bytes);
+        if (ret < 0) {
+            Reset();
+        } else {
+            msg_bytes = msg_bytes.subspan(ret);
+        }
         return ret;
     }
     std::optional<CNetMessage> GetMessage(int64_t time, uint32_t& out_err_raw_size) override;
@@ -1271,7 +1276,16 @@ public:
         return nRefCount;
     }
 
-    bool ReceiveMsgBytes(const char *pch, unsigned int nBytes, bool& complete);
+    /**
+     * Receive bytes from the buffer and deserialize them into messages.
+     *
+     * @param[in]   msg_bytes   The raw data
+     * @param[out]  complete    Set True if at least one message has been
+     *                          deserialized and is ready to be processed
+     * @return  True if the peer should stay connected,
+     *          False if the peer should be disconnected from.
+     */
+    bool ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete);
 
     void SetRecvVersion(int nVersionIn)
     {

src/net_processing.cpp

@@ -589,13 +589,13 @@ struct CNodeState {
       */
     struct ChainSyncTimeoutState {
         //! A timeout used for checking whether our peer has sufficiently synced
-        int64_t m_timeout;
+        int64_t m_timeout{0};
         //! A header with the work we require on our peer's chain
-        const CBlockIndex * m_work_header;
+        const CBlockIndex* m_work_header{nullptr};
         //! After timeout is reached, set to true after sending getheaders
-        bool m_sent_getheaders;
+        bool m_sent_getheaders{false};
         //! Whether this peer is protected from disconnection due to a bad/slow chain
-        bool m_protect;
+        bool m_protect{false};
     };
 
     ChainSyncTimeoutState m_chain_sync;

src/rpc/mining.cpp

@@ -352,7 +352,7 @@ static UniValue generateblock(const JSONRPCRequest& request)
             txs.push_back(MakeTransactionRef(std::move(mtx)));
 
         } else {
-            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("Transaction decode failed for %s", str));
+            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("Transaction decode failed for %s. Make sure the tx has at least one input.", str));
         }
     }
 

src/rpc/misc.cpp

@@ -1097,7 +1097,7 @@ static UniValue mockscheduler(const JSONRPCRequest& request)
     // check params are valid values
     RPCTypeCheck(request.params, {UniValue::VNUM});
     int64_t delta_seconds = request.params[0].get_int64();
-    if ((delta_seconds <= 0) || (delta_seconds > 3600)) {
+    if (delta_seconds <= 0 || delta_seconds > 3600) {
         throw std::runtime_error("delta_time must be between 1 and 3600 seconds (1 hr)");
     }
 

src/rpc/rawtransaction.cpp

@@ -636,7 +636,7 @@ static UniValue combinerawtransaction(const JSONRPCRequest& request)
 
     for (unsigned int idx = 0; idx < txs.size(); idx++) {
         if (!DecodeHexTx(txVariants[idx], txs[idx].get_str())) {
-            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed for tx %d", idx));
+            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed for tx %d. Make sure the tx has at least one input.", idx));
         }
     }
 
@@ -757,7 +757,7 @@ static UniValue signrawtransactionwithkey(const JSONRPCRequest& request)
 
     CMutableTransaction mtx;
     if (!DecodeHexTx(mtx, request.params[0].get_str())) {
-        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
+        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input.");
     }
 
     FillableSigningProvider keystore;
@@ -823,10 +823,10 @@ UniValue sendrawtransaction(const JSONRPCRequest& request)
         UniValue::VBOOL
     });
 
-    // parse hex string from parameter
     CMutableTransaction mtx;
-    if (!DecodeHexTx(mtx, request.params[0].get_str()))
+    if (!DecodeHexTx(mtx, request.params[0].get_str())) {
-        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
+        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input.");
+    }
     CTransactionRef tx(MakeTransactionRef(std::move(mtx)));
 
     const CFeeRate max_raw_tx_fee_rate = request.params[1].isNull() ?
@@ -898,7 +898,7 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request)
 
     CMutableTransaction mtx;
     if (!DecodeHexTx(mtx, request.params[0].get_array()[0].get_str())) {
-        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
+        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input.");
     }
     CTransactionRef tx(MakeTransactionRef(std::move(mtx)));
     const uint256& tx_hash = tx->GetHash();

src/scheduler.cpp

@@ -5,6 +5,7 @@
 #include <scheduler.h>
 
 #include <random.h>
+#include <util/time.h>
 
 #include <assert.h>
 #include <utility>
@@ -79,7 +80,7 @@ void CScheduler::schedule(CScheduler::Function f, std::chrono::system_clock::tim
 
 void CScheduler::MockForward(std::chrono::seconds delta_seconds)
 {
-    assert(delta_seconds.count() > 0 && delta_seconds < std::chrono::hours{1});
+    assert(delta_seconds > 0s && delta_seconds <= 1h);
 
     {
         LOCK(newTaskMutex);

src/sync.cpp

@@ -13,11 +13,12 @@
 #include <util/strencodings.h>
 #include <util/threadnames.h>
 
-
 #include <map>
+#include <mutex>
 #include <set>
 #include <system_error>
 #include <thread>
+#include <type_traits>
 #include <unordered_map>
 #include <utility>
 #include <vector>
@@ -139,16 +140,50 @@ static void potential_deadlock_detected(const LockPair& mismatch, const LockStac
     throw std::logic_error(strprintf("potential deadlock detected: %s -> %s -> %s", mutex_b, mutex_a, mutex_b));
 }
 
-static void push_lock(void* c, const CLockLocation& locklocation)
+static void double_lock_detected(const void* mutex, LockStack& lock_stack)
 {
+    LogPrintf("DOUBLE LOCK DETECTED\n");
+    LogPrintf("Lock order:\n");
+    for (const LockStackItem& i : lock_stack) {
+        if (i.first == mutex) {
+            LogPrintf(" (*)"); /* Continued */
+        }
+        LogPrintf(" %s\n", i.second.ToString());
+    }
+    if (g_debug_lockorder_abort) {
+        tfm::format(std::cerr, "Assertion failed: detected double lock at %s:%i, details in debug log.\n", __FILE__, __LINE__);
+        abort();
+    }
+    throw std::logic_error("double lock detected");
+}
+
+template <typename MutexType>
+static void push_lock(MutexType* c, const CLockLocation& locklocation)
+{
+    constexpr bool is_recursive_mutex =
+        std::is_base_of<RecursiveMutex, MutexType>::value ||
+        std::is_base_of<std::recursive_mutex, MutexType>::value;
+
     LockData& lockdata = GetLockData();
     std::lock_guard<std::mutex> lock(lockdata.dd_mutex);
 
     LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()];
     lock_stack.emplace_back(c, locklocation);
-    for (const LockStackItem& i : lock_stack) {
+    for (size_t j = 0; j < lock_stack.size() - 1; ++j) {
-        if (i.first == c)
+        const LockStackItem& i = lock_stack[j];
+        if (i.first == c) {
+            if (is_recursive_mutex) {
                 break;
+            }
+            // It is not a recursive mutex and it appears in the stack two times:
+            // at position `j` and at the end (which we added just before this loop).
+            // Can't allow locking the same (non-recursive) mutex two times from the
+            // same thread as that results in an undefined behavior.
+            auto lock_stack_copy = lock_stack;
+            lock_stack.pop_back();
+            double_lock_detected(c, lock_stack_copy);
+            // double_lock_detected() does not return.
+        }
 
         const LockPair p1 = std::make_pair(i.first, c);
         if (lockdata.lockorders.count(p1))
@@ -179,10 +214,15 @@ static void pop_lock()
     }
 }
 
-void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry)
+template <typename MutexType>
+void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexType* cs, bool fTry)
 {
     push_lock(cs, CLockLocation(pszName, pszFile, nLine, fTry, util::ThreadGetInternalName()));
 }
+template void EnterCritical(const char*, const char*, int, Mutex*, bool);
+template void EnterCritical(const char*, const char*, int, RecursiveMutex*, bool);
+template void EnterCritical(const char*, const char*, int, std::mutex*, bool);
+template void EnterCritical(const char*, const char*, int, std::recursive_mutex*, bool);
 
 void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line)
 {

src/sync.h

@@ -48,7 +48,8 @@ LEAVE_CRITICAL_SECTION(mutex); // no RAII
 ///////////////////////////////
 
 #ifdef DEBUG_LOCKORDER
-void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false);
+template <typename MutexType>
+void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexType* cs, bool fTry = false);
 void LeaveCritical();
 void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line);
 std::string LocksHeld();
@@ -65,7 +66,8 @@ bool LockStackEmpty();
  */
 extern bool g_debug_lockorder_abort;
 #else
-inline void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {}
+template <typename MutexType>
+inline void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexType* cs, bool fTry = false) {}
 inline void LeaveCritical() {}
 inline void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {}
 template <typename MutexType>
@@ -133,7 +135,7 @@ class SCOPED_LOCKABLE UniqueLock : public Base
 private:
     void Enter(const char* pszName, const char* pszFile, int nLine)
     {
-        EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()));
+        EnterCritical(pszName, pszFile, nLine, Base::mutex());
 #ifdef DEBUG_LOCKCONTENTION
         if (!Base::try_lock()) {
             PrintLockContention(pszName, pszFile, nLine);
@@ -146,7 +148,7 @@ private:
 
     bool TryEnter(const char* pszName, const char* pszFile, int nLine)
     {
-        EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()), true);
+        EnterCritical(pszName, pszFile, nLine, Base::mutex(), true);
         Base::try_lock();
         if (!Base::owns_lock())
             LeaveCritical();
@@ -203,7 +205,7 @@ public:
 
         ~reverse_lock() {
             templock.swap(lock);
-            EnterCritical(lockname.c_str(), file.c_str(), line, (void*)lock.mutex());
+            EnterCritical(lockname.c_str(), file.c_str(), line, lock.mutex());
             lock.lock();
         }
 
@@ -234,7 +236,7 @@ using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove
 
 #define ENTER_CRITICAL_SECTION(cs)                            \
     {                                                         \
-        EnterCritical(#cs, __FILE__, __LINE__, (void*)(&cs)); \
+        EnterCritical(#cs, __FILE__, __LINE__, &cs); \
         (cs).lock();                                          \
     }
 

src/test/fuzz/net.cpp

@@ -129,7 +129,7 @@ FUZZ_TARGET_INIT(net, initialize_net)
             [&] {
                 const std::vector<uint8_t> b = ConsumeRandomLengthByteVector(fuzzed_data_provider);
                 bool complete;
-                node.ReceiveMsgBytes((const char*)b.data(), b.size(), complete);
+                node.ReceiveMsgBytes(b, complete);
             });
     }
 

src/test/fuzz/p2p_transport_deserializer.cpp

@@ -22,15 +22,12 @@ FUZZ_TARGET_INIT(p2p_transport_deserializer, initialize_p2p_transport_deserializ
 {
     // Construct deserializer, with a dummy NodeId
     V1TransportDeserializer deserializer{Params(), (NodeId)0, SER_NETWORK, INIT_PROTO_VERSION};
-    const char* pch = (const char*)buffer.data();
+    Span<const uint8_t> msg_bytes{buffer};
-    size_t n_bytes = buffer.size();
+    while (msg_bytes.size() > 0) {
-    while (n_bytes > 0) {
+        const int handled = deserializer.Read(msg_bytes);
-        const int handled = deserializer.Read(pch, n_bytes);
         if (handled < 0) {
             break;
         }
-        pch += handled;
-        n_bytes -= handled;
         if (deserializer.Complete()) {
             const int64_t m_time = std::numeric_limits<int64_t>::max();
             uint32_t out_err_raw_size{0};

src/test/sync_tests.cpp

@@ -7,6 +7,8 @@
 
 #include <boost/test/unit_test.hpp>
 
+#include <mutex>
+
 namespace {
 template <typename MutexType>
 void TestPotentialDeadLockDetected(MutexType& mutex1, MutexType& mutex2)
@@ -42,6 +44,38 @@ void TestInconsistentLockOrderDetected(MutexType& mutex1, MutexType& mutex2) NO_
     LEAVE_CRITICAL_SECTION(mutex1);
     BOOST_CHECK(LockStackEmpty());
 }
+
+#ifdef DEBUG_LOCKORDER
+template <typename MutexType>
+void TestDoubleLock2(MutexType& m)
+{
+    ENTER_CRITICAL_SECTION(m);
+    LEAVE_CRITICAL_SECTION(m);
+}
+
+template <typename MutexType>
+void TestDoubleLock(bool should_throw)
+{
+    const bool prev = g_debug_lockorder_abort;
+    g_debug_lockorder_abort = false;
+
+    MutexType m;
+    ENTER_CRITICAL_SECTION(m);
+    if (should_throw) {
+        BOOST_CHECK_EXCEPTION(
+            TestDoubleLock2(m), std::logic_error, [](const std::logic_error& e) {
+                return strcmp(e.what(), "double lock detected") == 0;
+            });
+    } else {
+        BOOST_CHECK_NO_THROW(TestDoubleLock2(m));
+    }
+    LEAVE_CRITICAL_SECTION(m);
+
+    BOOST_CHECK(LockStackEmpty());
+
+    g_debug_lockorder_abort = prev;
+}
+#endif /* DEBUG_LOCKORDER */
 } // namespace
 
 BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup)
@@ -92,4 +126,19 @@ BOOST_AUTO_TEST_CASE(inconsistent_lock_order_detected)
 #endif // DEBUG_LOCKORDER
 }
 
+/* Double lock would produce an undefined behavior. Thus, we only do that if
+ * DEBUG_LOCKORDER is activated to detect it. We don't want non-DEBUG_LOCKORDER
+ * build to produce tests that exhibit known undefined behavior. */
+#ifdef DEBUG_LOCKORDER
+BOOST_AUTO_TEST_CASE(double_lock_mutex)
+{
+    TestDoubleLock<Mutex>(true /* should throw */);
+}
+
+BOOST_AUTO_TEST_CASE(double_lock_recursive_mutex)
+{
+    TestDoubleLock<RecursiveMutex>(false /* should not throw */);
+}
+#endif /* DEBUG_LOCKORDER */
+
 BOOST_AUTO_TEST_SUITE_END()

src/test/util/net.cpp

@@ -7,9 +7,9 @@
 #include <chainparams.h>
 #include <net.h>
 
-void ConnmanTestMsg::NodeReceiveMsgBytes(CNode& node, const char* pch, unsigned int nBytes, bool& complete) const
+void ConnmanTestMsg::NodeReceiveMsgBytes(CNode& node, Span<const uint8_t> msg_bytes, bool& complete) const
 {
-    assert(node.ReceiveMsgBytes(pch, nBytes, complete));
+    assert(node.ReceiveMsgBytes(msg_bytes, complete));
     if (complete) {
         size_t nSizeAdded = 0;
         auto it(node.vRecvMsg.begin());
@@ -29,11 +29,11 @@ void ConnmanTestMsg::NodeReceiveMsgBytes(CNode& node, const char* pch, unsigned
 
 bool ConnmanTestMsg::ReceiveMsgFrom(CNode& node, CSerializedNetMsg& ser_msg) const
 {
-    std::vector<unsigned char> ser_msg_header;
+    std::vector<uint8_t> ser_msg_header;
     node.m_serializer->prepareForTransport(ser_msg, ser_msg_header);
 
     bool complete;
-    NodeReceiveMsgBytes(node, (const char*)ser_msg_header.data(), ser_msg_header.size(), complete);
+    NodeReceiveMsgBytes(node, ser_msg_header, complete);
-    NodeReceiveMsgBytes(node, (const char*)ser_msg.data.data(), ser_msg.data.size(), complete);
+    NodeReceiveMsgBytes(node, ser_msg.data, complete);
     return complete;
 }

src/test/util/net.h

@@ -31,7 +31,7 @@ struct ConnmanTestMsg : public CConnman {
 
     void ProcessMessagesOnce(CNode& node) { m_msgproc->ProcessMessages(&node, flagInterruptMsgProc); }
 
-    void NodeReceiveMsgBytes(CNode& node, const char* pch, unsigned int nBytes, bool& complete) const;
+    void NodeReceiveMsgBytes(CNode& node, Span<const uint8_t> msg_bytes, bool& complete) const;
 
     bool ReceiveMsgFrom(CNode& node, CSerializedNetMsg& ser_msg) const;
 };

src/wallet/rpcdump.cpp

@@ -298,8 +298,9 @@ UniValue importprunedfunds(const JSONRPCRequest& request)
     CWallet* const pwallet = wallet.get();
 
     CMutableTransaction tx;
-    if (!DecodeHexTx(tx, request.params[0].get_str()))
+    if (!DecodeHexTx(tx, request.params[0].get_str())) {
-        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
+        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input.");
+    }
     uint256 hashTx = tx.GetHash();
     CWalletTx wtx(pwallet, MakeTransactionRef(std::move(tx)));
 

src/wallet/rpcwallet.cpp

@@ -2961,7 +2961,7 @@ static UniValue unloadwallet(const JSONRPCRequest& request)
         "Unloads the wallet referenced by the request endpoint otherwise unloads the wallet specified in the argument.\n"
         "Specifying the wallet name on a wallet endpoint is invalid.",
         {
-            {"wallet_name", RPCArg::Type::STR, /* default */ "the wallet name from the RPC endpoint", "The name of the wallet to unload. Must be provided in the RPC endpoint or this parameter (but not both)."},
+            {"wallet_name", RPCArg::Type::STR, /* default */ "the wallet name from the RPC endpoint", "The name of the wallet to unload. If provided both here and in the RPC endpoint, the two must be identical."},
             {"load_on_startup", RPCArg::Type::BOOL, /* default */ "null", "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."},
         },
         RPCResult{RPCResult::Type::OBJ, "", "", {
@@ -2975,8 +2975,8 @@ static UniValue unloadwallet(const JSONRPCRequest& request)
 
     std::string wallet_name;
     if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) {
-        if (!request.params[0].isNull()) {
+        if (!(request.params[0].isNull() || request.params[0].get_str() == wallet_name)) {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "Both the RPC endpoint wallet and wallet_name parameter were provided (only one allowed)");
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "RPC endpoint wallet and wallet_name parameter specify different wallets");
         }
     } else {
         wallet_name = request.params[0].get_str();
@@ -3464,7 +3464,7 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request)
 
     CMutableTransaction mtx;
     if (!DecodeHexTx(mtx, request.params[0].get_str())) {
-        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
+        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input.");
     }
 
     // Sign the transaction

test/functional/wallet_multiwallet.py

@@ -339,13 +339,18 @@ class MultiWalletTest(BitcoinTestFramework):
         assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].unloadwallet)
         assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", self.nodes[0].unloadwallet, "dummy")
         assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", node.get_wallet_rpc("dummy").unloadwallet)
-        assert_raises_rpc_error(-8, "Both the RPC endpoint wallet and wallet_name parameter were provided (only one allowed)", w1.unloadwallet, "w2"),
+        assert_raises_rpc_error(-8, "RPC endpoint wallet and wallet_name parameter specify different wallets", w1.unloadwallet, "w2"),
-        assert_raises_rpc_error(-8, "Both the RPC endpoint wallet and wallet_name parameter were provided (only one allowed)", w1.unloadwallet, "w1"),
 
         # Successfully unload the specified wallet name
         self.nodes[0].unloadwallet("w1")
         assert 'w1' not in self.nodes[0].listwallets()
 
+        # Unload w1 again, this time providing the wallet name twice
+        self.nodes[0].loadwallet("w1")
+        assert 'w1' in self.nodes[0].listwallets()
+        w1.unloadwallet("w1")
+        assert 'w1' not in self.nodes[0].listwallets()
+
         # Successfully unload the wallet referenced by the request endpoint
         # Also ensure unload works during walletpassphrase timeout
         w2.encryptwallet('test')

test/util/data/bitcoin-util-test.json

@@ -202,7 +202,7 @@
   { "exec": "./dash-tx",
     "args": ["-create", "outscript=0:123badscript"],
     "return_code": 1,
-    "error_txt": "error: script parse error",
+    "error_txt": "error: script parse error: unknown opcode",
     "description": "Create a new transaction with an invalid output script"
   },
   { "exec": "./dash-tx",