Merge pull request #5143 from vijaydasmp/bp21_10

backport: Merge bitcoin#19237,19378,18422

src/net_processing.cpp

@@ -207,7 +207,7 @@ namespace {
      * We use this to avoid requesting transactions that have already been
      * confirnmed.
      */
-    RecursiveMutex g_cs_recent_confirmed_transactions;
+    Mutex g_cs_recent_confirmed_transactions;
     std::unique_ptr<CRollingBloomFilter> g_recent_confirmed_transactions GUARDED_BY(g_cs_recent_confirmed_transactions);
 
     /** Blocks that are in flight, and that are in the queue to be downloaded. */

src/pubkey.h

@@ -142,6 +142,9 @@ public:
         unsigned int len = ::ReadCompactSize(s);
         if (len <= SIZE) {
             s.read((char*)vch, len);
+            if (len != size()) {
+                Invalidate();
+            }
         } else {
             // invalid pubkey, skip available data
             char dummy;

src/script/interpreter.cpp

@@ -274,6 +274,35 @@ int FindAndDelete(CScript& script, const CScript& b)
     return nFound;
 }
 
+/** Helper for OP_CHECKSIG and OP_CHECKSIGVERIFY
+ *
+ * A return value of false means the script fails entirely. When true is returned, the
+ * fSuccess variable indicates whether the signature check itself succeeded.
+ */
+static bool EvalChecksig(const valtype& vchSig, const valtype& vchPubKey, CScript::const_iterator pbegincodehash, CScript::const_iterator pend, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* serror, bool& fSuccess)
+{
+    // Subset of script starting at the most recent codeseparator
+    CScript scriptCode(pbegincodehash, pend);
+
+    // Drop the signature, since there's no way for a signature to sign itself
+    if (sigversion == SigVersion::BASE) {
+        int found = FindAndDelete(scriptCode, CScript() << vchSig);
+        if (found > 0 && (flags & SCRIPT_VERIFY_CONST_SCRIPTCODE))
+            return set_error(serror, SCRIPT_ERR_SIG_FINDANDDELETE);
+    }
+
+    if (!CheckSignatureEncoding(vchSig, flags, serror) || !CheckPubKeyEncoding(vchPubKey, flags, sigversion, serror)) {
+        //serror is set
+        return false;
+    }
+    fSuccess = checker.CheckSig(vchSig, vchPubKey, scriptCode, sigversion);
+
+    if (!fSuccess && (flags & SCRIPT_VERIFY_NULLFAIL) && vchSig.size())
+        return set_error(serror, SCRIPT_ERR_SIG_NULLFAIL);
+
+    return true;
+}
+
 bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* serror)
 {
     static const CScriptNum bnZero(0);
@@ -978,25 +1007,8 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
                     valtype& vchSig    = stacktop(-2);
                     valtype& vchPubKey = stacktop(-1);
 
-                    // Subset of script starting at the most recent codeseparator
+                    bool fSuccess = true;
-                    CScript scriptCode(pbegincodehash, pend);
+                    if (!EvalChecksig(vchSig, vchPubKey, pbegincodehash, pend, flags, checker, sigversion, serror, fSuccess)) return false;
-
-                    // Drop the signature, since there's no way for a signature to sign itself
-                    if (sigversion == SigVersion::BASE) {
-                        int found = FindAndDelete(scriptCode, CScript() << vchSig);
-                        if (found > 0 && (flags & SCRIPT_VERIFY_CONST_SCRIPTCODE))
-                            return set_error(serror, SCRIPT_ERR_SIG_FINDANDDELETE);
-                    }
-
-                    if (!CheckSignatureEncoding(vchSig, flags, serror) || !CheckPubKeyEncoding(vchPubKey, flags, sigversion, serror)) {
-                        //serror is set
-                        return false;
-                    }
-                    bool fSuccess = checker.CheckSig(vchSig, vchPubKey, scriptCode, sigversion);
-
-                    if (!fSuccess && (flags & SCRIPT_VERIFY_NULLFAIL) && vchSig.size())
-                        return set_error(serror, SCRIPT_ERR_SIG_NULLFAIL);
-
                     popstack(stack);
                     popstack(stack);
                     stack.push_back(fSuccess ? vchTrue : vchFalse);

src/test/key_tests.cpp

@@ -5,6 +5,7 @@
 #include <key.h>
 
 #include <key_io.h>
+#include <streams.h>
 #include <uint256.h>
 #include <util/system.h>
 #include <util/strencodings.h>
@@ -220,4 +221,47 @@ BOOST_AUTO_TEST_CASE(key_key_negation)
     BOOST_CHECK(key.GetPubKey().data()[0] == 0x03);
 }
 
+static CPubKey UnserializePubkey(const std::vector<uint8_t>& data)
+{
+    CDataStream stream{SER_NETWORK, INIT_PROTO_VERSION};
+    stream << data;
+    CPubKey pubkey;
+    stream >> pubkey;
+    return pubkey;
+}
+
+static unsigned int GetLen(unsigned char chHeader)
+{
+    if (chHeader == 2 || chHeader == 3)
+        return CPubKey::COMPRESSED_SIZE;
+    if (chHeader == 4 || chHeader == 6 || chHeader == 7)
+        return CPubKey::SIZE;
+    return 0;
+}
+
+static void CmpSerializationPubkey(const CPubKey& pubkey)
+{
+    CDataStream stream{SER_NETWORK, INIT_PROTO_VERSION};
+    stream << pubkey;
+    CPubKey pubkey2;
+    stream >> pubkey2;
+    BOOST_CHECK(pubkey == pubkey2);
+}
+
+BOOST_AUTO_TEST_CASE(pubkey_unserialize)
+{
+    for (uint8_t i = 2; i <= 7; ++i) {
+        CPubKey key = UnserializePubkey({0x02});
+        BOOST_CHECK(!key.IsValid());
+        CmpSerializationPubkey(key);
+        key = UnserializePubkey(std::vector<uint8_t>(GetLen(i), i));
+        CmpSerializationPubkey(key);
+        if (i == 5) {
+            BOOST_CHECK(!key.IsValid());
+        } else {
+            BOOST_CHECK(key.IsValid());
+        }
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END()