diff --git a/src/script/scriptcache.h b/src/script/scriptcache.h
--- a/src/script/scriptcache.h
+++ b/src/script/scriptcache.h
@@ -5,12 +5,31 @@
#ifndef BITCOIN_SCRIPT_SCRIPTCACHE_H
#define BITCOIN_SCRIPT_SCRIPTCACHE_H
-#include <uint256.h>
-
+#include <array>
#include <cstdint>
class CTransaction;
+/**
+ * The script cache is a map using a key/value element.
+ * The key is slightly shorter than a power-of-two size to make room for
+ * the value.
+ */
+class ScriptCacheKey {
+ std::array<uint8_t, 30> data;
+
+public:
+ ScriptCacheKey() = default;
+ ScriptCacheKey(const ScriptCacheKey &rhs) = default;
+ ScriptCacheKey(const CTransaction &tx, uint32_t flags);
+
+ bool operator==(const ScriptCacheKey &rhs) const {
+ return rhs.data == data;
+ }
+
+ friend class ScriptCacheHasher;
+};
+
// DoS prevention: limit cache size to 32MB (over 1000000 entries on 64-bit
// systems). Due to how we count cache size, actual memory usage is slightly
// more (~32.25 MB)
@@ -21,13 +40,18 @@
/** Initializes the script-execution cache */
void InitScriptExecutionCache();
-/** Compute the cache key for a given transaction and flags. */
-uint256 GetScriptCacheKey(const CTransaction &tx, uint32_t flags);
+/** Check if a given key is in the cache, and if so, return its values. */
+bool IsKeyInScriptCache(ScriptCacheKey key, bool erase, int &nSigChecksRet);
-/** Check if a given key is in the cache. */
-bool IsKeyInScriptCache(uint256 key, bool erase);
+constexpr int SCRIPT_CACHE_MAX_SIGCHECKS = UINT16_MAX;
-/** Add an entry in the cache. */
-void AddKeyInScriptCache(uint256 key);
+/**
+ * Add an entry in the cache. The sigchecks count must be in the range
+ * 0-SCRIPT_CACHE_MAX_SIGCHECKS inclusive, otherwise a cache entry must not
+ * be added (if successfully added, it would be corrupt data, and possibly
+ * lead to consensus failure when the cache is consulted during block
+ * validation).
+ */
+void AddKeyInScriptCache(ScriptCacheKey key, int nSigChecks);
#endif // BITCOIN_SCRIPT_SCRIPTCACHE_H
diff --git a/src/script/scriptcache.cpp b/src/script/scriptcache.cpp
--- a/src/script/scriptcache.cpp
+++ b/src/script/scriptcache.cpp
@@ -10,10 +10,71 @@
#include <random.h>
#include <script/sigcache.h>
#include <sync.h>
+#include <uint256.h>
#include <util/system.h>
#include <validation.h>
-static CuckooCache::cache<CuckooCache::KeyOnly<uint256>, SignatureCacheHasher>
+#include <limits>
+
+/**
+ * In future if many more values are added, it should be considered to
+ * expand the element size to 64 bytes (with padding the spare space as
+ * needed) so the key can be long. Shortening the key too much risks
+ * opening ourselves up to consensus-failing collisions, however it should
+ * be noted that our cache nonce is private and unique, so collisions would
+ * affect only one node and attackers have no way of offline-preparing a
+ * collision attack even on short keys.
+ */
+struct ScriptCacheElement {
+ using KeyType = ScriptCacheKey;
+
+ KeyType key;
+ uint16_t nSigChecksCount;
+
+ static_assert(SCRIPT_CACHE_MAX_SIGCHECKS ==
+ std::numeric_limits<decltype(nSigChecksCount)>::max(),
+ "the declared maximum must match the type");
+
+ ScriptCacheElement() = default;
+ ScriptCacheElement(const ScriptCacheElement &rhs) = default;
+
+ ScriptCacheElement(const KeyType &keyIn, uint16_t nSigChecksIn)
+ : key(keyIn), nSigChecksCount(nSigChecksIn) {}
+
+ const KeyType &getKey() const { return key; }
+};
+
+static_assert(sizeof(ScriptCacheElement) == 32,
+ "ScriptCacheElement must be 32 bytes");
+
+class ScriptCacheHasher {
+public:
+ template <uint8_t hash_select>
+ uint32_t operator()(const ScriptCacheKey &k) const {
+ static_assert(hash_select < 8, "only has 8 hashes available.");
+
+ const auto &d = k.data;
+
+ static_assert(sizeof(d) == 30,
+ "modify the following if key size changes");
+
+ uint32_t u;
+ if (hash_select < 7) {
+ std::memcpy(&u, d.data() + 4 * hash_select, 4);
+ } else {
+ // We are required to produce 8 subhashes. We have only two
+ // key bytes left over but this is fine. We put the leftovers
+ // on the higher bits, as these are what primarily get used to
+ // determine the index. The lower bits are filled by reusing
+ // some other bytes.
+ u = (uint32_t(d[28]) << 24) + (uint32_t(d[29]) << 16) +
+ (uint32_t(d[3]) << 8) + uint32_t(d[7]);
+ }
+ return u;
+ }
+};
+
+static CuckooCache::cache<ScriptCacheElement, ScriptCacheHasher>
scriptExecutionCache;
static uint256 scriptExecutionCacheNonce(GetRandHash());
@@ -32,8 +93,8 @@
(nElems * sizeof(uint256)) >> 20, nMaxCacheSize >> 20, nElems);
}
-uint256 GetScriptCacheKey(const CTransaction &tx, uint32_t flags) {
- uint256 key;
+ScriptCacheKey::ScriptCacheKey(const CTransaction &tx, uint32_t flags) {
+ uint256 hash;
// We only use the first 19 bytes of nonce to avoid a second SHA round -
// giving us 19 + 32 + 4 = 55 bytes (+ 8 + 1 = 64)
static_assert(55 - sizeof(flags) - 32 >= 128 / 8,
@@ -42,21 +103,32 @@
.Write(scriptExecutionCacheNonce.begin(), 55 - sizeof(flags) - 32)
.Write(tx.GetHash().begin(), 32)
.Write((uint8_t *)&flags, sizeof(flags))
- .Finalize(key.begin());
+ .Finalize(hash.begin());
- return key;
+ assert(data.size() < hash.size());
+ std::copy(hash.begin() + hash.size() - data.size(), hash.end(),
+ data.begin());
}
-bool IsKeyInScriptCache(uint256 key, bool erase) {
+bool IsKeyInScriptCache(ScriptCacheKey key, bool erase, int &nSigChecksRet) {
// TODO: Remove this requirement by making CuckooCache not require external
// locks
AssertLockHeld(cs_main);
- return scriptExecutionCache.contains(key, erase);
+ ScriptCacheElement elem(key, 0);
+ bool ret = scriptExecutionCache.get(elem, erase);
+ if (ret) {
+ nSigChecksRet = elem.nSigChecksCount;
+ }
+ return ret;
}
-void AddKeyInScriptCache(uint256 key) {
+void AddKeyInScriptCache(ScriptCacheKey key, int nSigChecks) {
+ if (0 > nSigChecks || nSigChecks > SCRIPT_CACHE_MAX_SIGCHECKS) {
+ throw std::range_error("sigchecks out of range");
+ }
// TODO: Remove this requirement by making CuckooCache not require external
// locks
AssertLockHeld(cs_main);
- scriptExecutionCache.insert(key);
+ ScriptCacheElement elem(key, nSigChecks);
+ scriptExecutionCache.insert(elem);
}
diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
--- a/src/test/txvalidationcache_tests.cpp
+++ b/src/test/txvalidationcache_tests.cpp
@@ -431,4 +431,71 @@
}
}
+BOOST_AUTO_TEST_CASE(scriptcache_values) {
+ // Test insertion and querying of keys&values from the script cache.
+
+ InitScriptExecutionCache();
+
+ // construct four distinct keys from very slightly different data
+ CMutableTransaction tx1;
+ tx1.nVersion = 1;
+ CMutableTransaction tx2;
+ tx2.nVersion = 2;
+ uint32_t flagsA = 0x7fffffff;
+ uint32_t flagsB = 0xffffffff;
+ ScriptCacheKey key1A(CTransaction(tx1), flagsA);
+ ScriptCacheKey key1B(CTransaction(tx1), flagsB);
+ ScriptCacheKey key2A(CTransaction(tx2), flagsA);
+ ScriptCacheKey key2B(CTransaction(tx2), flagsB);
+
+ BOOST_CHECK(key1A == key1A);
+ BOOST_CHECK(!(key1A == key1B));
+ BOOST_CHECK(!(key1A == key2A));
+ BOOST_CHECK(!(key1A == key2B));
+ BOOST_CHECK(key1B == key1B);
+ BOOST_CHECK(!(key1B == key2A));
+ BOOST_CHECK(!(key1B == key2B));
+ BOOST_CHECK(key2A == key2A);
+ BOOST_CHECK(!(key2A == key2B));
+ BOOST_CHECK(key2B == key2B);
+
+ int nSigChecks = 0xfeed;
+
+ // key is not yet inserted
+ BOOST_CHECK(!IsKeyInScriptCache(key1A, false, nSigChecks));
+ // add the key and check it worked
+ AddKeyInScriptCache(key1A, 42);
+ BOOST_CHECK(IsKeyInScriptCache(key1A, false, nSigChecks));
+ BOOST_CHECK(nSigChecks == 42);
+
+ BOOST_CHECK(!IsKeyInScriptCache(key1B, false, nSigChecks));
+ BOOST_CHECK(!IsKeyInScriptCache(key2A, false, nSigChecks));
+ BOOST_CHECK(!IsKeyInScriptCache(key2B, false, nSigChecks));
+
+ // add a couple more keys with value the limit of storeable range
+ AddKeyInScriptCache(key1B, 0);
+ AddKeyInScriptCache(key2A, 0xffff);
+
+ // try adding an unstoreable value
+ BOOST_CHECK_THROW(AddKeyInScriptCache(key2B, 0x10000), std::range_error);
+ BOOST_CHECK_THROW(AddKeyInScriptCache(key2B, -1), std::range_error);
+
+ // read out values again
+ nSigChecks = -1;
+ BOOST_CHECK(IsKeyInScriptCache(key1A, false, nSigChecks));
+ BOOST_CHECK(nSigChecks == 42);
+ BOOST_CHECK(IsKeyInScriptCache(key1B, false, nSigChecks));
+ BOOST_CHECK(nSigChecks == 0);
+ BOOST_CHECK(IsKeyInScriptCache(key2A, false, nSigChecks));
+ BOOST_CHECK(nSigChecks == 0xffff);
+ BOOST_CHECK(!IsKeyInScriptCache(key2B, false, nSigChecks));
+
+ // try overwriting an existing entry with different value (should never
+ // happen in practice but see what happens)
+ AddKeyInScriptCache(key1A, 99);
+ BOOST_CHECK(IsKeyInScriptCache(key1A, false, nSigChecks));
+ // currently, no replacement is done
+ BOOST_CHECK(nSigChecks == 42);
+}
+
BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/validation.cpp b/src/validation.cpp
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1232,8 +1232,10 @@
// Note that this assumes that the inputs provided are correct (ie that the
// transaction hash which is in tx's prevouts properly commits to the
// scriptPubKey in the inputs view of that transaction).
- uint256 hashCacheEntry = GetScriptCacheKey(tx, flags);
- if (IsKeyInScriptCache(hashCacheEntry, !scriptCacheStore)) {
+ int nSigChecksDummy;
+ ScriptCacheKey hashCacheEntry(tx, flags);
+ if (IsKeyInScriptCache(hashCacheEntry, !scriptCacheStore,
+ nSigChecksDummy)) {
return true;
}
@@ -1296,7 +1298,7 @@
if (scriptCacheStore && !pvChecks) {
// We executed all of the provided scripts, and were told to cache the
// result. Do so now.
- AddKeyInScriptCache(hashCacheEntry);
+ AddKeyInScriptCache(hashCacheEntry, 0);
}
return true;