diff --git a/src/script/interpreter.h b/src/script/interpreter.h --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -31,7 +31,7 @@ public: virtual bool VerifySignature(const std::vector &vchSig, const CPubKey &vchPubKey, - const uint256 &sighash, uint32_t flags) const; + const uint256 &sighash) const; virtual bool CheckSig(const std::vector &vchSigIn, const std::vector &vchPubKey, diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -937,8 +937,7 @@ .Write(vchMessage.data(), vchMessage.size()) .Finalize(vchHash.data()); fSuccess = checker.VerifySignature( - vchSig, CPubKey(vchPubKey), uint256(vchHash), - flags); + vchSig, CPubKey(vchPubKey), uint256(vchHash)); } if (!fSuccess && (flags & SCRIPT_VERIFY_NULLFAIL) && @@ -1460,8 +1459,7 @@ bool BaseSignatureChecker::VerifySignature(const std::vector &vchSig, const CPubKey &pubkey, - const uint256 &sighash, - uint32_t flags) const { + const uint256 &sighash) const { if (vchSig.size() == 64) { return pubkey.VerifySchnorr(sighash, vchSig); } else { @@ -1488,7 +1486,7 @@ uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, sigHashType, amount, this->txdata, flags); - if (!VerifySignature(vchSig, pubkey, sighash, flags)) { + if (!VerifySignature(vchSig, pubkey, sighash)) { return false; } diff --git a/src/script/sigcache.h b/src/script/sigcache.h --- a/src/script/sigcache.h +++ b/src/script/sigcache.h @@ -44,7 +44,7 @@ bool store; bool IsCached(const std::vector &vchSig, const CPubKey &vchPubKey, - const uint256 &sighash, uint32_t flags) const; + const uint256 &sighash) const; public: CachingTransactionSignatureChecker(const CTransaction *txToIn, @@ -55,8 +55,8 @@ store(storeIn) {} bool VerifySignature(const std::vector &vchSig, - const CPubKey &vchPubKey, const uint256 &sighash, - uint32_t flags) const override; + const CPubKey &vchPubKey, + const uint256 &sighash) const override; friend class TestCachingTransactionSignatureChecker; }; diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp --- a/src/script/sigcache.cpp +++ b/src/script/sigcache.cpp @@ -16,21 +16,6 @@ namespace { -/** - * Declare which flags absolutely do not affect VerifySignature() result. - * We this to reduce unnecessary cache misses (such as when policy and consensus - * flags differ on unrelated aspects). - */ -static const uint32_t INVARIANT_FLAGS = - SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC | SCRIPT_VERIFY_DERSIG | - SCRIPT_VERIFY_LOW_S | SCRIPT_VERIFY_NULLDUMMY | SCRIPT_VERIFY_SIGPUSHONLY | - SCRIPT_VERIFY_MINIMALDATA | SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS | - SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY | - SCRIPT_VERIFY_CHECKSEQUENCEVERIFY | SCRIPT_VERIFY_MINIMALIF | - SCRIPT_VERIFY_NULLFAIL | SCRIPT_VERIFY_COMPRESSED_PUBKEYTYPE | - SCRIPT_ENABLE_SIGHASH_FORKID | SCRIPT_ENABLE_REPLAY_PROTECTION | - SCRIPT_VERIFY_CHECKDATASIG_SIGOPS | SCRIPT_DISALLOW_SEGWIT_RECOVERY; - /** * Valid signature cache, to avoid doing expensive ECDSA signature checking * twice for every transaction (once when accepted into memory pool, and @@ -38,8 +23,7 @@ */ class CSignatureCache { private: - //! Entries are SHA256(nonce || flags || signature hash || public key || - //! signature): + //! Entries are SHA256(nonce || signature hash || public key || signature): uint256 nonce; typedef CuckooCache::cache map_type; map_type setValid; @@ -48,13 +32,11 @@ public: CSignatureCache() { GetRandBytes(nonce.begin(), 32); } - void ComputeEntry(uint256 &entry, const std::vector &vchSig, - const CPubKey &pubkey, const uint256 &hash, - uint32_t flags) { - flags &= ~INVARIANT_FLAGS; + void ComputeEntry(uint256 &entry, const uint256 &hash, + const std::vector &vchSig, + const CPubKey &pubkey) { CSHA256() .Write(nonce.begin(), 32) - .Write(reinterpret_cast(&flags), sizeof(flags)) .Write(hash.begin(), 32) .Write(&pubkey[0], pubkey.size()) .Write(&vchSig[0], vchSig.size()) @@ -101,10 +83,9 @@ template bool RunMemoizedCheck(const std::vector &vchSig, const CPubKey &pubkey, - const uint256 &sighash, uint32_t flags, bool storeOrErase, - const F &fun) { + const uint256 &sighash, bool storeOrErase, const F &fun) { uint256 entry; - signatureCache.ComputeEntry(entry, vchSig, pubkey, sighash, flags); + signatureCache.ComputeEntry(entry, sighash, vchSig, pubkey); if (signatureCache.Get(entry, !storeOrErase)) { return true; } @@ -119,16 +100,16 @@ bool CachingTransactionSignatureChecker::IsCached( const std::vector &vchSig, const CPubKey &pubkey, - const uint256 &sighash, uint32_t flags) const { - return RunMemoizedCheck(vchSig, pubkey, sighash, flags, true, + const uint256 &sighash) const { + return RunMemoizedCheck(vchSig, pubkey, sighash, true, [] { return false; }); } bool CachingTransactionSignatureChecker::VerifySignature( const std::vector &vchSig, const CPubKey &pubkey, - const uint256 &sighash, uint32_t flags) const { - return RunMemoizedCheck(vchSig, pubkey, sighash, flags, store, [&] { + const uint256 &sighash) const { + return RunMemoizedCheck(vchSig, pubkey, sighash, store, [&] { return TransactionSignatureChecker::VerifySignature(vchSig, pubkey, - sighash, flags); + sighash); }); } diff --git a/src/test/sigcache_tests.cpp b/src/test/sigcache_tests.cpp --- a/src/test/sigcache_tests.cpp +++ b/src/test/sigcache_tests.cpp @@ -25,27 +25,6 @@ static const std::string strSecret1C = "Kwr371tjA9u2rFSMZjTNun2PXXP3WPZu2afRHTcta6KxEUdm1vEw"; -/* We will be testing that these flags do not affect the cache entry. - * This list must match the one found in script/sigcache.cpp , however - * we duplicate it here to make sure that changes in cache behaviour also - * require an intentional change to this test. - */ -static const uint32_t TEST_INVARIANT_FLAGS = - SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC | SCRIPT_VERIFY_DERSIG | - SCRIPT_VERIFY_LOW_S | SCRIPT_VERIFY_NULLDUMMY | SCRIPT_VERIFY_SIGPUSHONLY | - SCRIPT_VERIFY_MINIMALDATA | SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS | - SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY | - SCRIPT_VERIFY_CHECKSEQUENCEVERIFY | SCRIPT_VERIFY_MINIMALIF | - SCRIPT_VERIFY_NULLFAIL | SCRIPT_VERIFY_COMPRESSED_PUBKEYTYPE | - SCRIPT_ENABLE_SIGHASH_FORKID | SCRIPT_ENABLE_REPLAY_PROTECTION | - SCRIPT_VERIFY_CHECKDATASIG_SIGOPS | SCRIPT_DISALLOW_SEGWIT_RECOVERY; -/* We will be testing that these flags DO affect the cache entry. The expected - * behaviour is that flags which are not explicitly listed as invariant in - * script/sigcache.cpp will affect the cache entry. Here we will thus enforce - * that certain flags are omitted from that sigcache.cpp list. - */ -static const uint32_t TEST_VARIANT_FLAGS = 0; - /** * Sigcache is only accessible via CachingTransactionSignatureChecker * as friend. @@ -60,15 +39,13 @@ } inline bool VerifyAndStore(const std::vector &vchSig, - const CPubKey &pubkey, const uint256 &sighash, - uint32_t flags) { - return pchecker->VerifySignature(vchSig, pubkey, sighash, flags); + const CPubKey &pubkey, const uint256 &sighash) { + return pchecker->VerifySignature(vchSig, pubkey, sighash); } inline bool IsCached(const std::vector &vchSig, - const CPubKey &pubkey, const uint256 &sighash, - uint32_t flags) { - return pchecker->IsCached(vchSig, pubkey, sighash, flags); + const CPubKey &pubkey, const uint256 &sighash) { + return pchecker->IsCached(vchSig, pubkey, sighash); } }; @@ -91,8 +68,6 @@ TestCachingTransactionSignatureChecker testChecker(checker); - uint32_t flags = 0; - CBitcoinSecret bsecret1, bsecret1C; BOOST_CHECK(bsecret1.SetString(strSecret1)); BOOST_CHECK(bsecret1C.SetString(strSecret1C)); @@ -116,95 +91,32 @@ BOOST_CHECK(key1.SignECDSA(hashMsg2, sig2)); // cross-check - BOOST_CHECK(!testChecker.VerifyAndStore(sig2, pubkey1, hashMsg, flags)); - BOOST_CHECK(!testChecker.VerifyAndStore(sig, pubkey1, hashMsg2, flags)); + BOOST_CHECK(!testChecker.VerifyAndStore(sig2, pubkey1, hashMsg)); + BOOST_CHECK(!testChecker.VerifyAndStore(sig, pubkey1, hashMsg2)); // that should not have put them in cache... - BOOST_CHECK(!testChecker.IsCached(sig2, pubkey1, hashMsg, flags)); - BOOST_CHECK(!testChecker.IsCached(sig, pubkey1, hashMsg2, flags)); + BOOST_CHECK(!testChecker.IsCached(sig2, pubkey1, hashMsg)); + BOOST_CHECK(!testChecker.IsCached(sig, pubkey1, hashMsg2)); // check that it's not in cache at start - BOOST_CHECK(!testChecker.IsCached(sig, pubkey1, hashMsg, flags)); - BOOST_CHECK(!testChecker.IsCached(sig2, pubkey1, hashMsg2, flags)); + BOOST_CHECK(!testChecker.IsCached(sig, pubkey1, hashMsg)); + BOOST_CHECK(!testChecker.IsCached(sig2, pubkey1, hashMsg2)); // Insert into cache - BOOST_CHECK(testChecker.VerifyAndStore(sig, pubkey1, hashMsg, flags)); - BOOST_CHECK(testChecker.VerifyAndStore(sig2, pubkey1, hashMsg2, flags)); + BOOST_CHECK(testChecker.VerifyAndStore(sig, pubkey1, hashMsg)); + BOOST_CHECK(testChecker.VerifyAndStore(sig2, pubkey1, hashMsg2)); // check that it's in - BOOST_CHECK(testChecker.IsCached(sig, pubkey1, hashMsg, flags)); - BOOST_CHECK(testChecker.IsCached(sig2, pubkey1, hashMsg2, flags)); + BOOST_CHECK(testChecker.IsCached(sig, pubkey1, hashMsg)); + BOOST_CHECK(testChecker.IsCached(sig2, pubkey1, hashMsg2)); // check that different signature hits different entry - BOOST_CHECK(!testChecker.IsCached(sig2, pubkey1, hashMsg, flags)); + BOOST_CHECK(!testChecker.IsCached(sig2, pubkey1, hashMsg)); // check that compressed pubkey hits different entry - BOOST_CHECK(!testChecker.IsCached(sig, pubkey1C, hashMsg, flags)); + BOOST_CHECK(!testChecker.IsCached(sig, pubkey1C, hashMsg)); // check that different message hits different entry - BOOST_CHECK(!testChecker.IsCached(sig, pubkey1, hashMsg2, flags)); + BOOST_CHECK(!testChecker.IsCached(sig, pubkey1, hashMsg2)); // compressed key is for same privkey, so verifying works: - BOOST_CHECK(testChecker.VerifyAndStore(sig, pubkey1C, hashMsg, flags)); + BOOST_CHECK(testChecker.VerifyAndStore(sig, pubkey1C, hashMsg)); // now we *should* get a hit - BOOST_CHECK(testChecker.IsCached(sig, pubkey1C, hashMsg, flags)); - } -} - -BOOST_AUTO_TEST_CASE(flag_invariants) { - /** - * Making CachingTransactionSignatureChecker requires a tx. So we make a - * dummy transaction (doesn't matter what it is) to construct it. - */ - CDataStream stream( - ParseHex( - "010000000122739e70fbee987a8be1788395a2f2e6ad18ccb7ff611cd798071539" - "dde3c38e000000000151ffffffff010000000000000000016a00000000"), - SER_NETWORK, PROTOCOL_VERSION); - CTransaction dummyTx(deserialize, stream); - PrecomputedTransactionData txdata(dummyTx); - CachingTransactionSignatureChecker checker(&dummyTx, 0, 0 * SATOSHI, true, - txdata); - - TestCachingTransactionSignatureChecker testChecker(checker); - - CBitcoinSecret bsecret1; - bsecret1.SetString(strSecret1); - CKey key1 = bsecret1.GetKey(); - CPubKey pubkey1 = key1.GetPubKey(); - - // there should not be any overlap - BOOST_REQUIRE((TEST_VARIANT_FLAGS & TEST_INVARIANT_FLAGS) == 0); - - for (int n = 0; n < 16; n++) { - std::string strMsg = strprintf("Sigcache testflags %i: xx", n); - uint256 hashMsg = Hash(strMsg.begin(), strMsg.end()); - - std::vector sig; - BOOST_CHECK(key1.SignECDSA(hashMsg, sig)); - - // choose random background flagset to test - uint32_t base_flags = insecure_rand(); - - // shouldn't be in cache at start - BOOST_CHECK(!testChecker.IsCached(sig, pubkey1, hashMsg, base_flags)); - // insert into cache - BOOST_CHECK( - testChecker.VerifyAndStore(sig, pubkey1, hashMsg, base_flags)); - // check that it's in - BOOST_CHECK(testChecker.IsCached(sig, pubkey1, hashMsg, base_flags)); - - // now we flip each of 32 flags one by one, checking cache - for (uint32_t flag = 1; flag; flag <<= 1) { - uint32_t alt_flags = base_flags ^ flag; - BOOST_CHECK(alt_flags != base_flags); - bool hit = testChecker.IsCached(sig, pubkey1, hashMsg, alt_flags); - if (TEST_VARIANT_FLAGS & flag) { - // if it's in TEST_VARIANT_FLAGS, we must miss cache - BOOST_CHECK_MESSAGE(!hit, n << " bad cache hit 0x" << std::hex - << base_flags << " ^ 0x" << flag); - } else if (TEST_INVARIANT_FLAGS & flag) { - // if it's in TEST_INVARIANT_FLAGS, we must hit cache - BOOST_CHECK_MESSAGE(hit, n << " bad cache miss 0x" << std::hex - << base_flags << " ^ 0x" << flag); - } else { - // if it's in neither, don't care. - } - } + BOOST_CHECK(testChecker.IsCached(sig, pubkey1C, hashMsg)); } }