Skip to content

Commit

Permalink
Remove ltx from network config getter.
Browse files Browse the repository at this point in the history
  • Loading branch information
dmkozh committed Nov 10, 2023
1 parent 0448bf0 commit 743a2ed
Show file tree
Hide file tree
Showing 28 changed files with 130 additions and 186 deletions.
2 changes: 1 addition & 1 deletion src/bucket/BucketList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ BucketList::scanForEviction(Application& app, AbstractLedgerTxn& ltx,
SOROBAN_PROTOCOL_VERSION))
{
auto const& networkConfig =
app.getLedgerManager().getSorobanNetworkConfig(ltx);
app.getLedgerManager().getSorobanNetworkConfig();
auto const firstScanLevel =
networkConfig.stateArchivalSettings().startingEvictionScanLevel;
auto evictionIter = networkConfig.evictionIterator();
Expand Down
9 changes: 3 additions & 6 deletions src/bucket/test/BucketListTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -673,10 +673,7 @@ TEST_CASE_VERSIONS("network config snapshots BucketList size", "[bucketlist]")
for_versions_from(20, *app, [&] {
LedgerManagerForBucketTests& lm = app->getLedgerManager();

LedgerTxn ltx(app->getLedgerTxnRoot());
auto& networkConfig =
app->getLedgerManager().getSorobanNetworkConfig(ltx);
ltx.~LedgerTxn();
auto& networkConfig = app->getLedgerManager().getSorobanNetworkConfig();

uint32_t windowSize = networkConfig.stateArchivalSettings()
.bucketListSizeWindowSampleSize;
Expand Down Expand Up @@ -718,7 +715,7 @@ TEST_CASE_VERSIONS("network config snapshots BucketList size", "[bucketlist]")

// Take snapshots more frequently for faster testing
app->getLedgerManager()
.getMutableSorobanNetworkConfig(ltx)
.getMutableSorobanNetworkConfig()
.setBucketListSnapshotPeriodForTesting(64);

// Generate enough ledgers to fill sliding window
Expand Down Expand Up @@ -770,7 +767,7 @@ TEST_CASE_VERSIONS("eviction scan", "[bucketlist]")

auto& networkCfg = [&]() -> SorobanNetworkConfig& {
LedgerTxn ltx(app->getLedgerTxnRoot());
return app->getLedgerManager().getMutableSorobanNetworkConfig(ltx);
return app->getLedgerManager().getMutableSorobanNetworkConfig();
}();

auto& stateArchivalSettings = networkCfg.stateArchivalSettings();
Expand Down
2 changes: 1 addition & 1 deletion src/bucket/test/BucketTestUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ LedgerManagerForBucketTests::transferLedgerEntriesToBucketList(
ltxEvictions.commit();
}
mApp.getLedgerManager()
.getMutableSorobanNetworkConfig(ltx)
.getMutableSorobanNetworkConfig()
.maybeSnapshotBucketListSize(ledgerSeq, ltx, mApp);
}

Expand Down
8 changes: 8 additions & 0 deletions src/catchup/CatchupWork.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,14 @@ CatchupWork::runCatchupStep()
return res;
}
}
{
LedgerTxn ltx(mApp.getLedgerTxnRoot());
if (protocolVersionStartsFrom(ltx.loadHeader().current().ledgerVersion,
SOROBAN_PROTOCOL_VERSION))
{
mApp.getLedgerManager().updateNetworkConfig(ltx);
}
}

// Step 4: Download, verify and apply ledgers, buckets and transactions

Expand Down
4 changes: 2 additions & 2 deletions src/herder/HerderImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2014,7 +2014,7 @@ HerderImpl::maybeHandleUpgrade()
// no-op on any earlier protocol
return;
}
auto const& conf = mApp.getLedgerManager().getSorobanNetworkConfig(ltx);
auto const& conf = mApp.getLedgerManager().getSorobanNetworkConfig();

