diff --git a/src/bucket/BucketManager.cpp b/src/bucket/BucketManager.cpp index 13693100e7..1fcbc268dd 100644 --- a/src/bucket/BucketManager.cpp +++ b/src/bucket/BucketManager.cpp @@ -23,6 +23,7 @@ #include "ledger/NetworkConfig.h" #include "main/Application.h" #include "main/Config.h" +#include "test/CovMark.h" #include "util/Fs.h" #include "util/GlobalChecks.h" #include "util/LogSlowExecution.h" @@ -1205,6 +1206,7 @@ BucketManager::resolveBackgroundEvictionScan( } else { + COVMARK_HIT(EVICTION_TTL_MODIFIED_BETWEEN_DECISION_AND_EVICTION); iter = eligibleEntries.erase(iter); } } @@ -1246,10 +1248,13 @@ BucketManager::resolveBackgroundEvictionScan( entryToEvictIter = eligibleEntries.erase(entryToEvictIter); } - // If remainingEntriesToEvict == 0, that means we could not evict the entire - // scan region, so the new eviction iterator should be after the last entry - // evicted. Otherwise, eviction iterator should be at the end of the scan - // region + // If remainingEntriesToEvict != 0, that means we evicted the entire set of + // eligible entries without hitting our numeric limit, and should advance + // newEvictionIterator to the end of the scan region. + // + // If remainingEntriesToEvict == 0, we _did_ hit our limit, but there's + // nothing to do in that case: we already updated newEvictionIterator to + // after the the last scanned eligible entry in the body of the loop. if (remainingEntriesToEvict != 0) { newEvictionIterator = evictionCandidates->endOfRegionIterator; diff --git a/src/test/CovMark.cpp b/src/test/CovMark.cpp new file mode 100644 index 0000000000..b4d87bae8a --- /dev/null +++ b/src/test/CovMark.cpp @@ -0,0 +1,85 @@ +// Copyright 2025 Stellar Development Foundation and contributors. Licensed +// under the Apache License, Version 2.0. See the COPYING file at the root +// of this distribution or at http://www.apache.org/licenses/LICENSE-2.0 + +#ifdef BUILD_TESTS + +#include "test/CovMark.h" +#include "crypto/ShortHash.h" + +#include +#include + +namespace stellar +{ + +CovMarks gCovMarks; + +CovMarks::CovMarks() +{ + reset(); +} + +void +CovMarks::hit(CovMark mark) +{ + mCovMarks[static_cast(mark)].fetch_add( + 1, std::memory_order_relaxed); +} + +std::uint64_t +CovMarks::get(CovMark mark) const +{ + return mCovMarks[static_cast(mark)].load( + std::memory_order_relaxed); +} + +void +CovMarks::reset() +{ + for (auto& mark : mCovMarks) + { + mark.store(0, std::memory_order_relaxed); + } +} + +std::uint64_t +CovMarks::hash() const +{ + std::array + markBytes; + for (size_t i = 0; i < CovMark::COVMARK_COUNT; ++i) + { + auto mark = mCovMarks[i].load(std::memory_order_relaxed); + for (size_t j = 0; j < sizeof(std::uint64_t); ++j) + { + markBytes[i * sizeof(std::uint64_t) + j] = (mark >> (j * 8)) & 0xFF; + } + } + return shortHash::computeHash( + ByteSlice(markBytes.data(), markBytes.size())); +} + +CovMarkGuard::CovMarkGuard(CovMark mark, char const* file, int line, + char const* name) + : mMark(mark) + , mValueOnEntry(gCovMarks.get(mark)) + , mFile(file) + , mLine(line) + , mName(name) +{ +} + +CovMarkGuard::~CovMarkGuard() noexcept(false) +{ + auto valueOnExit = gCovMarks.get(mMark); + // We only throw if we are not already unwinding due to another exception. + if (!(valueOnExit > mValueOnEntry) && std::uncaught_exceptions() == 0) + { + throw std::runtime_error(fmt::format( + "expected mark '{}' not hit in scope {}:{}", mFile, mLine, mName)); + } +} +} + +#endif // BUILD_TESTS \ No newline at end of file diff --git a/src/test/CovMark.h b/src/test/CovMark.h new file mode 100644 index 0000000000..5f64ee0181 --- /dev/null +++ b/src/test/CovMark.h @@ -0,0 +1,80 @@ +#pragma once + +// Copyright 2025 Stellar Development Foundation and contributors. Licensed +// under the Apache License, Version 2.0. See the COPYING file at the root +// of this distribution or at http://www.apache.org/licenses/LICENSE-2.0 + +#include +#include +#include + +// This is a small utility class used in tests to mark coverage points +// that are otherwise hard to verify via external observation. +// +// It is similar to the mechanism described here: +// https://ferrous-systems.com/blog/coverage-marks/ +// +// But it has a few differences: +// +// 1. It is only enabled in test builds +// +// 2. It keeps all atomics in a single global array rather than scattered +// among a bunch of different variables. +// +// 3. This allows us to do reset all counters to zero at the beginning of +// each test run, and also do complex checks if we want to, e.g. verify +// that certain marks were hit in a certain order, or a certain number of +// times, or different marks sum up to some value, etc. +// +// 4. It also allows us to hash all the marks together to get a trajectory +// summmary for a run, which we can feed to a fuzzer to explore different +// code paths. + +#ifdef BUILD_TESTS + +namespace stellar +{ + +enum CovMark : std::size_t +{ + EVICTION_TTL_MODIFIED_BETWEEN_DECISION_AND_EVICTION = 0, + COVMARK_COUNT // This must be the last entry +}; + +class CovMarks +{ + std::array, CovMark::COVMARK_COUNT> mCovMarks; + + public: + CovMarks(); + void hit(CovMark mark); + std::uint64_t get(CovMark mark) const; + void reset(); + std::uint64_t hash() const; +}; + +extern CovMarks gCovMarks; + +class CovMarkGuard final +{ + CovMark mMark; + std::uint64_t mValueOnEntry{0}; + char const* mFile; + int mLine; + char const* mName; + + public: + CovMarkGuard(CovMark mark, char const* file, int line, char const* name); + ~CovMarkGuard() noexcept(false); +}; +} + +#define COVMARK_HIT(covmark) ::stellar::gCovMarks.hit(CovMark::covmark); +#define COVMARK_CHECK_HIT_IN_CURR_SCOPE(covmark) \ + ::stellar::CovMarkGuard covmark_guard_##covmark##_( \ + CovMark::covmark, __FILE__, __LINE__, #covmark); + +#else // !BUILD_TESTS +#define COVMARK_HIT(covmark) // no-op +#define COVMARK_CHECK_HIT_IN_CURR_SCOPE(covmark) // no-op +#endif // BUILD_TESTS diff --git a/src/transactions/test/SorobanEvictionTests.cpp b/src/transactions/test/SorobanEvictionTests.cpp new file mode 100644 index 0000000000..4d9f52b26a --- /dev/null +++ b/src/transactions/test/SorobanEvictionTests.cpp @@ -0,0 +1,69 @@ +// Copyright 2025 Stellar Development Foundation and contributors. Licensed +// under the Apache License, Version 2.0. See the COPYING file at the root +// of this distribution or at http://www.apache.org/licenses/LICENSE-2.0 + +#include "test/CovMark.h" +#include "test/test.h" +#include "test/Catch2.h" +#include "test/TxTests.h" +#include "transactions/test/SorobanTxTestUtils.h" +#include "util/Logging.h" + +using namespace stellar; +using namespace stellar::txtest; + +TEST_CASE("defer eviction due to concurrent TTL modify", "[tx][soroban][archival]") +{ + auto cfg = getTestConfig(); + SorobanTest test(cfg, true, [](SorobanNetworkConfig& cfg) { + cfg.mStateArchivalSettings.minPersistentTTL = + MinimumSorobanNetworkConfig::MINIMUM_PERSISTENT_ENTRY_LIFETIME; + cfg.mStateArchivalSettings.startingEvictionScanLevel = 1; + }); + ContractStorageTestClient client(test); + + auto expirationLedger = + test.getLCLSeq() + + MinimumSorobanNetworkConfig::MINIMUM_PERSISTENT_ENTRY_LIFETIME; + + client.put("key", ContractDataDurability::PERSISTENT, 123); + + CLOG_INFO(Ledger, "Expiration ledger is {}", expirationLedger); + + while (test.getLCLSeq() < expirationLedger - 1) + { + CLOG_INFO(Ledger, "Closing ledger as LCL {} < Expiration ledger - 1 = {}", test.getLCLSeq(), expirationLedger - 1); + closeLedger(test.getApp()); + } + + auto checklive = [&](ExpirationStatus expected) { + auto status = + client.getEntryExpirationStatus("key", ContractDataDurability::PERSISTENT); + CLOG_INFO(Ledger, "Checking status at ledger {}, expected {}, found {}", test.getLCLSeq(), (int)expected, + (int)status); + REQUIRE(status == expected); + }; + + checklive(ExpirationStatus::LIVE); + closeLedger(test.getApp()); + checklive(ExpirationStatus::EXPIRED_IN_LIVE_STATE); + + COVMARK_CHECK_HIT_IN_CURR_SCOPE(EVICTION_TTL_MODIFIED_BETWEEN_DECISION_AND_EVICTION); + + // At present _this_ will trigger the restored-while-evicting logic; I guess + // because eviction didn't quite get to the key in the previous ledger, even + // though the entry _expired_ it wasn't evicted yet? Can we provoke that? + client.restore("key", ContractDataDurability::PERSISTENT); + checklive(ExpirationStatus::LIVE); + + while (test.getLCLSeq() < expirationLedger - 1) + { + CLOG_INFO(Ledger, "Closing ledger as LCL {} < Expiration ledger - 1 = {}", test.getLCLSeq(), expirationLedger - 1); + closeLedger(test.getApp()); + } + checklive(ExpirationStatus::LIVE); + // Now do an extend _on_ the expiration ledger to ensure that eviction is deferred + client.extend("key", ContractDataDurability::PERSISTENT, + MinimumSorobanNetworkConfig::MINIMUM_PERSISTENT_ENTRY_LIFETIME, + expirationLedger + 10); +} \ No newline at end of file diff --git a/src/transactions/test/SorobanTxTestUtils.cpp b/src/transactions/test/SorobanTxTestUtils.cpp index a80a0f9e53..dc94b1b3b4 100644 --- a/src/transactions/test/SorobanTxTestUtils.cpp +++ b/src/transactions/test/SorobanTxTestUtils.cpp @@ -12,6 +12,7 @@ #include "transactions/InvokeHostFunctionOpFrame.h" #include "transactions/TransactionUtils.h" #include "util/XDRCereal.h" +#include "xdr/Stellar-transaction.h" #include "xdrpp/printer.h" namespace stellar @@ -2168,6 +2169,23 @@ ContractStorageTestClient::extend(std::string const& key, return *invocation.getResultCode(); } +InvokeHostFunctionResultCode +ContractStorageTestClient::restore(std::string const& key, + ContractDataDurability durability, + std::optional spec) +{ + if (!spec) + { + spec = writeKeySpec(key, durability); + } + + auto tx = mContract.getTest().createRestoreTx( + spec->getResources(), spec->getInclusionFee(), spec->getResourceFee()); + + auto result = mContract.getTest().invokeTx(tx); + return result.result.results()[0].tr().invokeHostFunctionResult().code(); +} + TestContract::Invocation ContractStorageTestClient::resizeStorageAndExtendInvocation( std::string const& key, uint32_t numKiloBytes, uint32_t thresh, diff --git a/src/transactions/test/SorobanTxTestUtils.h b/src/transactions/test/SorobanTxTestUtils.h index 6a92fb8773..1647a5327f 100644 --- a/src/transactions/test/SorobanTxTestUtils.h +++ b/src/transactions/test/SorobanTxTestUtils.h @@ -10,6 +10,7 @@ #include "test/test.h" #include "transactions/TransactionFrameBase.h" #include "transactions/TransactionUtils.h" +#include "xdr/Stellar-transaction.h" namespace stellar { @@ -472,6 +473,10 @@ class ContractStorageTestClient uint32_t threshold, uint32_t extendTo, std::optional spec = std::nullopt); + InvokeHostFunctionResultCode + restore(std::string const& key, ContractDataDurability durability, + std::optional spec = std::nullopt); + TestContract::Invocation resizeStorageAndExtendInvocation( std::string const& key, uint32_t numKiloBytes, uint32_t thresh, uint32_t extendTo,