Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions src/bucket/BucketManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -1205,6 +1206,7 @@ BucketManager::resolveBackgroundEvictionScan(
}
else
{
COVMARK_HIT(EVICTION_TTL_MODIFIED_BETWEEN_DECISION_AND_EVICTION);
iter = eligibleEntries.erase(iter);
}
}
Expand Down Expand Up @@ -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.
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove duplicate word 'the' - should be 'after the last scanned eligible entry'.

Suggested change
// after the the last scanned eligible entry in the body of the loop.
// after the last scanned eligible entry in the body of the loop.

Copilot uses AI. Check for mistakes.
if (remainingEntriesToEvict != 0)
{
newEvictionIterator = evictionCandidates->endOfRegionIterator;
Expand Down
85 changes: 85 additions & 0 deletions src/test/CovMark.cpp
Original file line number Diff line number Diff line change
@@ -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 <fmt/format.h>
#include <stdexcept>

namespace stellar
{

CovMarks gCovMarks;

CovMarks::CovMarks()
{
reset();
}

void
CovMarks::hit(CovMark mark)
{
mCovMarks[static_cast<std::uint64_t>(mark)].fetch_add(
1, std::memory_order_relaxed);
}

std::uint64_t
CovMarks::get(CovMark mark) const
{
return mCovMarks[static_cast<std::uint64_t>(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<std::uint8_t, sizeof(std::uint64_t) * CovMark::COVMARK_COUNT>
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
80 changes: 80 additions & 0 deletions src/test/CovMark.h
Original file line number Diff line number Diff line change
@@ -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 <array>
#include <atomic>
#include <cstdint>

// 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
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'summmary' to 'summary'.

Suggested change
// summmary for a run, which we can feed to a fuzzer to explore different
// summary for a run, which we can feed to a fuzzer to explore different

Copilot uses AI. Check for mistakes.
// 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<std::atomic<std::uint64_t>, 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
69 changes: 69 additions & 0 deletions src/transactions/test/SorobanEvictionTests.cpp
Original file line number Diff line number Diff line change
@@ -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());
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove trailing whitespace at end of line.

Suggested change
closeLedger(test.getApp());
closeLedger(test.getApp());

Copilot uses AI. Check for mistakes.
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);
}
18 changes: 18 additions & 0 deletions src/transactions/test/SorobanTxTestUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -2168,6 +2169,23 @@ ContractStorageTestClient::extend(std::string const& key,
return *invocation.getResultCode();
}

InvokeHostFunctionResultCode
ContractStorageTestClient::restore(std::string const& key,
ContractDataDurability durability,
std::optional<SorobanInvocationSpec> 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();
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect result accessor used. Restore operations should use restoreFootprintResult().code() instead of invokeHostFunctionResult().code(). This will cause a runtime error when accessing the wrong union member.

Suggested change
return result.result.results()[0].tr().invokeHostFunctionResult().code();
return result.result.results()[0].tr().restoreFootprintResult().code();

Copilot uses AI. Check for mistakes.
}

TestContract::Invocation
ContractStorageTestClient::resizeStorageAndExtendInvocation(
std::string const& key, uint32_t numKiloBytes, uint32_t thresh,
Expand Down
5 changes: 5 additions & 0 deletions src/transactions/test/SorobanTxTestUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "test/test.h"
#include "transactions/TransactionFrameBase.h"
#include "transactions/TransactionUtils.h"
#include "xdr/Stellar-transaction.h"

namespace stellar
{
Expand Down Expand Up @@ -472,6 +473,10 @@ class ContractStorageTestClient
uint32_t threshold, uint32_t extendTo,
std::optional<SorobanInvocationSpec> spec = std::nullopt);

InvokeHostFunctionResultCode
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The restore() method should return RestoreFootprintResultCode instead of InvokeHostFunctionResultCode, as restore operations use RESTORE_FOOTPRINT operation type, not INVOKE_HOST_FUNCTION.

Suggested change
InvokeHostFunctionResultCode
RestoreFootprintResultCode

Copilot uses AI. Check for mistakes.
restore(std::string const& key, ContractDataDurability durability,
std::optional<SorobanInvocationSpec> spec = std::nullopt);

TestContract::Invocation resizeStorageAndExtendInvocation(
std::string const& key, uint32_t numKiloBytes, uint32_t thresh,
uint32_t extendTo,
Expand Down
Loading