Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup apply and validation flows #4471

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
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
3 changes: 1 addition & 2 deletions src/bucket/test/BucketListTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -726,8 +726,7 @@ TEST_CASE_VERSIONS("network config snapshots BucketList size", "[bucketlist]")

uint64_t correctAverage = sum / correctWindow.size();

LedgerTxn ltx(app->getLedgerTxnRoot(), false,
TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);
LedgerTxn ltx(app->getLedgerTxnRoot());
REQUIRE(networkConfig.getAverageBucketListSize() == correctAverage);

// Check on-disk sliding window
Expand Down
8 changes: 3 additions & 5 deletions src/herder/HerderImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2147,11 +2147,9 @@ HerderImpl::start()
{
mMaxTxSize = mApp.getHerder().getMaxClassicTxSize();
{
LedgerTxn ltx(mApp.getLedgerTxnRoot(),
/* shouldUpdateLastModified */ true,
TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);

uint32_t version = ltx.loadHeader().current().ledgerVersion;
uint32_t version = mApp.getLedgerManager()
.getLastClosedLedgerHeader()
.header.ledgerVersion;
if (protocolVersionStartsFrom(version, SOROBAN_PROTOCOL_VERSION))
{
auto const& conf =
Expand Down
4 changes: 0 additions & 4 deletions src/herder/HerderSCPDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -360,8 +360,6 @@ HerderSCPDriver::validateValue(uint64_t slotIndex, Value const& value,
validateValueHelper(slotIndex, b, nomination);
if (res != SCPDriver::kInvalidValue)
{
auto const& lcl = mLedgerManager.getLastClosedLedgerHeader();

LedgerUpgradeType lastUpgradeType = LEDGER_UPGRADE_VERSION;

// check upgrades
Expand Down Expand Up @@ -419,8 +417,6 @@ HerderSCPDriver::extractValidValue(uint64_t slotIndex, Value const& value)
if (validateValueHelper(slotIndex, b, true) ==
SCPDriver::kFullyValidatedValue)
{
auto const& lcl = mLedgerManager.getLastClosedLedgerHeader();

// remove the upgrade steps we don't like
LedgerUpgradeType thisUpgradeType;
for (auto it = b.upgrades.begin(); it != b.upgrades.end();)
Expand Down
7 changes: 4 additions & 3 deletions src/herder/TransactionQueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ TransactionQueue::canAdd(
{
auto txResult = tx->createSuccessResult();
if (!tx->checkSorobanResourceAndSetError(
mApp,
mApp.getAppConnector(),
mApp.getLedgerManager()
.getLastClosedLedgerHeader()
.header.ledgerVersion,
Expand Down Expand Up @@ -457,8 +457,9 @@ TransactionQueue::canAdd(
mApp.getLedgerManager().getLastClosedLedgerNum() + 1;
}

auto txResult = tx->checkValid(
mApp, ls, 0, 0, getUpperBoundCloseTimeOffset(mApp, closeTime));
auto txResult =
tx->checkValid(mApp.getAppConnector(), ls, 0, 0,
getUpperBoundCloseTimeOffset(mApp, closeTime));
if (!txResult->isSuccess())
{
return AddResult(TransactionQueue::AddResultCode::ADD_STATUS_ERROR,
Expand Down
3 changes: 2 additions & 1 deletion src/herder/TxSetFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,8 @@ phaseTxsAreValid(TxSetTransactions const& phase, Application& app,
app.getLedgerManager().getLastClosedLedgerNum() + 1;
for (auto const& tx : phase)
{
auto txResult = tx->checkValid(app, ls, 0, lowerBoundCloseTimeOffset,
auto txResult = tx->checkValid(app.getAppConnector(), ls, 0,
lowerBoundCloseTimeOffset,
upperBoundCloseTimeOffset);
if (!txResult->isSuccess())
{
Expand Down
3 changes: 2 additions & 1 deletion src/herder/TxSetUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ TxSetUtils::getInvalidTxList(TxSetTransactions const& txs, Application& app,

for (auto const& tx : txs)
{
auto txResult = tx->checkValid(app, ls, 0, lowerBoundCloseTimeOffset,
auto txResult = tx->checkValid(app.getAppConnector(), ls, 0,
lowerBoundCloseTimeOffset,
upperBoundCloseTimeOffset);
if (!txResult->isSuccess())
{
Expand Down
6 changes: 3 additions & 3 deletions src/herder/test/TxSetTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,7 @@ TEST_CASE("generalized tx set XDR conversion", "[txset]")
auto txSetFrame = TxSetXDRFrame::makeFromWire(txSetXdr);
ApplicableTxSetFrameConstPtr applicableFrame;
{
LedgerTxn ltx(app->getLedgerTxnRoot(), false,
TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);
LedgerTxn ltx(app->getLedgerTxnRoot());
applicableFrame = txSetFrame->prepareForApply(*app);
}
REQUIRE(applicableFrame->checkValid(*app, 0, 0));
Expand Down Expand Up @@ -846,7 +845,8 @@ TEST_CASE("generalized tx set fees", "[txset][soroban]")
LedgerTxn ltx(app->getLedgerTxnRoot());
if (validateTx)
{
REQUIRE(tx->checkValidForTesting(*app, ltx, 0, 0, 0));
REQUIRE(tx->checkValidForTesting(app->getAppConnector(), ltx, 0,
0, 0));
}
return tx;
}
Expand Down
5 changes: 3 additions & 2 deletions src/herder/test/UpgradesTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2433,8 +2433,9 @@ TEST_CASE_VERSIONS("upgrade base reserve", "[upgrades]")
auto submitTx = [&](TransactionTestFramePtr tx) {
LedgerTxn ltx(app->getLedgerTxnRoot());
TransactionMetaFrame txm(ltx.loadHeader().current().ledgerVersion);
REQUIRE(tx->checkValidForTesting(*app, ltx, 0, 0, 0));
REQUIRE(tx->apply(*app, ltx, txm));
REQUIRE(
tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, 0));
REQUIRE(tx->apply(app->getAppConnector(), ltx, txm));
ltx.commit();

REQUIRE(tx->getResultCode() == txSUCCESS);
Expand Down
14 changes: 4 additions & 10 deletions src/ledger/LedgerManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1139,8 +1139,7 @@ LedgerManagerImpl::setLastClosedLedger(

mRebuildInMemoryState = false;
advanceLedgerPointers(lastClosed.header);
LedgerTxn ltx2(mApp.getLedgerTxnRoot(), false,
TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);
LedgerTxn ltx2(mApp.getLedgerTxnRoot());
if (protocolVersionStartsFrom(ltx2.loadHeader().current().ledgerVersion,
SOROBAN_PROTOCOL_VERSION))
{
Expand Down Expand Up @@ -1322,12 +1321,7 @@ LedgerManagerImpl::updateNetworkConfig(AbstractLedgerTxn& rootLtx)
{
ZoneScoped;

uint32_t ledgerVersion{};
{
LedgerTxn ltx(rootLtx, false,
TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);
ledgerVersion = ltx.loadHeader().current().ledgerVersion;
}
uint32_t ledgerVersion = rootLtx.loadHeader().current().ledgerVersion;

if (protocolVersionStartsFrom(ledgerVersion, SOROBAN_PROTOCOL_VERSION))
{
Expand Down Expand Up @@ -1567,8 +1561,8 @@ LedgerManagerImpl::applyTransactions(
}
++txNum;

tx->apply(mApp, ltx, tm, mutableTxResult, subSeed);
tx->processPostApply(mApp, ltx, tm, mutableTxResult);
tx->apply(mApp.getAppConnector(), ltx, tm, mutableTxResult, subSeed);
tx->processPostApply(mApp.getAppConnector(), ltx, tm, mutableTxResult);
TransactionResultPair results;
results.transactionHash = tx->getContentsHash();
results.result = mutableTxResult->getResult();
Expand Down
2 changes: 2 additions & 0 deletions src/ledger/LedgerTxn.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,8 @@ enum class LedgerTxnConsistency
EXTRA_DELETES
};

// NOTE: Remove READ_ONLY_WITHOUT_SQL_TXN mode when BucketListDB is required
// and we stop supporting SQL backend for ledger state.
enum class TransactionMode
{
READ_ONLY_WITHOUT_SQL_TXN,
Expand Down
2 changes: 1 addition & 1 deletion src/ledger/NetworkConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -933,7 +933,7 @@ SorobanNetworkConfig::loadFromLedger(AbstractLedgerTxn& ltxRoot,
{
ZoneScoped;

LedgerTxn ltx(ltxRoot, false, TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);
LedgerTxn ltx(ltxRoot);
loadMaxContractSize(ltx);
loadMaxContractDataKeySize(ltx);
loadMaxContractDataEntrySize(ltx);
Expand Down
132 changes: 132 additions & 0 deletions src/main/AppConnector.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
#include "main/AppConnector.h"
#include "herder/Herder.h"
#include "invariant/InvariantManager.h"
#include "ledger/LedgerManager.h"
#include "ledger/LedgerTxn.h"
#include "main/Application.h"
#include "overlay/BanManager.h"
#include "overlay/OverlayManager.h"
#include "overlay/OverlayMetrics.h"
#include "util/Timer.h"

namespace stellar
{

AppConnector::AppConnector(Application& app)
: mApp(app), mConfig(app.getConfig())
{
}

Herder&
AppConnector::getHerder()
{
releaseAssert(threadIsMain());
return mApp.getHerder();
}

LedgerManager&
AppConnector::getLedgerManager()
{
releaseAssert(threadIsMain());
return mApp.getLedgerManager();
}

OverlayManager&
AppConnector::getOverlayManager()
{
releaseAssert(threadIsMain());
return mApp.getOverlayManager();
}

BanManager&
AppConnector::getBanManager()
{
releaseAssert(threadIsMain());
return mApp.getBanManager();
}

SorobanNetworkConfig const&
AppConnector::getSorobanNetworkConfig() const
{
releaseAssert(threadIsMain());
return mApp.getLedgerManager().getSorobanNetworkConfig();
}

medida::MetricsRegistry&
AppConnector::getMetrics() const
{
releaseAssert(threadIsMain());
return mApp.getMetrics();
}

SorobanMetrics&
AppConnector::getSorobanMetrics() const
{
releaseAssert(threadIsMain());
return mApp.getLedgerManager().getSorobanMetrics();
}

void
AppConnector::checkOnOperationApply(Operation const& operation,
OperationResult const& opres,
LedgerTxnDelta const& ltxDelta)
{
releaseAssert(threadIsMain());
mApp.getInvariantManager().checkOnOperationApply(operation, opres,
ltxDelta);
}

Hash const&
AppConnector::getNetworkID() const
{
releaseAssert(threadIsMain());
return mApp.getNetworkID();
}

void
AppConnector::postOnMainThread(std::function<void()>&& f, std::string&& message,
Scheduler::ActionType type)
{
mApp.postOnMainThread(std::move(f), std::move(message), type);
}

void
AppConnector::postOnOverlayThread(std::function<void()>&& f,
std::string const& message)
{
mApp.postOnOverlayThread(std::move(f), message);
}

Config const&
AppConnector::getConfig() const
{
return mConfig;
}

bool
AppConnector::overlayShuttingDown() const
{
return mApp.getOverlayManager().isShuttingDown();
}

VirtualClock::time_point
AppConnector::now() const
{
return mApp.getClock().now();
}

bool
AppConnector::shouldYield() const
{
releaseAssert(threadIsMain());
return mApp.getClock().shouldYield();
}

OverlayMetrics&
AppConnector::getOverlayMetrics()
{
// OverlayMetrics class is thread-safe
return mApp.getOverlayManager().getOverlayMetrics();
}

}
15 changes: 13 additions & 2 deletions src/overlay/OverlayAppConnector.h → src/main/AppConnector.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include "main/Config.h"
#include "medida/metrics_registry.h"

namespace stellar
{
Expand All @@ -10,25 +11,35 @@ class LedgerManager;
class Herder;
class BanManager;
struct OverlayMetrics;
class SorobanNetworkConfig;
class SorobanMetrics;
struct LedgerTxnDelta;

// Helper class to isolate access to Application; all function helpers must
// either be called from main or be thread-sade
class OverlayAppConnector
class AppConnector
{
Application& mApp;
// Copy config for threads to use, and avoid warnings from thread sanitizer
// about accessing mApp
Config const mConfig;

public:
OverlayAppConnector(Application& app);
AppConnector(Application& app);

// Methods that can only be called from main thread
Herder& getHerder();
LedgerManager& getLedgerManager();
OverlayManager& getOverlayManager();
BanManager& getBanManager();
bool shouldYield() const;
SorobanNetworkConfig const& getSorobanNetworkConfig() const;
medida::MetricsRegistry& getMetrics() const;
SorobanMetrics& getSorobanMetrics() const;
void checkOnOperationApply(Operation const& operation,
OperationResult const& opres,
LedgerTxnDelta const& ltxDelta);
Hash const& getNetworkID() const;

// Thread-safe methods
void postOnMainThread(
Expand Down
3 changes: 3 additions & 0 deletions src/main/Application.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class AbstractLedgerTxnParent;
class BasicWork;
enum class LoadGenMode;
struct GeneratedLoadConfig;
class AppConnector;

#ifdef BUILD_TESTS
class LoadGenerator;
Expand Down Expand Up @@ -326,6 +327,8 @@ class Application
// (while preserving the overlay data).
virtual void resetDBForInMemoryMode() = 0;

virtual AppConnector& getAppConnector() = 0;

protected:
Application()
{
Expand Down
8 changes: 8 additions & 0 deletions src/main/ApplicationImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "ledger/LedgerHeaderUtils.h"
#include "ledger/LedgerManager.h"
#include "ledger/LedgerTxn.h"
#include "main/AppConnector.h"
#include "main/ApplicationUtils.h"
#include "main/CommandHandler.h"
#include "main/ExternalQueue.h"
Expand Down Expand Up @@ -330,6 +331,7 @@ ApplicationImpl::initialize(bool createNewDB, bool forceRebuild)
mWorkScheduler = WorkScheduler::create(*this);
mBanManager = BanManager::create(*this);
mStatusManager = std::make_unique<StatusManager>();
mAppConnector = std::make_unique<AppConnector>(*this);

if (getConfig().MODE_USES_IN_MEMORY_LEDGER)
{
Expand Down Expand Up @@ -1635,4 +1637,10 @@ ApplicationImpl::getLedgerTxnRoot()
return mConfig.MODE_USES_IN_MEMORY_LEDGER ? *mNeverCommittingLedgerTxn
: *mLedgerTxnRoot;
}

AppConnector&
ApplicationImpl::getAppConnector()
{
return *mAppConnector;
}
}
Loading
Loading