if (conf.txMaxSizeBytes() > mMaxTxSize)
{
Expand Down Expand Up @@ -2066,7 +2066,7 @@ HerderImpl::start()
if (protocolVersionStartsFrom(version, SOROBAN_PROTOCOL_VERSION))
{
auto const& conf =
mApp.getLedgerManager().getSorobanNetworkConfig(ltx);
mApp.getLedgerManager().getSorobanNetworkConfig();
mMaxTxSize = std::max(mMaxTxSize, conf.txMaxSizeBytes());
}

Expand Down
22 changes: 9 additions & 13 deletions src/herder/TransactionQueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,12 @@ TransactionQueue::canAdd(TransactionFrameBasePtr tx,
LedgerTxn ltx(mApp.getLedgerTxnRoot(),
/* shouldUpdateLastModified */ true,
TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);
uint32_t ledgerVersion = ltx.loadHeader().current().ledgerVersion;
// Subtle: transactions are rejected based on the source account limit prior
// to this point. This is safe because we can't evict transactions from the
// same source account, so a newer transaction won't replace an old one.
auto canAddRes = mTxQueueLimiter->canAddTx(tx, oldTx, txsToEvict, ltx);
auto canAddRes =
mTxQueueLimiter->canAddTx(tx, oldTx, txsToEvict, ledgerVersion);
if (!canAddRes.first)
{
ban({tx});
Expand All @@ -361,9 +363,7 @@ TransactionQueue::canAdd(TransactionFrameBasePtr tx,
auto closeTime = mApp.getLedgerManager()
.getLastClosedLedgerHeader()
.header.scpValue.closeTime;

if (protocolVersionStartsFrom(ltx.loadHeader().current().ledgerVersion,
ProtocolVersion::V_19))
if (protocolVersionStartsFrom(ledgerVersion, ProtocolVersion::V_19))
{
// This is done so minSeqLedgerGap is validated against the next
// ledgerSeq, which is what will be used at apply time
Expand Down Expand Up @@ -976,7 +976,7 @@ TransactionQueue::clearAll()
LedgerTxn ltx(mApp.getLedgerTxnRoot(),
/* shouldUpdateLastModified */ true,
TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);
mTxQueueLimiter->reset(ltx);
mTxQueueLimiter->reset(ltx.loadHeader().current().ledgerVersion);
mKnownTxHashes.clear();
}

Expand Down Expand Up @@ -1232,9 +1232,7 @@ SorobanTransactionQueue::getMaxResourcesToFloodThisPeriod() const
auto const& cfg = mApp.getConfig();
double ratePerLedger = cfg.FLOOD_SOROBAN_RATE_PER_LEDGER;

LedgerTxn ltx(mApp.getLedgerTxnRoot(), /* shouldUpdateLastModified */ true,
TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);
auto sorRes = mApp.getLedgerManager().maxLedgerResources(true, ltx);
auto sorRes = mApp.getLedgerManager().maxLedgerResources(true);

auto totalFloodPerLedger = multiplyByDouble(sorRes, ratePerLedger);

Expand Down Expand Up @@ -1301,10 +1299,8 @@ SorobanTransactionQueue::broadcastSome()
std::make_shared<SorobanGenericLaneConfig>(resToFlood), mBroadcastSeed);
queue.visitTopTxs(trackersToBroadcast, visitor, mBroadcastOpCarryover);

LedgerTxn ltx(mApp.getLedgerTxnRoot(), true,
TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);
Resource maxPerTx =
mApp.getLedgerManager().maxSorobanTransactionResources(ltx);
mApp.getLedgerManager().maxSorobanTransactionResources();
for (auto& resLeft : mBroadcastOpCarryover)
{
// Limit carry-over to 1 maximum resource transaction
Expand All @@ -1322,7 +1318,7 @@ SorobanTransactionQueue::getMaxQueueSizeOps() const
if (protocolVersionStartsFrom(ltx.loadHeader().current().ledgerVersion,
SOROBAN_PROTOCOL_VERSION))
{
auto res = mTxQueueLimiter->maxScaledLedgerResources(true, ltx);
auto res = mTxQueueLimiter->maxScaledLedgerResources(true);
releaseAssert(res.size() == NUM_SOROBAN_TX_RESOURCES);
return res.getVal(Resource::Type::OPERATIONS);
}
Expand Down Expand Up @@ -1532,7 +1528,7 @@ ClassicTransactionQueue::getMaxQueueSizeOps() const
LedgerTxn ltx(mApp.getLedgerTxnRoot(),
/* shouldUpdateLastModified */ true,
TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);
auto res = mTxQueueLimiter->maxScaledLedgerResources(false, ltx);
auto res = mTxQueueLimiter->maxScaledLedgerResources(false);
releaseAssert(res.size() == NUM_CLASSIC_TX_RESOURCES);
return res.getVal(Resource::Type::OPERATIONS);
}
Expand Down
34 changes: 15 additions & 19 deletions src/herder/TxQueueLimiter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,10 @@ TxQueueLimiter::size() const
#endif

Resource
TxQueueLimiter::maxScaledLedgerResources(bool isSoroban,
AbstractLedgerTxn& ltxOuter) const
TxQueueLimiter::maxScaledLedgerResources(bool isSoroban) const
{
return multiplyByDouble(
mLedgerManager.maxLedgerResources(isSoroban, ltxOuter),
mPoolLedgerMultiplier);
return multiplyByDouble(mLedgerManager.maxLedgerResources(isSoroban),
mPoolLedgerMultiplier);
}

void
Expand Down Expand Up @@ -116,6 +114,7 @@ TxQueueLimiter::removeTransaction(TransactionFrameBasePtr const& tx)
}
}

#ifdef BUILD_TESTS
std::pair<bool, int64>
TxQueueLimiter::canAddTx(TransactionFrameBasePtr const& newTx,
TransactionFrameBasePtr const& oldTx,
Expand All @@ -124,14 +123,16 @@ TxQueueLimiter::canAddTx(TransactionFrameBasePtr const& newTx,

LedgerTxn ltx(mApp.getLedgerTxnRoot(), /* shouldUpdateLastModified */ true,
TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);
return canAddTx(newTx, oldTx, txsToEvict, ltx);
return canAddTx(newTx, oldTx, txsToEvict,
ltx.loadHeader().current().ledgerVersion);
}
#endif

std::pair<bool, int64>
TxQueueLimiter::canAddTx(TransactionFrameBasePtr const& newTx,
TransactionFrameBasePtr const& oldTx,
std::vector<std::pair<TxStackPtr, bool>>& txsToEvict,
AbstractLedgerTxn& ltxOuter)
uint32_t ledgerVersion)
{
releaseAssert(newTx);
releaseAssert(newTx->isSoroban() == mIsSoroban);
Expand All @@ -147,7 +148,7 @@ TxQueueLimiter::canAddTx(TransactionFrameBasePtr const& newTx,
// Resetting both is fine here, as we always reset at the same time
if (mTxs == nullptr)
{
reset(ltxOuter);
reset(ledgerVersion);
}

// If some transactions were evicted from this or generic lane, make sure
Expand Down Expand Up @@ -181,7 +182,7 @@ TxQueueLimiter::canAddTx(TransactionFrameBasePtr const& newTx,
// Update the operation limit in case upgrade happened. This is cheap
// enough to happen unconditionally without relying on upgrade triggers.
mSurgePricingLaneConfig->updateGenericLaneLimit(
Resource(maxScaledLedgerResources(newTx->isSoroban(), ltxOuter)));
Resource(maxScaledLedgerResources(newTx->isSoroban())));
return mTxs->canFitWithEviction(*newTx, oldTxDiscount, txsToEvict);
}

Expand All @@ -196,10 +197,7 @@ TxQueueLimiter::evictTransactions(

auto txToFitLane = mSurgePricingLaneConfig->getLane(txToFit);

LedgerTxn ltx(mApp.getLedgerTxnRoot(), /* shouldUpdateLastModified */ true,
TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);

auto maxLimits = maxScaledLedgerResources(txToFit.isSoroban(), ltx);
auto maxLimits = maxScaledLedgerResources(txToFit.isSoroban());

for (auto const& [evictedStack, evictedDueToLaneLimit] : txsToEvict)
{
Expand Down Expand Up @@ -242,17 +240,15 @@ TxQueueLimiter::evictTransactions(
}

void
TxQueueLimiter::reset(AbstractLedgerTxn& ltxOuter)
TxQueueLimiter::reset(uint32_t ledgerVersion)
{
if (mIsSoroban)
{
if (protocolVersionStartsFrom(
ltxOuter.loadHeader().current().ledgerVersion,
SOROBAN_PROTOCOL_VERSION))
if (protocolVersionStartsFrom(ledgerVersion, SOROBAN_PROTOCOL_VERSION))
{
mSurgePricingLaneConfig =
std::make_shared<SorobanGenericLaneConfig>(
maxScaledLedgerResources(mIsSoroban, ltxOuter));
maxScaledLedgerResources(mIsSoroban));
}
else
{
Expand All @@ -262,7 +258,7 @@ TxQueueLimiter::reset(AbstractLedgerTxn& ltxOuter)
else
{
mSurgePricingLaneConfig = std::make_shared<DexLimitingLaneConfig>(
maxScaledLedgerResources(mIsSoroban, ltxOuter), mMaxDexOperations);
maxScaledLedgerResources(mIsSoroban), mMaxDexOperations);
// Ensure byte limits aren't counted in tx limiter
releaseAssert(mSurgePricingLaneConfig->getLaneLimits()[0].size() ==
NUM_CLASSIC_TX_RESOURCES);
Expand Down
15 changes: 7 additions & 8 deletions src/herder/TxQueueLimiter.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,12 @@ class TxQueueLimiter
void removeTransaction(TransactionFrameBasePtr const& tx);
#ifdef BUILD_TESTS
size_t size() const;
std::pair<bool, int64>
canAddTx(TransactionFrameBasePtr const& tx,
TransactionFrameBasePtr const& oldTx,
std::vector<std::pair<TxStackPtr, bool>>& txsToEvict);
#endif
Resource maxScaledLedgerResources(bool isSoroban,
AbstractLedgerTxn& ltxOuter) const;
Resource maxScaledLedgerResources(bool isSoroban) const;

// Evict `txsToEvict` from the limiter by calling `evict`.
// `txsToEvict` should be provided by the `canAddTx` call.
Expand All @@ -114,16 +117,12 @@ class TxQueueLimiter
canAddTx(TransactionFrameBasePtr const& tx,
TransactionFrameBasePtr const& oldTx,
std::vector<std::pair<TxStackPtr, bool>>& txsToEvict,
AbstractLedgerTxn& ltxOuter);
std::pair<bool, int64>
canAddTx(TransactionFrameBasePtr const& tx,
TransactionFrameBasePtr const& oldTx,
std::vector<std::pair<TxStackPtr, bool>>& txsToEvict);
uint32_t ledgerVersion);

// Resets the state related to evictions (maximum evicted bid).
void resetEvictionState();

// Resets the internal transaction container and the eviction state.
void reset(AbstractLedgerTxn& ltxOuter);
void reset(uint32_t ledgerVersion);
};
}
7 changes: 2 additions & 5 deletions src/herder/TxSetFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ ApplicableTxSetFrame::checkValid(Application& app,
{
LedgerTxn ltx(app.getLedgerTxnRoot());
auto limits = app.getLedgerManager().maxLedgerResources(
/* isSoroban */ true, ltx);
/* isSoroban */ true);
if (anyGreater(*totalTxSetRes, limits))
{
CLOG_DEBUG(Herder,
Expand Down Expand Up @@ -1435,11 +1435,8 @@ ApplicableTxSetFrame::applySurgePricing(Application& app)
releaseAssert(isGeneralizedTxSet());
releaseAssert(phaseType == TxSetFrame::Phase::SOROBAN);

LedgerTxn ltx(app.getLedgerTxnRoot(), false,
TransactionMode::READ_ONLY_WITHOUT_SQL_TXN);

auto limits = app.getLedgerManager().maxLedgerResources(
/* isSoroban */ true, ltx);
/* isSoroban */ true);

auto byteLimit =
std::min(static_cast<int64_t>(MAX_SOROBAN_BYTE_ALLOWANCE),
Expand Down
20 changes: 5 additions & 15 deletions src/herder/test/HerderTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1571,15 +1571,9 @@ TEST_CASE("tx set hits overlay byte limit during construction",
cfg.mLedgerMaxInstructions = max;
});

SorobanNetworkConfig conf;
auto const& conf = app->getLedgerManager().getSorobanNetworkConfig();
uint32_t maxContractSize = 0;
{
LedgerTxn ltx(app->getLedgerTxnRoot());
conf = app->getLedgerManager().getSorobanNetworkConfig(ltx);
maxContractSize = app->getLedgerManager()
.getSorobanNetworkConfig(ltx)
.maxContractSizeBytes();
}
maxContractSize = conf.maxContractSizeBytes();

auto makeTx = [&](TestAccount& acc, TxSetFrame::Phase const& phase) {
if (phase == TxSetFrame::Phase::SOROBAN)
Expand Down Expand Up @@ -1739,11 +1733,8 @@ TEST_CASE("surge pricing", "[herder][txset][soroban]")
// Valid classic
auto tx = makeMultiPayment(acc1, root, 1, 100, 0, 1);

SorobanNetworkConfig conf;
{
LedgerTxn ltx(app->getLedgerTxnRoot());
conf = app->getLedgerManager().getSorobanNetworkConfig(ltx);
}
SorobanNetworkConfig conf =
app->getLedgerManager().getSorobanNetworkConfig();

uint32_t const baseFee = 10'000'000;
SorobanResources resources;
Expand Down Expand Up @@ -3937,9 +3928,8 @@ TEST_CASE("soroban txs accepted by the network",
simulation->crankForAtLeast(std::chrono::seconds(20), false);
for (auto node : nodes)
{
LedgerTxn ltx(node->getLedgerTxnRoot());
REQUIRE(node->getLedgerManager()
.getSorobanNetworkConfig(ltx)
.getSorobanNetworkConfig()
.ledgerMaxTxCount() == ledgerWideLimit * 10);
}

Expand Down
21 changes: 7 additions & 14 deletions src/herder/test/TransactionQueueTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1158,11 +1158,8 @@ TEST_CASE("Soroban TransactionQueue limits",
auto account1 = root.create("a1", minBalance2);
auto account2 = root.create("a2", minBalance2);

SorobanNetworkConfig conf;
{
LedgerTxn ltx(app->getLedgerTxnRoot());
conf = app->getLedgerManager().getSorobanNetworkConfig(ltx);
}
SorobanNetworkConfig conf =
app->getLedgerManager().getSorobanNetworkConfig();

SorobanResources resources;
resources.instructions = 2'000'000;
Expand Down Expand Up @@ -1407,18 +1404,14 @@ TEST_CASE("Soroban TransactionQueue limits",
}
SECTION("limited lane eviction")
{
std::shared_ptr<Resource> limits = nullptr;
{
LedgerTxn ltx(app->getLedgerTxnRoot());
limits = std::make_shared<Resource>(
app->getLedgerManager().maxLedgerResources(true, ltx));
}
Resource limits = app->getLedgerManager().maxLedgerResources(true);

// Setup limits: generic fits 1 ledger worth of resources, while limited
// lane fits 1/4 ledger
auto limitedLane = std::optional<Resource>(
bigDivideOrThrow(*limits, 1, 4, Rounding::ROUND_UP));
Resource limitedLane(
bigDivideOrThrow(limits, 1, 4, Rounding::ROUND_UP));
auto config = std::make_shared<SorobanLimitingLaneConfigForTesting>(
*limits, limitedLane);
limits, limitedLane);
auto queue = std::make_unique<SurgePricingPriorityQueue>(
/* isHighestPriority */ false, config, 1);

Expand Down
Loading

0 comments on commit 743a2ed

Please sign in to comment.