diff --git a/src/crypto/SecretKey.cpp b/src/crypto/SecretKey.cpp index 1c92d1c090..6e4851152e 100644 --- a/src/crypto/SecretKey.cpp +++ b/src/crypto/SecretKey.cpp @@ -18,6 +18,8 @@ #include "util/Math.h" #include "util/RandomEvictionCache.h" #include +#include +#include #include #include #include @@ -41,16 +43,29 @@ namespace stellar // to the state of the process; caching its results centrally // makes all signature-verification in the program faster and // has no effect on correctness. +// +// The cache is sharded across NUM_VERIFY_CACHE_SHARDS shards to +// reduce mutex contention when multiple threads verify signatures +// in parallel. Each shard has its own mutex and cache partition. constexpr size_t VERIFY_SIG_CACHE_SIZE = 250'000; -static std::mutex gVerifySigCacheMutex; -static RandomEvictionCache gVerifySigCache(VERIFY_SIG_CACHE_SIZE); -static uint64_t gVerifyCacheHit = 0; -static uint64_t gVerifyCacheMiss = 0; +constexpr size_t NUM_VERIFY_CACHE_SHARDS = 16; +constexpr size_t VERIFY_SIG_CACHE_SHARD_SIZE = + VERIFY_SIG_CACHE_SIZE / NUM_VERIFY_CACHE_SHARDS; + +struct VerifySigCacheShard +{ + std::mutex mMutex; + RandomEvictionCache mCache; + VerifySigCacheShard() : mCache(VERIFY_SIG_CACHE_SHARD_SIZE) + { + } +}; -// Global flag to use Rust ed25519-dalek for signature verification -// Protected by gVerifySigCacheMutex -static bool gUseRustDalekVerify = false; +static std::array + gVerifySigCacheShards; +static std::atomic gVerifyCacheHit{0}; +static std::atomic gVerifyCacheMiss{0}; static Hash verifySigCacheKey(PublicKey const& key, Signature const& signature, @@ -322,32 +337,29 @@ SecretKey::fromStrKeySeed(std::string const& strKeySeed) void PubKeyUtils::clearVerifySigCache() { - std::lock_guard guard(gVerifySigCacheMutex); - gVerifySigCache.clear(); -} - -void -PubKeyUtils::enableRustDalekVerify() -{ - std::lock_guard guard(gVerifySigCacheMutex); - gUseRustDalekVerify = true; + for (auto& shard : gVerifySigCacheShards) + { + std::lock_guard guard(shard.mMutex); + shard.mCache.clear(); + } } void PubKeyUtils::seedVerifySigCache(unsigned int seed) { - std::lock_guard guard(gVerifySigCacheMutex); - gVerifySigCache.seed(seed); + for (size_t i = 0; i < NUM_VERIFY_CACHE_SHARDS; ++i) + { + std::lock_guard guard(gVerifySigCacheShards[i].mMutex); + gVerifySigCacheShards[i].mCache.seed(seed + + static_cast(i)); + } } void PubKeyUtils::flushVerifySigCacheCounts(uint64_t& hits, uint64_t& misses) { - std::lock_guard guard(gVerifySigCacheMutex); - hits = gVerifyCacheHit; - misses = gVerifyCacheMiss; - gVerifyCacheHit = 0; - gVerifyCacheMiss = 0; + hits = gVerifyCacheHit.exchange(0, std::memory_order_relaxed); + misses = gVerifyCacheMiss.exchange(0, std::memory_order_relaxed); } std::string @@ -456,41 +468,30 @@ PubKeyUtils::verifySig(PublicKey const& key, Signature const& signature, } auto cacheKey = verifySigCacheKey(key, signature, bin); - bool shouldUseRustDalekVerify; + + // Select shard based on cache key hash to distribute lock contention + auto shardIdx = std::hash{}(cacheKey) % NUM_VERIFY_CACHE_SHARDS; + auto& shard = gVerifySigCacheShards[shardIdx]; { - std::lock_guard guard(gVerifySigCacheMutex); - if (gVerifySigCache.exists(cacheKey)) + std::lock_guard guard(shard.mMutex); + if (auto* cached = shard.mCache.maybeGet(cacheKey)) { - ++gVerifyCacheHit; - std::string hitStr("hit"); - ZoneText(hitStr.c_str(), hitStr.size()); - return {gVerifySigCache.get(cacheKey), - VerifySigCacheLookupResult::HIT}; + gVerifyCacheHit.fetch_add(1, std::memory_order_relaxed); + ZoneText("hit", 3); + return {*cached, VerifySigCacheLookupResult::HIT}; } - - shouldUseRustDalekVerify = gUseRustDalekVerify; } - std::string missStr("miss"); - ZoneText(missStr.c_str(), missStr.size()); + ZoneText("miss", 4); - bool ok; - if (shouldUseRustDalekVerify) - { - ok = stellar::rust_bridge::verify_ed25519_signature_dalek( - key.ed25519().data(), signature.data(), bin.data(), bin.size()); - } - else + bool ok = stellar::rust_bridge::verify_ed25519_signature_dalek( + key.ed25519().data(), signature.data(), bin.data(), bin.size()); { - ok = (crypto_sign_verify_detached(signature.data(), bin.data(), - bin.size(), - key.ed25519().data()) == 0); + std::lock_guard guard(shard.mMutex); + gVerifyCacheMiss.fetch_add(1, std::memory_order_relaxed); + shard.mCache.put(cacheKey, ok); } - - std::lock_guard guard(gVerifySigCacheMutex); - ++gVerifyCacheMiss; - gVerifySigCache.put(cacheKey, ok); return {ok, VerifySigCacheLookupResult::MISS}; } diff --git a/src/crypto/SecretKey.h b/src/crypto/SecretKey.h index 9c3c96aeba..c30286e4d0 100644 --- a/src/crypto/SecretKey.h +++ b/src/crypto/SecretKey.h @@ -166,13 +166,6 @@ void clearVerifySigCache(); void seedVerifySigCache(unsigned int seed); void flushVerifySigCacheCounts(uint64_t& hits, uint64_t& misses); -// Enable Rust ed25519-dalek for signature verification -// Once enabled, it cannot be disabled. It should be enabled at the protocol 24 -// boundary. -// Note: This should be removed following the protocol 24 upgrade, rust ed25519 -// can be used unconditionally after upgrade, even for replay. -void enableRustDalekVerify(); - PublicKey random(); #ifdef BUILD_TESTS PublicKey pseudoRandomForTesting(); diff --git a/src/crypto/test/CryptoTests.cpp b/src/crypto/test/CryptoTests.cpp index 100a37163f..858c10a406 100644 --- a/src/crypto/test/CryptoTests.cpp +++ b/src/crypto/test/CryptoTests.cpp @@ -1630,7 +1630,6 @@ ZcashTestVector const ZCASH_TEST_VECTORS[196] = { TEST_CASE("Ed25519 test vectors from Zcash", "[crypto]") { - PubKeyUtils::enableRustDalekVerify(); for (auto const& tv : ZCASH_TEST_VECTORS) { PublicKey pk; diff --git a/src/herder/Upgrades.cpp b/src/herder/Upgrades.cpp index e8b1423f64..a2b232979a 100644 --- a/src/herder/Upgrades.cpp +++ b/src/herder/Upgrades.cpp @@ -1249,7 +1249,6 @@ Upgrades::applyVersionUpgrade(Application& app, AbstractLedgerTxn& ltx, } if (needUpgradeToVersion(ProtocolVersion::V_25, prevVersion, newVersion)) { - PubKeyUtils::enableRustDalekVerify(); SorobanNetworkConfig::createCostTypesForV25(ltx, app); } if (needUpgradeToVersion(ProtocolVersion::V_26, prevVersion, newVersion)) diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp index aa45c7f16d..46c9536851 100644 --- a/src/ledger/LedgerManagerImpl.cpp +++ b/src/ledger/LedgerManagerImpl.cpp @@ -1928,10 +1928,6 @@ LedgerManagerImpl::setLastClosedLedger( advanceLastClosedLedgerState(output); auto ledgerVersion = lastClosed.header.ledgerVersion; - if (protocolVersionStartsFrom(ledgerVersion, ProtocolVersion::V_25)) - { - PubKeyUtils::enableRustDalekVerify(); - } if (rebuildInMemoryState) { diff --git a/src/main/ApplicationImpl.cpp b/src/main/ApplicationImpl.cpp index 810522d4df..c0c9ab48c1 100644 --- a/src/main/ApplicationImpl.cpp +++ b/src/main/ApplicationImpl.cpp @@ -801,14 +801,6 @@ ApplicationImpl::start() // LCL is now loaded; unblock HTTP endpoints that were gated during boot. mCommandHandler->setReady(); - // Check if we're already on protocol V_24 or later and enable Rust Dalek - auto const& lcl = mLedgerManager->getLastClosedLedgerHeader(); - if (protocolVersionStartsFrom(lcl.header.ledgerVersion, - ProtocolVersion::V_25)) - { - PubKeyUtils::enableRustDalekVerify(); - } - startServices(); }