From 45c677d2fb63750f61afb70bc78c8a084478366f Mon Sep 17 00:00:00 2001 From: dmkozh Date: Mon, 14 Sep 2026 13:58:10 -0400 Subject: [PATCH 1/2] Simplify the ledger view hierarchy and interfaces. - CheckValidLedgerViewWrapper is gone. It was used only for test compatibility, which is is unnecessary. It was also used outside of tx validation quite a bit, which made it even more confusing. - LedgerTxnReadOnly is renamed to LedgerTxnView for clarity, and now it's used more generally anywhere we need an adapter from LTX to ledger view - executeWithMaybeInnerSnapshot is gone. We only need the hacky pre-p8 logic in 2 places, and now it's used there explicitly (with LedgerTxnView on the child transaction) - All views now provide network config with `getSorobanNetworkConfig` consistently. We don't use it too broadly at the moment, but it makes sense for views to be able to expose the config (in the same way as it makes sense for them to expose the ledger header). - SorobanNetworkConfig is now loaded using a closure in order to avoid a weird circular dependency that the previous step would introduce (as we used to load the config from an AbstractLedgerView) - ImmutableLedgerData now always automatically loads the network config, which reduces awkwardness around creating a throw-away view (another symptom of the circular dependency weirdness above) - Concrete `copyImmutableLedgerView()` is only exposed to tests. I still dislike the name (as it's unclear which state does the view represent), but I kept it as is for now to reduce the change amount a bit. All the production users can get by with the new `getLCLView()` that returns an instance of an abstract ledger view of LCL. Also renamed `maybeUpdateImmutableLedgerView` to `syncWithLCLView` for clarity. Consumers also use abstract view, which ties the above changes together. - `ledgerView.mSkipSeqNumCheck` is gone, we just check the overlay-only mode flag directly. - Misc minor cleanups and comment updates. --- src/bucket/BucketManager.cpp | 7 +- src/bucket/test/BucketIndexTests.cpp | 4 +- src/bucket/test/BucketListTests.cpp | 13 +- src/bucket/test/BucketTestUtils.cpp | 4 +- src/herder/HerderImpl.cpp | 8 +- src/herder/TransactionQueue.cpp | 14 +- src/herder/TxSetUtils.cpp | 18 +- src/herder/Upgrades.cpp | 31 +- src/herder/Upgrades.h | 19 +- src/herder/test/HerderTests.cpp | 108 ++++--- src/herder/test/TxSetTests.cpp | 43 ++- src/herder/test/UpgradesTests.cpp | 47 +-- src/invariant/BucketListStateConsistency.cpp | 11 +- src/ledger/ImmutableLedgerView.cpp | 184 ++++------- src/ledger/ImmutableLedgerView.h | 109 ++----- src/ledger/InMemorySorobanState.cpp | 7 +- src/ledger/LedgerManager.h | 46 ++- src/ledger/LedgerManagerImpl.cpp | 122 ++++++-- src/ledger/LedgerManagerImpl.h | 18 +- src/ledger/NetworkConfig.cpp | 139 +++++---- src/ledger/NetworkConfig.h | 51 +-- src/ledger/test/ImmutableLedgerViewTests.cpp | 101 +++--- src/main/AppConnector.cpp | 14 +- src/main/AppConnector.h | 4 +- src/main/ApplicationImpl.cpp | 1 + src/main/ApplicationImpl.h | 5 +- src/main/ApplicationUtils.cpp | 17 +- src/main/CommandHandler.cpp | 21 +- src/main/test/CommandHandlerTests.cpp | 4 +- src/overlay/OverlayManager.h | 6 +- src/overlay/OverlayManagerImpl.cpp | 11 +- src/overlay/OverlayManagerImpl.h | 4 +- src/overlay/Peer.cpp | 14 +- src/overlay/test/OverlayTests.cpp | 22 +- src/simulation/ApplyLoad.cpp | 24 +- src/simulation/LoadGenerator.cpp | 6 +- src/simulation/TxGenerator.cpp | 16 +- src/test/TestAccount.cpp | 81 ++--- src/test/TxTests.cpp | 29 +- src/test/fuzz/targets/TxFuzzTarget.cpp | 20 +- src/transactions/FeeBumpTransactionFrame.cpp | 29 +- src/transactions/FeeBumpTransactionFrame.h | 29 +- src/transactions/OperationFrame.cpp | 133 ++++---- src/transactions/OperationFrame.h | 11 +- src/transactions/ParallelApplyUtils.cpp | 7 +- src/transactions/TransactionFrame.cpp | 295 ++++++++---------- src/transactions/TransactionFrame.h | 78 +++-- src/transactions/TransactionFrameBase.h | 14 +- .../BeginSponsoringFutureReservesTests.cpp | 26 +- src/transactions/test/ChangeTrustTests.cpp | 35 ++- .../test/ClaimableBalanceTests.cpp | 31 +- src/transactions/test/CreateAccountTests.cpp | 28 +- .../test/EndSponsoringFutureReservesTests.cpp | 6 +- .../test/FeeBumpTransactionTests.cpp | 129 ++++---- .../test/FrozenLedgerKeysTests.cpp | 48 +-- .../test/InvokeHostFunctionTests.cpp | 95 +++--- src/transactions/test/ManageBuyOfferTests.cpp | 5 +- src/transactions/test/ParallelApplyTest.cpp | 21 +- src/transactions/test/PaymentTests.cpp | 4 +- .../test/RevokeSponsorshipTests.cpp | 25 +- .../test/SetTrustLineFlagsTests.cpp | 36 ++- src/transactions/test/SorobanTxTestUtils.cpp | 18 +- .../test/TransactionTestFrame.cpp | 66 ++-- src/transactions/test/TransactionTestFrame.h | 22 +- src/transactions/test/TxEnvelopeTests.cpp | 270 ++++++++-------- 65 files changed, 1448 insertions(+), 1416 deletions(-) diff --git a/src/bucket/BucketManager.cpp b/src/bucket/BucketManager.cpp index 232f9a874f..e3b6f16164 100644 --- a/src/bucket/BucketManager.cpp +++ b/src/bucket/BucketManager.cpp @@ -1185,10 +1185,9 @@ BucketManager::resolveBackgroundEvictionScan( ZoneScoped; releaseAssert(mEvictionStatistics); auto timer = mBucketListEvictionMetrics.blockingTime.TimeScope(); - LedgerTxnReadOnly ltxSnap(ltx); - auto ledgerSeq = ltxSnap.getLedgerHeader().current().ledgerSeq; - auto ledgerVers = ltxSnap.getLedgerHeader().current().ledgerVersion; - auto networkConfig = SorobanNetworkConfig::loadFromLedger(ltxSnap); + auto ledgerSeq = ltx.loadHeader().current().ledgerSeq; + auto ledgerVers = ltx.loadHeader().current().ledgerVersion; + auto networkConfig = SorobanNetworkConfig::loadFromLedger(ltx); releaseAssert(ledgerSeq == lclApplyView.getLedgerSeq() + 1); if (!mEvictionFuture.valid()) diff --git a/src/bucket/test/BucketIndexTests.cpp b/src/bucket/test/BucketIndexTests.cpp index 4b14f0f881..f42be7629f 100644 --- a/src/bucket/test/BucketIndexTests.cpp +++ b/src/bucket/test/BucketIndexTests.cpp @@ -1780,8 +1780,8 @@ TEST_CASE("scanForLiveEntriesOfType loser tree unit tests", HistoryArchiveState has; has.currentLedger = lcl.header.ledgerSeq; auto state = std::make_shared( - bl, bm.getHotArchiveBucketList(), lcl, has, std::nullopt, - app->getMetrics()); + bl, bm.getHotArchiveBucketList(), lcl, has, + SorobanNetworkConfig::emptyConfig(), app->getMetrics()); return ImmutableLedgerView{state, app->getMetrics()}; }; diff --git a/src/bucket/test/BucketListTests.cpp b/src/bucket/test/BucketListTests.cpp index e76064db3a..257d3f18f6 100644 --- a/src/bucket/test/BucketListTests.cpp +++ b/src/bucket/test/BucketListTests.cpp @@ -1112,16 +1112,17 @@ TEST_CASE_VERSIONS("eviction scan", "[bucketlist][archival][soroban]") LedgerKey stateArchivalKey(CONFIG_SETTING); stateArchivalKey.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_STATE_ARCHIVAL; - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto stateArchivalEntry = - ledgerView.load(stateArchivalKey).current(); + ledgerView->load(stateArchivalKey).current(); modifyStateArchivalFn(stateArchivalEntry.data.configSetting() .stateArchivalSettings()); LedgerKey evictionIterKey(CONFIG_SETTING); evictionIterKey.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_EVICTION_ITERATOR; - auto evictionIterEntry = ledgerView.load(evictionIterKey).current(); + auto evictionIterEntry = + ledgerView->load(evictionIterKey).current(); modifyEvictionIteratorFn( evictionIterEntry.data.configSetting().evictionIterator()); @@ -1723,11 +1724,11 @@ TEST_CASE_VERSIONS("Searchable BucketListDB snapshots", "[bucketlist]") } closeLedger(*app); - auto blLedgerView = app->getLedgerManager().copyImmutableLedgerView(); + auto blLedgerView = app->getLedgerManager().getLCLView(); // Snapshot should automatically update with latest version - auto loadedEntry = blLedgerView.loadLiveEntry(LedgerEntryKey(entry)); - REQUIRE((loadedEntry && *loadedEntry == entry)); + auto loadedEntry = blLedgerView->load(LedgerEntryKey(entry)); + REQUIRE((loadedEntry && loadedEntry.current() == entry)); } } diff --git a/src/bucket/test/BucketTestUtils.cpp b/src/bucket/test/BucketTestUtils.cpp index 1e8da564ea..53a613a0e4 100644 --- a/src/bucket/test/BucketTestUtils.cpp +++ b/src/bucket/test/BucketTestUtils.cpp @@ -318,9 +318,9 @@ LedgerManagerForBucketTests::finalizeLedgerTxnChanges( HistoryArchiveState tempHas; tempHas.currentLedger = lh.ledgerSeq; auto& bm = mApp.getBucketManager(); - auto tempState = ImmutableLedgerData::createAndMaybeLoadConfig( + auto tempState = std::make_shared( bm.getLiveBucketList(), bm.getHotArchiveBucketList(), tempLcl, - tempHas, mApp.getMetrics()); + tempHas, std::nullopt, mApp.getMetrics()); finalSorobanConfig = tempState->getSorobanConfig(); } diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp index 42baaeadc0..42cf955ee8 100644 --- a/src/herder/HerderImpl.cpp +++ b/src/herder/HerderImpl.cpp @@ -1787,8 +1787,8 @@ HerderImpl::triggerNextLedger(uint32_t ledgerSeqToTrigger, // see if we need to include some upgrades std::vector upgrades; { - CheckValidLedgerViewWrapper ledgerView(mApp); - upgrades = mUpgrades.createUpgradesFor(lcl.header, ledgerView, + auto ledgerView = mApp.getLedgerManager().getLCLView(); + upgrades = mUpgrades.createUpgradesFor(lcl.header, *ledgerView, mApp.getConfig()); } for (auto const& upgrade : upgrades) @@ -1868,8 +1868,8 @@ HerderImpl::setUpgrades(Upgrades::UpgradeParameters const& upgrades) std::string HerderImpl::getUpgradesJson() { - auto ledgerView = CheckValidLedgerViewWrapper(mApp); - return mUpgrades.getParameters().toDebugJson(ledgerView); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + return mUpgrades.getParameters().toDebugJson(*ledgerView); } void diff --git a/src/herder/TransactionQueue.cpp b/src/herder/TransactionQueue.cpp index af1d1638a7..958596bef5 100644 --- a/src/herder/TransactionQueue.cpp +++ b/src/herder/TransactionQueue.cpp @@ -414,13 +414,7 @@ TransactionQueue::canAdd( } } - CheckValidLedgerViewWrapper ledgerView(mApp); -#ifdef BUILD_TESTS - // Overlay-only mode freezes on-disk seqnums at genesis but LoadGenerator - // keeps advancing local ones, so checkValid must skip the seqnum equality - // check or every tx after the first fails. - ledgerView.mSkipSeqNumCheck = mApp.getRunInOverlayOnlyMode(); -#endif + auto ledgerView = mApp.getLedgerManager().getLCLView(); // 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 @@ -461,7 +455,7 @@ TransactionQueue::canAdd( #endif { auto validationResult = tx->checkValidForOverlay( - mApp.getAppConnector(), ledgerView, 0, 0, + mApp.getAppConnector(), *ledgerView, 0, 0, getUpperBoundCloseTimeOffset(mApp, closeTime), diagnosticEvents, validationLedgerSeq); if (!validationResult->isSuccess()) @@ -480,12 +474,12 @@ TransactionQueue::canAdd( if (!isLoadgenTx) #endif { - auto const feeSource = ledgerView.getAccount(tx->getFeeSourceID()); + auto const feeSource = ledgerView->getAccount(tx->getFeeSourceID()); auto feeStateIter = mAccountStates.find(tx->getFeeSourceID()); int64_t totalFees = feeStateIter == mAccountStates.end() ? 0 : feeStateIter->second.mTotalFees; - if (getAvailableBalance(ledgerView.getLedgerHeader().current(), + if (getAvailableBalance(ledgerView->getLedgerHeader().current(), feeSource.current()) - newFullFee < totalFees) diff --git a/src/herder/TxSetUtils.cpp b/src/herder/TxSetUtils.cpp index 2a98d38b32..2153393fcf 100644 --- a/src/herder/TxSetUtils.cpp +++ b/src/herder/TxSetUtils.cpp @@ -171,16 +171,13 @@ TxSetUtils::getInvalidTxListWithErrors( { ZoneScoped; releaseAssert(threadIsMain()); - CheckValidLedgerViewWrapper ledgerView(app); -#ifdef BUILD_TESTS - // See TransactionQueue::canAdd for the overlay-only-mode rationale. - ledgerView.mSkipSeqNumCheck = app.getRunInOverlayOnlyMode(); -#endif + + auto ledgerView = app.getLedgerManager().getLCLView(); // Validate minSeqLedgerGap and LedgerBounds against the next ledgerSeq, // which is what will be used at apply time. std::optional validationLedgerSeq; if (protocolVersionStartsFrom( - ledgerView.getLedgerHeader().current().ledgerVersion, + ledgerView->getLedgerHeader().current().ledgerVersion, ProtocolVersion::V_19)) { validationLedgerSeq = @@ -188,8 +185,7 @@ TxSetUtils::getInvalidTxListWithErrors( } TxFrameListWithErrors invalidTxsWithError; - auto& invalidTxs = invalidTxsWithError.first; - auto& errorCode = invalidTxsWithError.second; + auto& [invalidTxs, errorCode] = invalidTxsWithError; errorCode = TxSetValidationResult::VALID; std::unordered_set seenInvalidTxs; @@ -197,7 +193,7 @@ TxSetUtils::getInvalidTxListWithErrors( for (auto const& tx : txs) { auto txResult = tx->checkValid( - app.getAppConnector(), ledgerView, 0, lowerBoundCloseTimeOffset, + app.getAppConnector(), *ledgerView, 0, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset, diagnostics, validationLedgerSeq); if (!txResult->isSuccess()) { @@ -219,7 +215,7 @@ TxSetUtils::getInvalidTxListWithErrors( } } - auto header = ledgerView.getLedgerHeader().current(); + auto header = ledgerView->getLedgerHeader().current(); for (auto const& tx : txs) { // Already added invalid tx @@ -229,7 +225,7 @@ TxSetUtils::getInvalidTxListWithErrors( } auto feeSourceID = tx->getFeeSourceID(); - auto feeSource = ledgerView.getAccount(feeSourceID); + auto feeSource = ledgerView->getAccount(feeSourceID); // feeSource should exist since we've already run checkValid, log // internal bug if (!feeSource) diff --git a/src/herder/Upgrades.cpp b/src/herder/Upgrades.cpp index e8b1423f64..2053044fa7 100644 --- a/src/herder/Upgrades.cpp +++ b/src/herder/Upgrades.cpp @@ -174,7 +174,7 @@ namespace stellar namespace { uint32_t -readMaxSorobanTxSetSize(CheckValidLedgerViewWrapper const& ledgerView) +readMaxSorobanTxSetSize(AbstractLedgerView const& ledgerView) { LedgerKey key(LedgerEntryType::CONFIG_SETTING); key.configSetting().configSettingID = @@ -212,7 +212,7 @@ Upgrades::UpgradeParameters::toJson() const std::string Upgrades::UpgradeParameters::toDebugJson( - CheckValidLedgerViewWrapper const& ledgerView) const + AbstractLedgerView const& ledgerView) const { Json::Value upgradesJson; Json::Reader reader; @@ -278,7 +278,7 @@ Upgrades::getParameters() const std::vector Upgrades::createUpgradesFor(LedgerHeader const& lclHeader, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, Config const& appCfg) const { auto result = std::vector{}; @@ -368,9 +368,9 @@ Upgrades::applyTo(LedgerUpgrade const& upgrade, Application& app, break; case LEDGER_UPGRADE_CONFIG: { - CheckValidLedgerViewWrapper ltxState(ltx); + LedgerTxnView ltxView(ltx); auto cfgUpgrade = - ConfigUpgradeSetFrame::makeFromKey(ltxState, upgrade.newConfig()); + ConfigUpgradeSetFrame::makeFromKey(ltxView, upgrade.newConfig()); if (!cfgUpgrade) { throw std::runtime_error( @@ -568,7 +568,7 @@ Upgrades::removeUpgrades(std::vector::const_iterator beginUpdates, Upgrades::UpgradeValidity Upgrades::isValidForApply(UpgradeType const& opaqueUpgrade, LedgerUpgrade& upgrade, Application& app, - CheckValidLedgerViewWrapper const& ledgerView) + AbstractLedgerView const& ledgerView) { try { @@ -641,9 +641,8 @@ Upgrades::isValidForApply(UpgradeType const& opaqueUpgrade, } bool -Upgrades::isValidForNomination( - LedgerUpgrade const& upgrade, - CheckValidLedgerViewWrapper const& ledgerView) const +Upgrades::isValidForNomination(LedgerUpgrade const& upgrade, + AbstractLedgerView const& ledgerView) const { if (!timeForUpgrade( ledgerView.getLedgerHeader().current().scpValue.closeTime)) @@ -693,13 +692,13 @@ Upgrades::isValid(UpgradeType const& upgrade, LedgerUpgradeType& upgradeType, bool nomination, Application& app) const { LedgerUpgrade lupgrade; - auto ledgerView = CheckValidLedgerViewWrapper(app); - bool res = isValidForApply(upgrade, lupgrade, app, ledgerView) == + auto ledgerView = app.getLedgerManager().getLCLView(); + bool res = isValidForApply(upgrade, lupgrade, app, *ledgerView) == UpgradeValidity::VALID; if (nomination) { - res = res && isValidForNomination(lupgrade, ledgerView); + res = res && isValidForNomination(lupgrade, *ledgerView); } if (res) @@ -1296,9 +1295,8 @@ Upgrades::applyReserveUpgrade(AbstractLedgerTxn& ltx, uint32_t newReserve) } ConfigUpgradeSetFrameConstPtr -ConfigUpgradeSetFrame::makeFromKey( - CheckValidLedgerViewWrapper const& ledgerView, - ConfigUpgradeSetKey const& key) +ConfigUpgradeSetFrame::makeFromKey(AbstractLedgerView const& ledgerView, + ConfigUpgradeSetKey const& key) { auto lk = ConfigUpgradeSetFrame::getLedgerKey(key); auto ltxe = ledgerView.load(lk); @@ -1422,8 +1420,7 @@ ConfigUpgradeSetFrame::getLedgerKey(ConfigUpgradeSetKey const& upgradeKey) } bool -ConfigUpgradeSetFrame::upgradeNeeded( - CheckValidLedgerViewWrapper const& ledgerView) const +ConfigUpgradeSetFrame::upgradeNeeded(AbstractLedgerView const& ledgerView) const { if (protocolVersionIsBefore( ledgerView.getLedgerHeader().current().ledgerVersion, diff --git a/src/herder/Upgrades.h b/src/herder/Upgrades.h index b0bc8feb7b..455d379f58 100644 --- a/src/herder/Upgrades.h +++ b/src/herder/Upgrades.h @@ -20,7 +20,7 @@ class Config; class Database; struct LedgerHeader; struct LedgerUpgrade; -class CheckValidLedgerViewWrapper; +class AbstractLedgerView; class ConfigUpgradeSetFrame; using ConfigUpgradeSetFrameConstPtr = @@ -70,8 +70,7 @@ class Upgrades std::string toJson() const; void fromJson(std::string const& s); - std::string - toDebugJson(CheckValidLedgerViewWrapper const& ledgerView) const; + std::string toDebugJson(AbstractLedgerView const& ledgerView) const; }; Upgrades() @@ -86,7 +85,7 @@ class Upgrades // create upgrades for given ledger std::vector createUpgradesFor(LedgerHeader const& lclHeader, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, Config const& appCfg) const; // apply upgrade to ledger header @@ -110,8 +109,7 @@ class Upgrades // If the upgrade could be deserialized then lupgrade is set static UpgradeValidity isValidForApply(UpgradeType const& upgrade, LedgerUpgrade& lupgrade, - Application& app, - CheckValidLedgerViewWrapper const& ledgerView); + Application& app, AbstractLedgerView const& ledgerView); // returns true if upgrade is a valid upgrade step // in which case it also sets upgradeType @@ -137,9 +135,8 @@ class Upgrades // returns true if upgrade is a valid upgrade step // in which case it also sets lupgrade - bool - isValidForNomination(LedgerUpgrade const& upgrade, - CheckValidLedgerViewWrapper const& ledgerView) const; + bool isValidForNomination(LedgerUpgrade const& upgrade, + AbstractLedgerView const& ledgerView) const; static void applyVersionUpgrade(Application& app, AbstractLedgerTxn& ltx, uint32_t newVersion); @@ -159,7 +156,7 @@ class ConfigUpgradeSetFrame { public: static ConfigUpgradeSetFrameConstPtr - makeFromKey(CheckValidLedgerViewWrapper const& ledgerView, + makeFromKey(AbstractLedgerView const& ledgerView, ConfigUpgradeSetKey const& key); static LedgerKey getLedgerKey(ConfigUpgradeSetKey const& upgradeKey); @@ -168,7 +165,7 @@ class ConfigUpgradeSetFrame ConfigUpgradeSetKey const& getKey() const; - bool upgradeNeeded(CheckValidLedgerViewWrapper const& ledgerView) const; + bool upgradeNeeded(AbstractLedgerView const& ledgerView) const; void applyTo(AbstractLedgerTxn& ltx, Application& app) const; diff --git a/src/herder/test/HerderTests.cpp b/src/herder/test/HerderTests.cpp index 5d18f1ea63..69bcb6da0b 100644 --- a/src/herder/test/HerderTests.cpp +++ b/src/herder/test/HerderTests.cpp @@ -316,7 +316,7 @@ testTxSet(uint32 protocolVersion) SECTION("invalid tx") { auto diagnostics = DiagnosticEventManager::createDisabled(); - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); SECTION("no user") { @@ -326,7 +326,7 @@ testTxSet(uint32 protocolVersion) // Individual tx check: account doesn't exist REQUIRE(badTx - ->checkValid(app->getAppConnector(), ledgerView, 0, 0, + ->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0, diagnostics) ->getResultCode() == txNO_ACCOUNT); @@ -356,7 +356,7 @@ testTxSet(uint32 protocolVersion) // Individual tx check: bad sequence number REQUIRE(badTx - ->checkValid(app->getAppConnector(), ledgerView, 0, 0, + ->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0, diagnostics) ->getResultCode() == txBAD_SEQ); @@ -387,9 +387,9 @@ testTxSet(uint32 protocolVersion) // Individual tx check: insufficient balance // Need fresh snapshot after account creation - CheckValidLedgerViewWrapper lsNew(*app); + auto lsNew = app->getLedgerManager().getLCLView(); REQUIRE(badTx - ->checkValid(app->getAppConnector(), lsNew, 0, 0, 0, + ->checkValid(app->getAppConnector(), *lsNew, 0, 0, 0, diagnostics) ->getResultCode() == txINSUFFICIENT_BALANCE); @@ -421,7 +421,7 @@ testTxSet(uint32 protocolVersion) // Individual tx check: bad auth (signature invalidated by maxTime // change) REQUIRE(badTx - ->checkValid(app->getAppConnector(), ledgerView, 0, 0, + ->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0, diagnostics) ->getResultCode() == txBAD_AUTH); @@ -593,7 +593,7 @@ testTxSetWithFeeBumps(uint32 protocolVersion) }; auto diagnostics = DiagnosticEventManager::createDisabled(); - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); SECTION("invalid transaction") { @@ -603,8 +603,8 @@ testTxSetWithFeeBumps(uint32 protocolVersion) auto fb1 = feeBump(*app, account2, tx1, minBalance2); // Individual tx check: fee bump exceeds fee source balance - REQUIRE(fb1->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb1->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->getResultCode() == txINSUFFICIENT_BALANCE); SECTION("build block") @@ -629,11 +629,11 @@ testTxSetWithFeeBumps(uint32 protocolVersion) auto fb2 = feeBump(*app, account2, tx2, 200); // Individual tx checks: first exceeds balance, second is valid - REQUIRE(fb1->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb1->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->getResultCode() == txINSUFFICIENT_BALANCE); - REQUIRE(fb2->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb2->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->isSuccess()); SECTION("build block") @@ -657,11 +657,11 @@ testTxSetWithFeeBumps(uint32 protocolVersion) auto tx2 = transaction(*app, account3, 1, 1, 100); auto fb2 = feeBump(*app, account2, tx2, minBalance2); - REQUIRE(fb1->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb1->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->isSuccess()); - REQUIRE(fb2->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb2->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->getResultCode() == txINSUFFICIENT_BALANCE); SECTION("build block") @@ -686,11 +686,11 @@ testTxSetWithFeeBumps(uint32 protocolVersion) auto fb2 = feeBump(*app, account2, tx2, 200); // Individual tx checks - REQUIRE(fb1->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb1->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->isSuccess()); - REQUIRE(fb2->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb2->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->getResultCode() == txFEE_BUMP_INNER_FAILED); SECTION("build block") @@ -718,15 +718,15 @@ testTxSetWithFeeBumps(uint32 protocolVersion) feeBump(*app, account2, tx3, minBalance2 - minBalance0 - 199); // Individual tx checks - REQUIRE(fb1->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb1->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->isSuccess()); - REQUIRE(fb2->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb2->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->getResultCode() == txFEE_BUMP_INNER_FAILED); // Individually, fb2 is valid, but with fb1 it would exceed balance - REQUIRE(fb3->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb3->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->isSuccess()); SECTION("build block") @@ -746,10 +746,10 @@ testTxSetWithFeeBumps(uint32 protocolVersion) SECTION("two fee bumps, same fee source, valid individually, combined " "exceed balance") { - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto balanceOfFbAccount = getAvailableBalance( - ledgerView.getLedgerHeader().current(), - ledgerView.getAccount(account2.getPublicKey()).current()); + ledgerView->getLedgerHeader().current(), + ledgerView->getAccount(account2.getPublicKey()).current()); // Enforce balance invariance int64_t fee1 = 200; @@ -765,11 +765,11 @@ testTxSetWithFeeBumps(uint32 protocolVersion) auto fb2 = feeBump(*app, account2, tx2, fee2); // Individual txs are valid auto diagnostics = DiagnosticEventManager::createDisabled(); - REQUIRE(fb1->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb1->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->isSuccess()); - REQUIRE(fb2->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb2->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->isSuccess()); TxFrameList invalidTxs; @@ -818,10 +818,10 @@ testTxSetWithFeeBumps(uint32 protocolVersion) createUploadWasmTx(*app, sorobanAccount2, 100, DEFAULT_TEST_RESOURCE_FEE, resources); - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto balanceOfFbAccount = getAvailableBalance( - ledgerView.getLedgerHeader().current(), - ledgerView.getAccount(feeSourceAccount.getPublicKey()) + ledgerView->getLedgerHeader().current(), + ledgerView->getAccount(feeSourceAccount.getPublicKey()) .current()); // Set fees so that each is valid individually but combined they @@ -840,11 +840,11 @@ testTxSetWithFeeBumps(uint32 protocolVersion) // Individual txs are valid auto diagnostics = DiagnosticEventManager::createDisabled(); - REQUIRE(fb1->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb1->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->isSuccess()); - REQUIRE(fb2->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, - diagnostics) + REQUIRE(fb2->checkValid(app->getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics) ->isSuccess()); PerPhaseTransactionList invalidPerPhase; @@ -912,11 +912,11 @@ testTxSetWithFeeBumps(uint32 protocolVersion) auto diagnostics = DiagnosticEventManager::createDisabled(); REQUIRE(classicFb - ->checkValid(app->getAppConnector(), ledgerView, 0, 0, + ->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0, diagnostics) ->isSuccess()); REQUIRE(sorobanFb - ->checkValid(app->getAppConnector(), ledgerView, 0, 0, + ->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0, diagnostics) ->isSuccess()); @@ -969,10 +969,10 @@ TEST_CASE("getInvalidTxListWithErrors returns no duplicates") auto account3 = root->create("a3", minBalance2); auto account4 = root->create("a4", minBalance2); - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto balanceOfFeeSource = getAvailableBalance( - ledgerView.getLedgerHeader().current(), - ledgerView.getAccount(account2.getPublicKey()).current()); + ledgerView->getLedgerHeader().current(), + ledgerView->getAccount(account2.getPublicKey()).current()); // Create three fee bumps from account2 (fee source): // - fb1: fails checkValid (bad sequence number) @@ -1004,14 +1004,14 @@ TEST_CASE("getInvalidTxListWithErrors returns no duplicates") // Verify fb1 fails checkValid - inner tx has bad sequence number auto diagnostics = DiagnosticEventManager::createDisabled(); - REQUIRE(fb1->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, + REQUIRE(fb1->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0, diagnostics) ->getResultCode() == txFEE_BUMP_INNER_FAILED); // Verify fb2 and fb3 pass checkValid individually - REQUIRE(fb2->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, + REQUIRE(fb2->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0, diagnostics) ->isSuccess()); - REQUIRE(fb3->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0, + REQUIRE(fb3->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0, diagnostics) ->isSuccess()); @@ -5920,9 +5920,8 @@ TEST_CASE("ledger state update flow with parallel apply", "[herder][parallel]") REQUIRE(lm.getLastClosedLedgerNum() == lcl); REQUIRE(lm.getLastClosedLedgerHAS().currentLedger == lastHeader.ledgerSeq); - REQUIRE( - lm.copyImmutableLedgerView().getLedgerHeader().current() == - lastHeader); + REQUIRE(lm.getLCLView()->getLedgerHeader().current() == + lastHeader); // Apply state got committed, but has not yet been propagated to // read-only state @@ -5974,9 +5973,8 @@ TEST_CASE("ledger state update flow with parallel apply", "[herder][parallel]") auto readOnly = lm.getLastClosedLedgerHeader(); REQUIRE(readOnly.header.ledgerSeq == lcl + 1); REQUIRE(lm.getLastClosedLedgerNum() == lcl + 1); - REQUIRE( - lm.copyImmutableLedgerView().getLedgerHeader().current() == - readOnly.header); + REQUIRE(lm.getLCLView()->getLedgerHeader().current() == + readOnly.header); auto has = lm.getLastClosedLedgerHAS(); REQUIRE(has.currentLedger == readOnly.header.ledgerSeq); diff --git a/src/herder/test/TxSetTests.cpp b/src/herder/test/TxSetTests.cpp index 5d2ec0435d..6b95e131ca 100644 --- a/src/herder/test/TxSetTests.cpp +++ b/src/herder/test/TxSetTests.cpp @@ -5,6 +5,7 @@ #include "crypto/SHA.h" #include "herder/ParallelTxSetBuilder.h" #include "herder/TxSetFrame.h" +#include "herder/TxSetUtils.h" #include "herder/test/TestTxSetUtils.h" #include "ledger/LedgerManager.h" #include "ledger/test/LedgerTestUtils.h" @@ -18,6 +19,7 @@ #include "transactions/MutableTransactionResult.h" #include "transactions/TransactionUtils.h" #include "transactions/test/SorobanTxTestUtils.h" +#include "util/BatchExecutor.h" #include "util/Math.h" #include "util/ProtocolVersion.h" #include "util/XDRCereal.h" @@ -1171,8 +1173,8 @@ TEST_CASE("applicable txset validation - transactions belong to correct phase", 1)}, 2000); } - CheckValidLedgerViewWrapper ledgerView(*app); - REQUIRE(tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0) + auto ledgerView = app->getLedgerManager().getLCLView(); + REQUIRE(tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0) ->isSuccess()); return tx; }; @@ -1332,8 +1334,8 @@ TEST_CASE("applicable txset validation - Soroban resources", "[txset][soroban]") auto tx = sorobanTransactionFrameFromOps( app->getNetworkID(), source, {op}, {}, resources, 2000, 100'000'000); - CheckValidLedgerViewWrapper ledgerView(*app); - REQUIRE(tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0) + auto ledgerView = app->getLedgerManager().getLCLView(); + REQUIRE(tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0) ->isSuccess()); return tx; }; @@ -1699,9 +1701,10 @@ TEST_CASE("generalized tx set with multiple txs per source account", // tx1 is valid on its own { - CheckValidLedgerViewWrapper ledgerView(*app); - REQUIRE(tx1->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0) - ->isSuccess()); + auto ledgerView = app->getLedgerManager().getLCLView(); + REQUIRE( + tx1->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0) + ->isSuccess()); } SECTION("build block") @@ -1732,11 +1735,13 @@ TEST_CASE("generalized tx set with multiple txs per source account", // Both txs individually are valid { - CheckValidLedgerViewWrapper ledgerView(*app); - REQUIRE(tx1->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0) - ->isSuccess()); - REQUIRE(tx2->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0) - ->isSuccess()); + auto ledgerView = app->getLedgerManager().getLCLView(); + REQUIRE( + tx1->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0) + ->isSuccess()); + REQUIRE( + tx2->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0) + ->isSuccess()); } SECTION("build block") @@ -1901,7 +1906,8 @@ TEST_CASE("generalized tx set fees", "[txset][soroban]") if (validateTx) { REQUIRE(tx->checkValid(app->getAppConnector(), - CheckValidLedgerViewWrapper(*app), 0, 0, 0) + *app->getLedgerManager().getLCLView(), 0, 0, + 0) ->isSuccess()); } return tx; @@ -2108,7 +2114,8 @@ TEST_CASE("generalized tx set fees", "[txset][soroban]") auto feeBumpTx = feeBump(*app, *root, tx, 300); REQUIRE(feeBumpTx ->checkValid(app->getAppConnector(), - CheckValidLedgerViewWrapper(*app), 0, 0, 0) + *app->getLedgerManager().getLCLView(), 0, + 0, 0) ->isSuccess()); auto ledgerHash = app->getLedgerManager().getLastClosedLedgerHeader().hash; @@ -2147,7 +2154,8 @@ TEST_CASE("generalized tx set fees", "[txset][soroban]") auto feeBumpTx = feeBump(*app, *root, tx, 200); REQUIRE(feeBumpTx ->checkValid(app->getAppConnector(), - CheckValidLedgerViewWrapper(*app), 0, 0, 0) + *app->getLedgerManager().getLCLView(), 0, + 0, 0) ->isSuccess()); auto ledgerHash = app->getLedgerManager().getLastClosedLedgerHeader().hash; @@ -2647,8 +2655,8 @@ runParallelTxSetBuildingTest(bool variableStageCount) // its resources. auto tx = createUploadWasmTx(*app, source, inclusionFee, resourceFee, resources); - CheckValidLedgerViewWrapper ledgerView(*app); - REQUIRE(tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0) + auto ledgerView = app->getLedgerManager().getLCLView(); + REQUIRE(tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0) ->isSuccess()); return tx; @@ -3533,5 +3541,6 @@ TEST_CASE("parallel tx set building benchmark", runBenchmark(50, 50, 5); std::cout << "===" << std::endl; } + } // namespace } // namespace stellar diff --git a/src/herder/test/UpgradesTests.cpp b/src/herder/test/UpgradesTests.cpp index 18bb5d6c95..8ded31fe7b 100644 --- a/src/herder/test/UpgradesTests.cpp +++ b/src/herder/test/UpgradesTests.cpp @@ -339,12 +339,12 @@ testListUpgrades(VirtualClock::system_time_point preferredUpgradeDatetime, makeTxCountUpgrade(cfg.TESTING_UPGRADE_MAX_TX_SET_SIZE); auto baseReserveUpgrade = makeBaseReserveUpgrade(cfg.TESTING_UPGRADE_RESERVE); - auto ledgerView = CheckValidLedgerViewWrapper(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); SECTION("protocol version upgrade needed") { header.ledgerVersion--; - auto upgrades = Upgrades{cfg}.createUpgradesFor(header, ledgerView, + auto upgrades = Upgrades{cfg}.createUpgradesFor(header, *ledgerView, app->getConfig()); auto expected = shouldListAny ? std::vector{protocolVersionUpgrade} @@ -355,7 +355,7 @@ testListUpgrades(VirtualClock::system_time_point preferredUpgradeDatetime, SECTION("base fee upgrade needed") { header.baseFee /= 2; - auto upgrades = Upgrades{cfg}.createUpgradesFor(header, ledgerView, + auto upgrades = Upgrades{cfg}.createUpgradesFor(header, *ledgerView, app->getConfig()); auto expected = shouldListAny ? std::vector{baseFeeUpgrade} @@ -366,7 +366,7 @@ testListUpgrades(VirtualClock::system_time_point preferredUpgradeDatetime, SECTION("tx count upgrade needed") { header.maxTxSetSize /= 2; - auto upgrades = Upgrades{cfg}.createUpgradesFor(header, ledgerView, + auto upgrades = Upgrades{cfg}.createUpgradesFor(header, *ledgerView, app->getConfig()); auto expected = shouldListAny ? std::vector{txCountUpgrade} @@ -377,7 +377,7 @@ testListUpgrades(VirtualClock::system_time_point preferredUpgradeDatetime, SECTION("base reserve upgrade needed") { header.baseReserve /= 2; - auto upgrades = Upgrades{cfg}.createUpgradesFor(header, ledgerView, + auto upgrades = Upgrades{cfg}.createUpgradesFor(header, *ledgerView, app->getConfig()); auto expected = shouldListAny ? std::vector{baseReserveUpgrade} @@ -391,7 +391,7 @@ testListUpgrades(VirtualClock::system_time_point preferredUpgradeDatetime, header.baseFee /= 2; header.maxTxSetSize /= 2; header.baseReserve /= 2; - auto upgrades = Upgrades{cfg}.createUpgradesFor(header, ledgerView, + auto upgrades = Upgrades{cfg}.createUpgradesFor(header, *ledgerView, app->getConfig()); auto expected = shouldListAny @@ -711,7 +711,7 @@ TEST_CASE("config upgrade validation", "[upgrades]") LedgerTxn ltx(app->getLedgerTxnRoot()); ltx.loadHeader().current() = header; - auto ledgerView = CheckValidLedgerViewWrapper(ltx); + LedgerTxnView ledgerView(ltx); LedgerUpgrade outUpgrade; SECTION("valid") { @@ -886,7 +886,7 @@ TEST_CASE("config upgrade validation for protocol 23", "[upgrades]") } LedgerTxn ltx(app->getLedgerTxnRoot()); ltx.loadHeader().current() = header; - auto ledgerView = CheckValidLedgerViewWrapper(ltx); + LedgerTxnView ledgerView(ltx); LedgerUpgrade outUpgrade; return Upgrades::isValidForApply( toUpgradeType(makeConfigUpgrade(*configUpgradeSet)), outUpgrade, @@ -1111,11 +1111,11 @@ TEST_CASE("upgrades affect in-memory Soroban state state size", .getLedgerManager() .getSorobanInMemoryStateSizeForTesting(); auto getExpectedInMemorySize = [&]() { - CheckValidLedgerViewWrapper ledgerView(test.getApp()); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); auto res = expectedInMemorySizeDelta; for (auto const& key : addedKeys) { - auto le = ledgerView.load(key); + auto le = ledgerView->load(key); res += ledgerEntrySizeForRent(le.current(), xdr::xdr_size(le.current()), 23, test.getNetworkCfg()); @@ -1124,11 +1124,11 @@ TEST_CASE("upgrades affect in-memory Soroban state state size", }; auto getStateSizeWindow = [&]() { - CheckValidLedgerViewWrapper ledgerView(test.getApp()); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_LIVE_SOROBAN_STATE_SIZE_WINDOW; - auto le = ledgerView.load(key); + auto le = ledgerView->load(key); REQUIRE(le); std::vector windowFromLtx = le.current().data.configSetting().liveSorobanStateSizeWindow(); @@ -1421,11 +1421,11 @@ TEST_CASE("config upgrades applied to ledger", "[soroban][upgrades]") .liveSorobanStateSizeWindowSampleSize == size); }; auto loadWindow = [&]() { - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_LIVE_SOROBAN_STATE_SIZE_WINDOW; - return ledgerView.load(key) + return ledgerView->load(key) .current() .data.configSetting() .liveSorobanStateSizeWindow(); @@ -2988,8 +2988,8 @@ TEST_CASE("parallel Soroban settings upgrade", "[upgrades]") } { - CheckValidLedgerViewWrapper ledgerView(*app); - REQUIRE(!ledgerView.load(getParallelComputeSettingsLedgerKey())); + auto ledgerView = app->getLedgerManager().getLCLView(); + REQUIRE(!ledgerView->load(getParallelComputeSettingsLedgerKey())); } executeUpgrade(*app, makeProtocolVersionUpgrade(static_cast( @@ -2997,9 +2997,9 @@ TEST_CASE("parallel Soroban settings upgrade", "[upgrades]") // Make sure initial value is correct. { - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto parellelComputeEntry = - ledgerView.load(getParallelComputeSettingsLedgerKey()) + ledgerView->load(getParallelComputeSettingsLedgerKey()) .current() .data.configSetting(); REQUIRE(parellelComputeEntry.configSettingID() == @@ -3024,9 +3024,9 @@ TEST_CASE("parallel Soroban settings upgrade", "[upgrades]") executeUpgrade(*app, makeConfigUpgrade(*configUpgradeSet)); } - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); - REQUIRE(ledgerView.load(getParallelComputeSettingsLedgerKey()) + REQUIRE(ledgerView->load(getParallelComputeSettingsLedgerKey()) .current() .data.configSetting() .contractParallelCompute() @@ -3196,12 +3196,13 @@ TEST_CASE_VERSIONS("upgrade base reserve", "[upgrades]") }); auto submitTx = [&](TransactionTestFramePtr tx) { + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm(true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE( - tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, 0)); REQUIRE(tx->apply(app->getAppConnector(), ltx, txm)); ltx.commit(); @@ -4080,7 +4081,7 @@ TEST_CASE("p24 upgrade fixes corrupted hot archive entries", }; auto runUpgradeAndGetSnapshot = [&]() { executeUpgrade(*app, makeProtocolVersionUpgrade(fixedProtocolVersion)); - return app->getAppConnector().copyImmutableLedgerView(); + return app->getLedgerManager().copyImmutableLedgerView(); }; auto const& corruptedEntries = p23_hot_archive_bug::internal::P23_CORRUPTED_HOT_ARCHIVE_ENTRIES; diff --git a/src/invariant/BucketListStateConsistency.cpp b/src/invariant/BucketListStateConsistency.cpp index 1f83be0884..8c636fba08 100644 --- a/src/invariant/BucketListStateConsistency.cpp +++ b/src/invariant/BucketListStateConsistency.cpp @@ -66,13 +66,14 @@ BucketListStateConsistency::checkSnapshot( std::string errorMsg; // Property 7: Track total entry sizes for validation - auto sorobanConfig = SorobanNetworkConfig::loadFromLedger(applyView); + auto const* sorobanConfig = applyView.getSorobanNetworkConfig(); + releaseAssertOrThrow(sorobanConfig); uint64_t expectedSorobanSize = 0; auto checkLiveEntry = [&seenLiveNonTTLKeys, &seenDeadKeys, &errorMsg, &inMemorySnapshot, &applyView, checkHotArchive, &isStopping, &expectedSorobanSize, &header, - &sorobanConfig](BucketEntry const& be) { + sorobanConfig](BucketEntry const& be) { if (isStopping()) { return Loop::COMPLETE; @@ -130,9 +131,9 @@ BucketListStateConsistency::checkSnapshot( if (lk.type() == CONTRACT_CODE) { uint32_t entryXdrSize = xdr::xdr_size(be.liveEntry()); - expectedSorobanSize += - ledgerEntrySizeForRent(be.liveEntry(), entryXdrSize, - header.ledgerVersion, sorobanConfig); + expectedSorobanSize += ledgerEntrySizeForRent( + be.liveEntry(), entryXdrSize, header.ledgerVersion, + *sorobanConfig); } else { diff --git a/src/ledger/ImmutableLedgerView.cpp b/src/ledger/ImmutableLedgerView.cpp index 3f5da5b909..da876e14cc 100644 --- a/src/ledger/ImmutableLedgerView.cpp +++ b/src/ledger/ImmutableLedgerView.cpp @@ -93,29 +93,50 @@ LedgerHeaderWrapper::current() const } } -LedgerTxnReadOnly::LedgerTxnReadOnly(AbstractLedgerTxn& ltx) : mLedgerTxn(ltx) +LedgerTxnView::LedgerTxnView(AbstractLedgerTxn& ltx, + SorobanNetworkConfig const* sorobanConfig) + : mLedgerTxn(ltx), mSorobanConfig(sorobanConfig) { + if (protocolVersionStartsFrom( + mLedgerTxn.loadHeader().current().ledgerVersion, + SOROBAN_PROTOCOL_VERSION)) + { + releaseAssertOrThrow(sorobanConfig != nullptr); + } } -LedgerTxnReadOnly::~LedgerTxnReadOnly() +LedgerTxnView::LedgerTxnView(AbstractLedgerTxn& ltx) : mLedgerTxn(ltx) { + if (protocolVersionStartsFrom( + mLedgerTxn.loadHeader().current().ledgerVersion, + SOROBAN_PROTOCOL_VERSION)) + { + mLoadedConfig = SorobanNetworkConfig::loadFromLedger(mLedgerTxn); + mSorobanConfig = &*mLoadedConfig; + } } LedgerHeaderWrapper -LedgerTxnReadOnly::getLedgerHeader() const +LedgerTxnView::getLedgerHeader() const { return LedgerHeaderWrapper(mLedgerTxn.loadHeader()); } +SorobanNetworkConfig const* +LedgerTxnView::getSorobanNetworkConfig() const +{ + return mSorobanConfig; +} + LedgerEntryWrapper -LedgerTxnReadOnly::getAccount(AccountID const& account) const +LedgerTxnView::getAccount(AccountID const& account) const { return LedgerEntryWrapper(loadAccountWithoutRecord(mLedgerTxn, account)); } LedgerEntryWrapper -LedgerTxnReadOnly::getAccount(LedgerHeaderWrapper const& header, - TransactionFrame const& tx) const +LedgerTxnView::getAccount(LedgerHeaderWrapper const& header, + TransactionFrame const& tx) const { if (protocolVersionIsBefore(header.current().ledgerVersion, ProtocolVersion::V_8)) @@ -123,102 +144,29 @@ LedgerTxnReadOnly::getAccount(LedgerHeaderWrapper const& header, return LedgerEntryWrapper( tx.loadSourceAccount(mLedgerTxn, header.getLedgerTxnHeader())); } - return getAccount(tx.getSourceID()); } LedgerEntryWrapper -LedgerTxnReadOnly::getAccount(LedgerHeaderWrapper const& header, - TransactionFrame const& tx, - AccountID const& account) const +LedgerTxnView::getAccount(LedgerHeaderWrapper const& header, + TransactionFrame const& tx, + AccountID const& accountID) const { if (protocolVersionIsBefore(header.current().ledgerVersion, ProtocolVersion::V_8)) { return LedgerEntryWrapper( - tx.loadAccount(mLedgerTxn, header.getLedgerTxnHeader(), account)); + tx.loadAccount(mLedgerTxn, header.getLedgerTxnHeader(), accountID)); } - - return getAccount(account); + return getAccount(accountID); } LedgerEntryWrapper -LedgerTxnReadOnly::load(LedgerKey const& key) const +LedgerTxnView::load(LedgerKey const& key) const { return LedgerEntryWrapper(mLedgerTxn.loadWithoutRecord(key)); } -void -LedgerTxnReadOnly::executeWithMaybeInnerSnapshot( - std::function f) const -{ - LedgerTxn inner(mLedgerTxn); - CheckValidLedgerViewWrapper ledgerView(inner); - return f(ledgerView); -} - -CheckValidLedgerViewWrapper::CheckValidLedgerViewWrapper(AbstractLedgerTxn& ltx) - : mGetter(std::make_unique(ltx)) -{ -} - -CheckValidLedgerViewWrapper::CheckValidLedgerViewWrapper(Application& app) -{ - releaseAssert(threadIsMain()); -#ifdef BUILD_TESTS - if (app.getConfig().MODE_USES_IN_MEMORY_LEDGER) - { - // Legacy read-only SQL transaction - mLegacyLedgerTxn = std::make_unique( - app.getLedgerTxnRoot(), /* shouldUpdateLastModified*/ false, - TransactionMode::READ_ONLY_WITHOUT_SQL_TXN); - mGetter = std::make_unique(*mLegacyLedgerTxn); - } - else -#endif - { - mGetter = std::make_unique( - app.getLedgerManager().copyImmutableLedgerView()); - } -} - -CheckValidLedgerViewWrapper::CheckValidLedgerViewWrapper( - ImmutableLedgerView const& ledgerView) - : mGetter(std::make_unique(ledgerView)) -{ -} - -CheckValidLedgerViewWrapper::CheckValidLedgerViewWrapper( - std::unique_ptr getter) - : mGetter(std::move(getter)) -{ -} - -LedgerHeaderWrapper -CheckValidLedgerViewWrapper::getLedgerHeader() const -{ - return mGetter->getLedgerHeader(); -} - -LedgerEntryWrapper -CheckValidLedgerViewWrapper::getAccount(AccountID const& account) const -{ - return mGetter->getAccount(account); -} - -LedgerEntryWrapper -CheckValidLedgerViewWrapper::load(LedgerKey const& key) const -{ - return mGetter->load(key); -} - -void -CheckValidLedgerViewWrapper::executeWithMaybeInnerSnapshot( - std::function f) const -{ - return mGetter->executeWithMaybeInnerSnapshot(f); -} - void ImmutableLedgerData::checkInvariant() const { @@ -241,10 +189,22 @@ ImmutableLedgerData::ImmutableLedgerData( std::make_shared>(metrics)) , mHotArchiveSnapshotMetrics( std::make_shared>(metrics)) - , mSorobanConfig(std::move(sorobanConfig)) , mLastClosedLedgerHeader(lcl) , mLastClosedHistoryArchiveState(has) + , mSorobanConfig(std::move(sorobanConfig)) { + if (!mSorobanConfig.has_value() && + protocolVersionStartsFrom(lcl.header.ledgerVersion, + SOROBAN_PROTOCOL_VERSION)) + { + SearchableLiveBucketListSnapshot liveSnapshot( + metrics, mLiveSnapshotMetrics, mLiveBucketData); + mSorobanConfig.emplace(SorobanNetworkConfig::loadFromLedger( + lcl.header.ledgerVersion, [&liveSnapshot](LedgerKey const& key) { + return LedgerEntryWrapper(liveSnapshot.load(key)); + })); + } + checkInvariant(); } @@ -272,28 +232,6 @@ ImmutableLedgerData::getLastClosedHistoryArchiveState() const return mLastClosedHistoryArchiveState; } -ImmutableLedgerDataPtr -ImmutableLedgerData::createAndMaybeLoadConfig( - LiveBucketList const& liveBL, HotArchiveBucketList const& hotArchiveBL, - LedgerHeaderHistoryEntry const& lcl, HistoryArchiveState const& has, - MetricsRegistry& metrics) -{ - std::optional sorobanConfig; - if (protocolVersionStartsFrom(lcl.header.ledgerVersion, - SOROBAN_PROTOCOL_VERSION)) - { - // Bootstrap: build a lightweight temporary state just to load config - // from the current live bucket list. - auto tempState = std::make_shared( - liveBL, hotArchiveBL, lcl, has, /*sorobanConfig*/ std::nullopt, - metrics); - ImmutableLedgerView tempView(tempState, metrics); - sorobanConfig = SorobanNetworkConfig::loadFromLedger(tempView); - } - return std::make_shared( - liveBL, hotArchiveBL, lcl, has, std::move(sorobanConfig), metrics); -} - ImmutableLedgerView::ImmutableLedgerView(ImmutableLedgerDataPtr state, MetricsRegistry& metrics) : mState(state) @@ -320,6 +258,12 @@ ImmutableLedgerView::getLedgerHeader() const mState, &mState->getLastClosedLedgerHeader().header)); } +SorobanNetworkConfig const* +ImmutableLedgerView::getSorobanNetworkConfig() const +{ + return mState->hasSorobanConfig() ? &mState->getSorobanConfig() : nullptr; +} + uint32_t ImmutableLedgerView::getLedgerSeq() const { @@ -352,16 +296,6 @@ ImmutableLedgerView::load(LedgerKey const& key) const { return LedgerEntryWrapper(loadLiveEntry(key)); } - -void -ImmutableLedgerView::executeWithMaybeInnerSnapshot( - std::function f) const -{ - throw std::runtime_error( - "ImmutableLedgerView::executeWithMaybeInnerSnapshot is illegal: " - "ImmutableLedgerView has no nested snapshots"); -} - SorobanPreApplyLedgerView::SorobanPreApplyLedgerView( std::shared_ptr header, UpdatedEntryGetter getUpdatedEntry, ApplyLedgerView const& lclView) @@ -378,6 +312,12 @@ SorobanPreApplyLedgerView::getLedgerHeader() const return LedgerHeaderWrapper(mHeader); } +SorobanNetworkConfig const* +SorobanPreApplyLedgerView::getSorobanNetworkConfig() const +{ + return mLclView.getSorobanNetworkConfig(); +} + LedgerEntryWrapper SorobanPreApplyLedgerView::getAccount(AccountID const& account) const { @@ -414,14 +354,6 @@ SorobanPreApplyLedgerView::load(LedgerKey const& key) const return LedgerEntryWrapper(mLclView.loadLiveEntry(key)); } -void -SorobanPreApplyLedgerView::executeWithMaybeInnerSnapshot( - std::function f) const -{ - throw std::runtime_error("SorobanPreApplyLedgerView::" - "executeWithMaybeInnerSnapshot is not supported"); -} - // === Live BucketList wrapper methods === std::shared_ptr diff --git a/src/ledger/ImmutableLedgerView.h b/src/ledger/ImmutableLedgerView.h index c253d40f0b..3c8e86e16b 100644 --- a/src/ledger/ImmutableLedgerView.h +++ b/src/ledger/ImmutableLedgerView.h @@ -18,7 +18,6 @@ namespace stellar class Application; class TransactionFrame; -class CheckValidLedgerViewWrapper; class ApplyLedgerView; class ImmutableLedgerData; class EvictionStatistics; @@ -81,6 +80,9 @@ class AbstractLedgerView public: virtual ~AbstractLedgerView() = default; virtual LedgerHeaderWrapper getLedgerHeader() const = 0; + // Returns the pointer to Soroban network config snapshot associated with + // this view, or nullptr when the view doesn't carry one. + virtual SorobanNetworkConfig const* getSorobanNetworkConfig() const = 0; virtual LedgerEntryWrapper getAccount(AccountID const& account) const = 0; virtual LedgerEntryWrapper getAccount(LedgerHeaderWrapper const& header, TransactionFrame const& tx) const = 0; @@ -88,35 +90,37 @@ class AbstractLedgerView TransactionFrame const& tx, AccountID const& AccountID) const = 0; virtual LedgerEntryWrapper load(LedgerKey const& key) const = 0; - // Execute a function with a nested snapshot, if supported. This is needed - // to support the replay of old buggy protocols (<8), see - // `TransactionFrame::loadSourceAccount` - virtual void executeWithMaybeInnerSnapshot( - std::function f) const = 0; }; -// A concrete implementation of read-only SQL snapshot wrapper -class LedgerTxnReadOnly : public AbstractLedgerView +// A read-only view backed by a ledger transaction. +// +// This should only serve as an adapter for the rare cases where LTX has to +// be passed to a function that expects an `AbstractLedgerView`. Prefer using +// other view types, or work with the `LedgerTxn` directly. +class LedgerTxnView : public AbstractLedgerView, public NonMovableOrCopyable { - // Callers are expected to manage `AbstractLedgerTxn` themselves. - // LedgerTxnReadOnly guarantees that LedgerTxn, LedgerTxnHeader and - // LedgerTxnEntry referenced all remain valid. + std::optional mLoadedConfig; AbstractLedgerTxn& mLedgerTxn; + SorobanNetworkConfig const* mSorobanConfig{nullptr}; public: - LedgerTxnReadOnly(AbstractLedgerTxn& ltx); - ~LedgerTxnReadOnly() override; + explicit LedgerTxnView(AbstractLedgerTxn& ltx); + // Takes an externally provided Soroban config (nullptr before the Soroban + // protocol version) instead of loading it from ltx. + // This only exists as optimization to avoid re-loading the config when many + // short-term views are created (e.g. for transaction/operation processing). + LedgerTxnView(AbstractLedgerTxn& ltx, + SorobanNetworkConfig const* sorobanConfig); + ~LedgerTxnView() override = default; LedgerHeaderWrapper getLedgerHeader() const override; + SorobanNetworkConfig const* getSorobanNetworkConfig() const override; LedgerEntryWrapper getAccount(AccountID const& account) const override; LedgerEntryWrapper getAccount(LedgerHeaderWrapper const& header, TransactionFrame const& tx) const override; LedgerEntryWrapper getAccount(LedgerHeaderWrapper const& header, TransactionFrame const& tx, - AccountID const& AccountID) const override; + AccountID const& accountID) const override; LedgerEntryWrapper load(LedgerKey const& key) const override; - void executeWithMaybeInnerSnapshot( - std::function f) - const override; }; // A copyable value type that provides searchable access to a @@ -139,6 +143,7 @@ class ImmutableLedgerView : public virtual AbstractLedgerView ImmutableLedgerData const& getState() const; LedgerHeaderWrapper getLedgerHeader() const override; + SorobanNetworkConfig const* getSorobanNetworkConfig() const override; uint32_t getLedgerSeq() const; // === AbstractLedgerView overrides === @@ -149,9 +154,6 @@ class ImmutableLedgerView : public virtual AbstractLedgerView TransactionFrame const& tx, AccountID const& AccountID) const override; LedgerEntryWrapper load(LedgerKey const& key) const override; - void executeWithMaybeInnerSnapshot( - std::function f) - const override; // === Live BucketList methods === std::shared_ptr loadLiveEntry(LedgerKey const& k) const; @@ -202,10 +204,10 @@ class ApplyLedgerView : private ImmutableLedgerView, explicit ApplyLedgerView(ImmutableLedgerDataPtr state, MetricsRegistry& metrics); - using ImmutableLedgerView::executeWithMaybeInnerSnapshot; using ImmutableLedgerView::getAccount; using ImmutableLedgerView::getLedgerHeader; using ImmutableLedgerView::getLedgerSeq; + using ImmutableLedgerView::getSorobanNetworkConfig; using ImmutableLedgerView::getState; using ImmutableLedgerView::load; using ImmutableLedgerView::loadArchiveEntry; @@ -248,6 +250,7 @@ class SorobanPreApplyLedgerView : public AbstractLedgerView ApplyLedgerView const& lclView); LedgerHeaderWrapper getLedgerHeader() const override; + SorobanNetworkConfig const* getSorobanNetworkConfig() const override; LedgerEntryWrapper getAccount(AccountID const& account) const override; LedgerEntryWrapper getAccount(LedgerHeaderWrapper const& header, TransactionFrame const& tx) const override; @@ -255,9 +258,6 @@ class SorobanPreApplyLedgerView : public AbstractLedgerView TransactionFrame const& tx, AccountID const& accountID) const override; LedgerEntryWrapper load(LedgerKey const& key) const override; - void executeWithMaybeInnerSnapshot( - std::function f) - const override; private: std::shared_ptr mHeader; @@ -265,54 +265,6 @@ class SorobanPreApplyLedgerView : public AbstractLedgerView ApplyLedgerView mLclView; }; -// A helper class to create and query read-only snapshots -// Automatically decides whether to create a BucketList (recommended), or SQL -// snapshot (deprecated, but currently supported) -// NOTE: CheckValidLedgerViewWrapper is meant to be short-lived, and should not -// be persisted across _different_ ledgers, as the state under the hood might -// change. Users are expected to construct a new CheckValidLedgerViewWrapper -// each time they want to query ledger state. -class CheckValidLedgerViewWrapper : public NonMovableOrCopyable -{ - std::unique_ptr mGetter; - std::unique_ptr mLegacyLedgerTxn; - - public: - CheckValidLedgerViewWrapper(AbstractLedgerTxn& ltx); - CheckValidLedgerViewWrapper(Application& app); - explicit CheckValidLedgerViewWrapper(ImmutableLedgerView const& ledgerView); - explicit CheckValidLedgerViewWrapper( - std::unique_ptr getter); -#ifdef BUILD_TESTS - // Set by overlay-only mode call sites so commonValid skips the seqnum - // equality check: on-disk seqnums are frozen at genesis while - // LoadGenerator keeps advancing its local counters, so every tx after the - // first would otherwise fail isBadSeq. - bool mSkipSeqNumCheck{false}; -#endif - LedgerHeaderWrapper getLedgerHeader() const; - LedgerEntryWrapper getAccount(AccountID const& account) const; - LedgerEntryWrapper - getAccount(LedgerHeaderWrapper const& header, - TransactionFrame const& tx) const - { - return mGetter->getAccount(header, tx); - } - LedgerEntryWrapper - getAccount(LedgerHeaderWrapper const& header, TransactionFrame const& tx, - AccountID const& AccountID) const - { - return mGetter->getAccount(header, tx, AccountID); - } - LedgerEntryWrapper load(LedgerKey const& key) const; - - // Execute a function with a nested snapshot, if supported. This is needed - // to support the replay of old buggy protocols (<8), see - // `TransactionFrame::loadSourceAccount` - void executeWithMaybeInnerSnapshot( - std::function f) const; -}; - // Immutable wrapper for a complete ledger state snapshot. // This object provides read-only access to all components of a full ledger // state at a specific ledger sequence. All components are instantiated together @@ -347,9 +299,9 @@ class ImmutableLedgerData : public NonMovableOrCopyable std::shared_ptr const> const mHotArchiveSnapshotMetrics; - std::optional const mSorobanConfig; LedgerHeaderHistoryEntry const mLastClosedLedgerHeader; HistoryArchiveState const mLastClosedHistoryArchiveState; + std::optional mSorobanConfig; void checkInvariant() const; @@ -357,8 +309,8 @@ class ImmutableLedgerData : public NonMovableOrCopyable public: // Construct a new immutable ledger state snapshot. - // sorobanConfig is nullopt for pre-Soroban protocol versions, or when - // building the empty initial state at startup. + // sorobanConfig may be nullopt, in which case the configuration is loaded + // from the live bucket list whenever the protocol version supports Soroban. ImmutableLedgerData(LiveBucketList const& liveBL, HotArchiveBucketList const& hotArchiveBL, LedgerHeaderHistoryEntry const& lcl, @@ -366,13 +318,6 @@ class ImmutableLedgerData : public NonMovableOrCopyable std::optional sorobanConfig, MetricsRegistry& metrics); - // Factory: constructs a ImmutableLedgerData, auto-loading the - // SorobanNetworkConfig from the bucket list when the protocol requires it. - static ImmutableLedgerDataPtr createAndMaybeLoadConfig( - LiveBucketList const& liveBL, HotArchiveBucketList const& hotArchiveBL, - LedgerHeaderHistoryEntry const& lcl, HistoryArchiveState const& has, - MetricsRegistry& metrics); - SorobanNetworkConfig const& getSorobanConfig() const; bool hasSorobanConfig() const; LedgerHeaderHistoryEntry const& getLastClosedLedgerHeader() const; diff --git a/src/ledger/InMemorySorobanState.cpp b/src/ledger/InMemorySorobanState.cpp index cf0e1e690e..83eeb21c70 100644 --- a/src/ledger/InMemorySorobanState.cpp +++ b/src/ledger/InMemorySorobanState.cpp @@ -455,7 +455,8 @@ InMemorySorobanState::initializeStateFromSnapshot( auto ledgerVersion = lclHeader.ledgerVersion; if (protocolVersionStartsFrom(ledgerVersion, SOROBAN_PROTOCOL_VERSION)) { - auto sorobanConfig = SorobanNetworkConfig::loadFromLedger(applyView); + auto const* sorobanConfig = applyView.getSorobanNetworkConfig(); + releaseAssertOrThrow(sorobanConfig); auto contractDataHandler = [this](LedgerEntry const& le, LedgerKey const&) { createContractDataEntry(le); @@ -465,10 +466,10 @@ InMemorySorobanState::initializeStateFromSnapshot( createTTL(le); }; - auto contractCodeHandler = [this, &sorobanConfig, + auto contractCodeHandler = [this, sorobanConfig, ledgerVersion](LedgerEntry const& le, LedgerKey const&) { - createContractCodeEntry(le, sorobanConfig, ledgerVersion); + createContractCodeEntry(le, *sorobanConfig, ledgerVersion); }; applyView.scanCurrentLiveEntriesOfType(CONTRACT_DATA, diff --git a/src/ledger/LedgerManager.h b/src/ledger/LedgerManager.h index a5127e8c61..4c6dea760d 100644 --- a/src/ledger/LedgerManager.h +++ b/src/ledger/LedgerManager.h @@ -250,22 +250,46 @@ class LedgerManager virtual LedgerHeaderHistoryEntry const& getLastClosedLedgerHeader() const = 0; - // Create a thread-safe copy of the current canonical ledger state - // snapshot. Can be called from any thread (except for apply, which must use - // copyApplyLedgerView instead). + // Scans the last closed ledger's live BucketList for entries of a given + // type. Note this iterates over all BucketEntries, so some may be shadowed + // and outdated. + virtual void scanLiveEntriesOfType( + LedgerEntryType type, + std::function callback) const = 0; + +#ifdef BUILD_TESTS + // Returns the concrete view backing `getLCLView`, which exposes the batch + // loading and Hot Archive functionality. + // At the moment this should only be necessary for the tests, as the + // production code-paths should only care about that functionality at apply + // time, which uses the concrete `ApplyLedgerView` implementation. virtual ImmutableLedgerView copyImmutableLedgerView() const = 0; +#endif - // Create a thread-safe copy of the current canonical ledger state - // snapshot, typed as an apply-time snapshot. Used by legacy (pre-V23) - // code paths that need an ApplyLedgerView but don't have - // access to ApplyState. + // Create a copy of the canonical last closed ledger snapshot. + // The returned snapshot is *not* thread-safe (albeit being `const`), so + // every thread must hold their own instance of the snapshot. + // `getLCLView` itself is thread-safe and can be called from multiple + // threads, except the apply thread, which must use `copyApplyLedgerView` + // instead. + virtual std::unique_ptr getLCLView() const = 0; + + // Create a copy of the canonical ledger snapshot at the current moment in + // apply flow. + // The returned snapshot is *not* thread-safe, so every thread used during + // transaction application must hold their own copy of the snapshot. + // `copyApplyLedgerView` itself is thread-safe and can be called from + // multiple threads. + // Used by legacy (pre-V23) code paths that need an ApplyLedgerView but + // don't have access to ApplyState to copy the it from. // TODO: Refactor such that this doesn't have to be a public function virtual ApplyLedgerView copyApplyLedgerView() const = 0; - // Refresh `ledgerView` if its ledger seq differs from the current canonical - // state. No-op otherwise. Can be called from any thread. - virtual void - maybeUpdateImmutableLedgerView(ImmutableLedgerView& ledgerView) const = 0; + // Makes the provided `ledgerView` match the current canonical LCL view. + // No-op when views have a matching ledger sequence number. + // Can be called from any thread. + virtual void syncWithLCLView( + std::unique_ptr& ledgerView) const = 0; // return the HAS that corresponds to the last closed ledger as persisted in // the database diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp index dc0e798543..495d207798 100644 --- a/src/ledger/LedgerManagerImpl.cpp +++ b/src/ledger/LedgerManagerImpl.cpp @@ -1875,7 +1875,10 @@ LedgerManagerImpl::applyLedger(LedgerCloseData const& ledgerData, for (size_t i = 0; i < sv.upgrades.size(); i++) { LedgerUpgrade lupgrade; - CheckValidLedgerViewWrapper ledgerView(ltx); + // Subtle: we re-create a ledger view for every upgrade here, as the + // invariant behind the ledger view is that it's immutable. Upgrades + // mutate the LTX, so we need a fresh view for each upgrade. + LedgerTxnView ledgerView(ltx); auto valid = Upgrades::isValidForApply(sv.upgrades[i], lupgrade, mApp, ledgerView); switch (valid) @@ -2327,18 +2330,9 @@ LedgerManagerImpl::buildLedgerState( lcl.header = header; lcl.hash = xdrSha256(header); - if (sorobanConfig) - { - // Caller already loaded config (e.g. from LTX during ledger close) - return std::make_shared( - bm.getLiveBucketList(), bm.getHotArchiveBucketList(), lcl, has, - std::move(sorobanConfig), mApp.getMetrics()); - } - - // Auto-load SorobanNetworkConfig from the BucketList - return ImmutableLedgerData::createAndMaybeLoadConfig( + return std::make_shared( bm.getLiveBucketList(), bm.getHotArchiveBucketList(), lcl, has, - mApp.getMetrics()); + std::move(sorobanConfig), mApp.getMetrics()); } ImmutableLedgerDataPtr @@ -2351,6 +2345,18 @@ LedgerManagerImpl::advanceApplySnapshotAndMakeLedgerState( return state; } +void +LedgerManagerImpl::scanLiveEntriesOfType( + LedgerEntryType type, + std::function callback) const +{ +#ifdef BUILD_TESTS + // The BucketList isn't populated in the in-memory ledger mode. + releaseAssert(!mApp.getConfig().MODE_USES_IN_MEMORY_LEDGER); +#endif + copyImmutableLedgerView().scanLiveEntriesOfType(type, std::move(callback)); +} + ImmutableLedgerView LedgerManagerImpl::copyImmutableLedgerView() const { @@ -2364,6 +2370,83 @@ LedgerManagerImpl::copyImmutableLedgerView() const return ImmutableLedgerView(mLastClosedLedgerState, mApp.getMetrics()); } +#ifdef BUILD_TESTS +namespace +{ +// A read-only view that owns the LedgerTxn it reads from. Only needed for the +// in-memory ledger mode, where the state lives in a never-committing LedgerTxn +// rather than in the BucketList, so a view has to hold a child LedgerTxn of it +// alive for as long as it is used. +class OwnedLedgerTxnView : public AbstractLedgerView, + public NonMovableOrCopyable +{ + // Declared first so that it outlives the view reading from it. + LedgerTxn mLedgerTxn; + LedgerTxnView mView; + + public: + explicit OwnedLedgerTxnView(AbstractLedgerTxnParent& root) + : mLedgerTxn(root, /* shouldUpdateLastModified */ false, + TransactionMode::READ_ONLY_WITHOUT_SQL_TXN) + , mView(mLedgerTxn) + { + } + + LedgerHeaderWrapper + getLedgerHeader() const override + { + return mView.getLedgerHeader(); + } + + SorobanNetworkConfig const* + getSorobanNetworkConfig() const override + { + return mView.getSorobanNetworkConfig(); + } + + LedgerEntryWrapper + getAccount(AccountID const& account) const override + { + return mView.getAccount(account); + } + + LedgerEntryWrapper + getAccount(LedgerHeaderWrapper const& header, + TransactionFrame const& tx) const override + { + return mView.getAccount(header, tx); + } + + LedgerEntryWrapper + getAccount(LedgerHeaderWrapper const& header, TransactionFrame const& tx, + AccountID const& accountID) const override + { + return mView.getAccount(header, tx, accountID); + } + + LedgerEntryWrapper + load(LedgerKey const& key) const override + { + return mView.load(key); + } +}; +} +#endif + +std::unique_ptr +LedgerManagerImpl::getLCLView() const +{ +#ifdef BUILD_TESTS + if (mApp.getConfig().MODE_USES_IN_MEMORY_LEDGER) + { + // The BucketList isn't populated in this mode, so the state can only + // be read through the never-committing LedgerTxn. + return std::make_unique(mApp.getLedgerTxnRoot()); + } +#endif + return std::make_unique(copyImmutableLedgerView()); +} + ApplyLedgerView LedgerManagerImpl::copyApplyLedgerView() const { @@ -2374,19 +2457,20 @@ LedgerManagerImpl::copyApplyLedgerView() const } void -LedgerManagerImpl::maybeUpdateImmutableLedgerView( - ImmutableLedgerView& ledgerView) const +LedgerManagerImpl::syncWithLCLView( + std::unique_ptr& ledgerView) const { JITTER_INJECT_DELAY(); SharedLockShared guard(mLastClosedLedgerStateMutex); JITTER_INJECT_DELAY(); releaseAssert(mLastClosedLedgerState); - if (ledgerView.getLedgerSeq() != - mLastClosedLedgerState->getLastClosedLedgerHeader().header.ledgerSeq) + if (!ledgerView || ledgerView->getLedgerHeader().current().ledgerSeq != + mLastClosedLedgerState->getLastClosedLedgerHeader() + .header.ledgerSeq) { - ledgerView = - ImmutableLedgerView(mLastClosedLedgerState, mApp.getMetrics()); + ledgerView = std::make_unique( + mLastClosedLedgerState, mApp.getMetrics()); } } @@ -3211,11 +3295,11 @@ LedgerManagerImpl::finalizeLedgerTxnChanges( { bool hotArchiveBatchedAdded = false; + auto sorobanConfig = SorobanNetworkConfig::loadFromLedger(ltx); // In `getAllTTLKeysWithoutSealing` it is important not to seal ltx, // because it is still being modified by the eviction flow. // `getAllTTLKeysWithoutSealing` must be called at the right time // _after_ all operations have been applied, but _before_ evictions. - auto sorobanConfig = SorobanNetworkConfig::loadFromLedger(ltx); evictedState = mApp.getBucketManager().resolveBackgroundEvictionScan( lclApplyView, ltx, ltx.getAllKeysWithoutSealing()); diff --git a/src/ledger/LedgerManagerImpl.h b/src/ledger/LedgerManagerImpl.h index 197c81f4a6..022628473a 100644 --- a/src/ledger/LedgerManagerImpl.h +++ b/src/ledger/LedgerManagerImpl.h @@ -493,6 +493,13 @@ class LedgerManagerImpl : public LedgerManager // have to actually be closed. void loadLastKnownLedgerInternal(bool skipBuildingFullState); +#ifndef BUILD_TESTS + ImmutableLedgerView copyImmutableLedgerView() const; +#else + public: + ImmutableLedgerView copyImmutableLedgerView() const override; +#endif + protected: // initialLedgerVers must be the ledger version at the start of the ledger // and currLedgerVers is the ledger version in the current ltx header. These @@ -613,10 +620,15 @@ class LedgerManagerImpl : public LedgerManager void maybeResetLedgerCloseMetaDebugStream(uint32_t ledgerSeq); SorobanMetrics& getSorobanMetrics() override; - ImmutableLedgerView copyImmutableLedgerView() const override; + void scanLiveEntriesOfType( + LedgerEntryType type, + std::function callback) const override; + + public: + std::unique_ptr getLCLView() const override; ApplyLedgerView copyApplyLedgerView() const override; - void maybeUpdateImmutableLedgerView( - ImmutableLedgerView& ledgerView) const override; + void syncWithLCLView( + std::unique_ptr& ledgerView) const override; #ifdef BUILD_TESTS void updateCanonicalStateForTesting(LedgerHeader const& header) override; #endif diff --git a/src/ledger/NetworkConfig.cpp b/src/ledger/NetworkConfig.cpp index 4966eebda0..0b2de8cc3b 100644 --- a/src/ledger/NetworkConfig.cpp +++ b/src/ledger/NetworkConfig.cpp @@ -1763,36 +1763,36 @@ SorobanNetworkConfig::initializeGenesisLedgerForTesting( } SorobanNetworkConfig -SorobanNetworkConfig::loadFromLedger(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadFromLedger(uint32_t protocolVersion, + LedgerEntryReader const& loadEntry) { ZoneScoped; SorobanNetworkConfig config; - config.loadMaxContractSize(ls); - config.loadMaxContractDataKeySize(ls); - config.loadMaxContractDataEntrySize(ls); - config.loadComputeSettings(ls); - config.loadLedgerAccessSettings(ls); - config.loadHistoricalSettings(ls); - config.loadContractEventsSettings(ls); - config.loadBandwidthSettings(ls); - config.loadCpuCostParams(ls); - config.loadMemCostParams(ls); - config.loadStateArchivalSettings(ls); - config.loadExecutionLanesSettings(ls); - config.loadLiveSorobanStateSizeWindow(ls); - config.loadEvictionIterator(ls); - - auto protocolVersion = ls.getLedgerHeader().current().ledgerVersion; + config.loadMaxContractSize(loadEntry); + config.loadMaxContractDataKeySize(loadEntry); + config.loadMaxContractDataEntrySize(loadEntry); + config.loadComputeSettings(loadEntry); + config.loadLedgerAccessSettings(protocolVersion, loadEntry); + config.loadHistoricalSettings(loadEntry); + config.loadContractEventsSettings(loadEntry); + config.loadBandwidthSettings(loadEntry); + config.loadCpuCostParams(loadEntry); + config.loadMemCostParams(loadEntry); + config.loadStateArchivalSettings(loadEntry); + config.loadExecutionLanesSettings(loadEntry); + config.loadLiveSorobanStateSizeWindow(loadEntry); + config.loadEvictionIterator(loadEntry); + if (protocolVersionStartsFrom(protocolVersion, ProtocolVersion::V_23)) { - config.loadParallelComputeConfig(ls); - config.loadLedgerCostExtConfig(ls); - config.loadSCPTimingConfig(ls); + config.loadParallelComputeConfig(loadEntry); + config.loadLedgerCostExtConfig(loadEntry); + config.loadSCPTimingConfig(loadEntry); } if (protocolVersionStartsFrom(protocolVersion, ProtocolVersion::V_26)) { - config.loadFrozenLedgerKeys(ls); - config.loadFreezeBypassTxs(ls); + config.loadFrozenLedgerKeys(loadEntry); + config.loadFreezeBypassTxs(loadEntry); } // NB: this should follow loading/updating state size window // size and state archival settings @@ -1803,8 +1803,10 @@ SorobanNetworkConfig::loadFromLedger(AbstractLedgerView const& ls) SorobanNetworkConfig SorobanNetworkConfig::loadFromLedger(AbstractLedgerTxn& ltx) { - LedgerTxnReadOnly snap(ltx); - return SorobanNetworkConfig::loadFromLedger(snap); + return SorobanNetworkConfig::loadFromLedger( + ltx.loadHeader().current().ledgerVersion, [<x](LedgerKey const& key) { + return LedgerEntryWrapper(ltx.loadWithoutRecord(key)); + }); } #ifdef BUILD_TESTS @@ -1816,28 +1818,29 @@ SorobanNetworkConfig::emptyConfig() #endif void -SorobanNetworkConfig::loadMaxContractSize(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadMaxContractSize(LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_MAX_SIZE_BYTES; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); mMaxContractSizeBytes = le.data.configSetting().contractMaxSizeBytes(); } void -SorobanNetworkConfig::loadMaxContractDataKeySize(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadMaxContractDataKeySize( + LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_DATA_KEY_SIZE_BYTES; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); mMaxContractDataKeySizeBytes = @@ -1845,14 +1848,15 @@ SorobanNetworkConfig::loadMaxContractDataKeySize(AbstractLedgerView const& ls) } void -SorobanNetworkConfig::loadMaxContractDataEntrySize(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadMaxContractDataEntrySize( + LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_DATA_ENTRY_SIZE_BYTES; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); mMaxContractDataEntrySizeBytes = @@ -1860,14 +1864,14 @@ SorobanNetworkConfig::loadMaxContractDataEntrySize(AbstractLedgerView const& ls) } void -SorobanNetworkConfig::loadComputeSettings(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadComputeSettings(LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_COMPUTE_V0; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); auto const& configSetting = le.data.configSetting().contractCompute(); @@ -1879,14 +1883,15 @@ SorobanNetworkConfig::loadComputeSettings(AbstractLedgerView const& ls) } void -SorobanNetworkConfig::loadLedgerAccessSettings(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadLedgerAccessSettings( + uint32_t protocolVersion, LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_LEDGER_COST_V0; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); auto const& configSetting = le.data.configSetting().contractLedgerCost(); @@ -1909,13 +1914,12 @@ SorobanNetworkConfig::loadLedgerAccessSettings(AbstractLedgerView const& ls) mSorobanStateRentFeeGrowthFactor = configSetting.sorobanStateRentFeeGrowthFactor; - if (protocolVersionStartsFrom(ls.getLedgerHeader().current().ledgerVersion, - ProtocolVersion::V_23)) + if (protocolVersionStartsFrom(protocolVersion, ProtocolVersion::V_23)) { LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_LEDGER_COST_EXT_V0; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); auto const& configSetting = @@ -1926,14 +1930,14 @@ SorobanNetworkConfig::loadLedgerAccessSettings(AbstractLedgerView const& ls) } void -SorobanNetworkConfig::loadHistoricalSettings(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadHistoricalSettings(LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_HISTORICAL_DATA_V0; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); auto const& configSetting = @@ -1942,14 +1946,15 @@ SorobanNetworkConfig::loadHistoricalSettings(AbstractLedgerView const& ls) } void -SorobanNetworkConfig::loadContractEventsSettings(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadContractEventsSettings( + LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_EVENTS_V0; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); auto const& configSetting = le.data.configSetting().contractEvents(); @@ -1958,14 +1963,14 @@ SorobanNetworkConfig::loadContractEventsSettings(AbstractLedgerView const& ls) } void -SorobanNetworkConfig::loadBandwidthSettings(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadBandwidthSettings(LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_BANDWIDTH_V0; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); auto const& configSetting = le.data.configSetting().contractBandwidth(); @@ -1975,42 +1980,43 @@ SorobanNetworkConfig::loadBandwidthSettings(AbstractLedgerView const& ls) } void -SorobanNetworkConfig::loadCpuCostParams(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadCpuCostParams(LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_COST_PARAMS_CPU_INSTRUCTIONS; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); mCpuCostParams = le.data.configSetting().contractCostParamsCpuInsns(); } void -SorobanNetworkConfig::loadMemCostParams(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadMemCostParams(LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_COST_PARAMS_MEMORY_BYTES; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); mMemCostParams = le.data.configSetting().contractCostParamsMemBytes(); } void -SorobanNetworkConfig::loadExecutionLanesSettings(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadExecutionLanesSettings( + LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_EXECUTION_LANES; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); auto const& configSetting = @@ -2020,14 +2026,14 @@ SorobanNetworkConfig::loadExecutionLanesSettings(AbstractLedgerView const& ls) void SorobanNetworkConfig::loadLiveSorobanStateSizeWindow( - AbstractLedgerView const& ls) + LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_LIVE_SOROBAN_STATE_SIZE_WINDOW; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& window = lsle.current().data.configSetting().liveSorobanStateSizeWindow(); @@ -2050,26 +2056,27 @@ SorobanNetworkConfig::loadLiveSorobanStateSizeWindow( } void -SorobanNetworkConfig::loadEvictionIterator(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadEvictionIterator(LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_EVICTION_ITERATOR; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); mEvictionIterator = lsle.current().data.configSetting().evictionIterator(); } void -SorobanNetworkConfig::loadParallelComputeConfig(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadParallelComputeConfig( + LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_PARALLEL_COMPUTE_V0; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); auto const& configSetting = @@ -2078,13 +2085,14 @@ SorobanNetworkConfig::loadParallelComputeConfig(AbstractLedgerView const& ls) } void -SorobanNetworkConfig::loadLedgerCostExtConfig(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadLedgerCostExtConfig( + LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_LEDGER_COST_EXT_V0; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); auto const& configSetting = le.data.configSetting().contractLedgerCostExt(); @@ -2093,13 +2101,13 @@ SorobanNetworkConfig::loadLedgerCostExtConfig(AbstractLedgerView const& ls) } void -SorobanNetworkConfig::loadSCPTimingConfig(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadSCPTimingConfig(LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_SCP_TIMING; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); auto const& configSetting = le.data.configSetting().contractSCPTiming(); @@ -2134,14 +2142,15 @@ SorobanNetworkConfig::maxContractDataEntrySizeBytes() const } void -SorobanNetworkConfig::loadStateArchivalSettings(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadStateArchivalSettings( + LedgerEntryReader const& loadEntry) { ZoneScoped; LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_STATE_ARCHIVAL; - auto lsle = ls.load(key); + auto lsle = loadEntry(key); releaseAssertOrThrow(lsle); auto const& le = lsle.current(); mStateArchivalSettings = le.data.configSetting().stateArchivalSettings(); @@ -2542,14 +2551,14 @@ SorobanNetworkConfig::isFreezeBypassTx(Hash const& txHash) const } void -SorobanNetworkConfig::loadFrozenLedgerKeys(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadFrozenLedgerKeys(LedgerEntryReader const& loadEntry) { ZoneScoped; mFrozenLedgerKeys.clear(); LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = CONFIG_SETTING_FROZEN_LEDGER_KEYS; - auto le = ls.load(key); + auto le = loadEntry(key); releaseAssertOrThrow(le); auto const& frozenKeys = @@ -2563,14 +2572,14 @@ SorobanNetworkConfig::loadFrozenLedgerKeys(AbstractLedgerView const& ls) } void -SorobanNetworkConfig::loadFreezeBypassTxs(AbstractLedgerView const& ls) +SorobanNetworkConfig::loadFreezeBypassTxs(LedgerEntryReader const& loadEntry) { ZoneScoped; mFreezeBypassTxs.clear(); LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = CONFIG_SETTING_FREEZE_BYPASS_TXS; - auto le = ls.load(key); + auto le = loadEntry(key); releaseAssertOrThrow(le); auto const& txHashes = diff --git a/src/ledger/NetworkConfig.h b/src/ledger/NetworkConfig.h index 1d76b89e56..a5f4785cfe 100644 --- a/src/ledger/NetworkConfig.h +++ b/src/ledger/NetworkConfig.h @@ -10,12 +10,13 @@ #include "util/TxResource.h" #include #include +#include namespace stellar { -class AbstractLedgerView; class Application; +class LedgerEntryWrapper; // Defines the minimum values allowed for the network configuration // settings during upgrades. An upgrade that does not follow the minimums @@ -260,8 +261,13 @@ struct TestOverrideSorobanNetworkConfig class SorobanNetworkConfig { public: - // Static factory function to create a SorobanNetworkConfig from ledger - static SorobanNetworkConfig loadFromLedger(AbstractLedgerView const& snap); + using LedgerEntryReader = + std::function; + + // Static factory function to create a SorobanNetworkConfig from ledger. + static SorobanNetworkConfig + loadFromLedger(uint32_t protocolVersion, + LedgerEntryReader const& loadEntry); static SorobanNetworkConfig loadFromLedger(AbstractLedgerTxn& ltx); #ifdef BUILD_TESTS @@ -479,25 +485,26 @@ class SorobanNetworkConfig private: SorobanNetworkConfig() = default; - void loadMaxContractSize(AbstractLedgerView const& ls); - void loadMaxContractDataKeySize(AbstractLedgerView const& ls); - void loadMaxContractDataEntrySize(AbstractLedgerView const& ls); - void loadComputeSettings(AbstractLedgerView const& ls); - void loadLedgerAccessSettings(AbstractLedgerView const& ls); - void loadHistoricalSettings(AbstractLedgerView const& ls); - void loadContractEventsSettings(AbstractLedgerView const& ls); - void loadBandwidthSettings(AbstractLedgerView const& ls); - void loadCpuCostParams(AbstractLedgerView const& ls); - void loadMemCostParams(AbstractLedgerView const& ls); - void loadStateArchivalSettings(AbstractLedgerView const& ls); - void loadExecutionLanesSettings(AbstractLedgerView const& ls); - void loadLiveSorobanStateSizeWindow(AbstractLedgerView const& ls); - void loadEvictionIterator(AbstractLedgerView const& ls); - void loadParallelComputeConfig(AbstractLedgerView const& ls); - void loadLedgerCostExtConfig(AbstractLedgerView const& ls); - void loadSCPTimingConfig(AbstractLedgerView const& ls); - void loadFrozenLedgerKeys(AbstractLedgerView const& ls); - void loadFreezeBypassTxs(AbstractLedgerView const& ls); + void loadMaxContractSize(LedgerEntryReader const& loadEntry); + void loadMaxContractDataKeySize(LedgerEntryReader const& loadEntry); + void loadMaxContractDataEntrySize(LedgerEntryReader const& loadEntry); + void loadComputeSettings(LedgerEntryReader const& loadEntry); + void loadLedgerAccessSettings(uint32_t protocolVersion, + LedgerEntryReader const& loadEntry); + void loadHistoricalSettings(LedgerEntryReader const& loadEntry); + void loadContractEventsSettings(LedgerEntryReader const& loadEntry); + void loadBandwidthSettings(LedgerEntryReader const& loadEntry); + void loadCpuCostParams(LedgerEntryReader const& loadEntry); + void loadMemCostParams(LedgerEntryReader const& loadEntry); + void loadStateArchivalSettings(LedgerEntryReader const& loadEntry); + void loadExecutionLanesSettings(LedgerEntryReader const& loadEntry); + void loadLiveSorobanStateSizeWindow(LedgerEntryReader const& loadEntry); + void loadEvictionIterator(LedgerEntryReader const& loadEntry); + void loadParallelComputeConfig(LedgerEntryReader const& loadEntry); + void loadLedgerCostExtConfig(LedgerEntryReader const& loadEntry); + void loadSCPTimingConfig(LedgerEntryReader const& loadEntry); + void loadFrozenLedgerKeys(LedgerEntryReader const& loadEntry); + void loadFreezeBypassTxs(LedgerEntryReader const& loadEntry); void computeRentWriteFee(uint32_t protocolVersion); #ifdef BUILD_TESTS diff --git a/src/ledger/test/ImmutableLedgerViewTests.cpp b/src/ledger/test/ImmutableLedgerViewTests.cpp index 68f5cbb696..2bcf6b0d4f 100644 --- a/src/ledger/test/ImmutableLedgerViewTests.cpp +++ b/src/ledger/test/ImmutableLedgerViewTests.cpp @@ -213,15 +213,16 @@ struct ThreadGroup class SnapshotThread { mutable ANNOTATED_SHARED_MUTEX(mMutex); - ImmutableLedgerView mLedgerView GUARDED_BY(mMutex); + std::unique_ptr mLedgerView GUARDED_BY(mMutex); // Updated only by mutation methods; a mismatch with - // mLedgerView.getLedgerSeq() indicates a race or corruption. + // `mLedgerView` ledger sequence number indicates a race or corruption. uint32_t mExpectedSeq; public: - explicit SnapshotThread(ImmutableLedgerView ledgerView) + explicit SnapshotThread( + std::unique_ptr ledgerView) : mLedgerView(std::move(ledgerView)) - , mExpectedSeq(mLedgerView.getLedgerSeq()) + , mExpectedSeq(mLedgerView->getLedgerHeader().current().ledgerSeq) { } @@ -236,19 +237,28 @@ class SnapshotThread ImmutableLedgerView const& ledgerView() const NO_THREAD_SAFETY_ANALYSIS { - return mLedgerView; + // Downcast the value to the concrete ImmutableLedgerView type that + // backs the abstract ledger view. + // We're intentionally doing a cast here instead of using test-only + // `copyImmutableLedgerView` function in order to exercise the real + // production code paths that use `getLCLView()`. + auto const* concrete = + dynamic_cast(mLedgerView.get()); + releaseAssert(concrete); + return *concrete; } bool seqMatchesExpected() const NO_THREAD_SAFETY_ANALYSIS { - return mLedgerView.getLedgerSeq() == mExpectedSeq; + return mLedgerView->getLedgerHeader().current().ledgerSeq == + mExpectedSeq; } bool headerMatchesExpected() const NO_THREAD_SAFETY_ANALYSIS { - return mLedgerView.getLedgerHeader().current().ledgerSeq == + return mLedgerView->getLedgerHeader().current().ledgerSeq == mExpectedSeq; } @@ -259,8 +269,8 @@ class SnapshotThread maybeUpdate(LedgerManager const& lm) { SharedLockExclusive lock(mMutex); - lm.maybeUpdateImmutableLedgerView(mLedgerView); - auto newSeq = mLedgerView.getLedgerSeq(); + lm.syncWithLCLView(mLedgerView); + auto newSeq = mLedgerView->getLedgerHeader().current().ledgerSeq; bool ok = newSeq >= mExpectedSeq; mExpectedSeq = newSeq; return ok; @@ -271,8 +281,8 @@ class SnapshotThread freshCopy(LedgerManager const& lm) { SharedLockExclusive lock(mMutex); - mLedgerView = lm.copyImmutableLedgerView(); - auto newSeq = mLedgerView.getLedgerSeq(); + mLedgerView = lm.getLCLView(); + auto newSeq = mLedgerView->getLedgerHeader().current().ledgerSeq; bool ok = newSeq >= mExpectedSeq; mExpectedSeq = newSeq; return ok; @@ -283,7 +293,7 @@ class SnapshotThread copySnapshot() const { SharedLockShared lock(mMutex); - return mLedgerView; + return ledgerView(); } // Replace with a snapshot copied from a peer. The peer may be @@ -292,8 +302,9 @@ class SnapshotThread replaceWith(ImmutableLedgerView ledgerView) { SharedLockExclusive lock(mMutex); - mLedgerView = std::move(ledgerView); - mExpectedSeq = mLedgerView.getLedgerSeq(); + mLedgerView = + std::make_unique(std::move(ledgerView)); + mExpectedSeq = mLedgerView->getLedgerHeader().current().ledgerSeq; } }; @@ -416,7 +427,7 @@ SnapshotStressTest::SnapshotStressTest(int numThreads, unsigned seed, for (int i = 0; i < mNumThreads; i++) { mThreads.push_back(std::make_unique( - mApp.getLedgerManager().copyImmutableLedgerView())); + mApp.getLedgerManager().getLCLView())); } } @@ -899,14 +910,14 @@ SnapshotStressTest::closeLedgers() // `ledgerView` with matching data. // --------------------------------------------------------------------------- void -requireEntries(ImmutableLedgerView& ledgerView, +requireEntries(AbstractLedgerView const& ledgerView, std::vector const& entries) { for (auto const& entry : entries) { - auto loaded = ledgerView.loadLiveEntry(LedgerEntryKey(entry)); + auto loaded = ledgerView.load(LedgerEntryKey(entry)); REQUIRE(loaded); - CHECK(*loaded == entry); + CHECK(loaded.current() == entry); } } @@ -948,37 +959,35 @@ TEST_CASE("basic snapshot copy semantics and isolation", "[snapshot]") // Add first batch, take snapshot S1. addLiveBatchAndUpdateSnapshot(*app, makeHeader(seq1, protocolVersion), entries1, {}, {}); - auto s1 = lm.copyImmutableLedgerView(); - REQUIRE(s1.getLedgerSeq() == seq1); + auto s1 = lm.getLCLView(); + auto s2 = lm.getLCLView(); + REQUIRE(s1->getLedgerHeader().current().ledgerSeq == seq1); + REQUIRE(s2->getLedgerHeader().current().ledgerSeq == seq1); // Advance to seq2 with new entries. addLiveBatchAndUpdateSnapshot(*app, makeHeader(seq2, protocolVersion), entries2, {}, {}); - // Copy-construct S2 from S1 (after state advanced — tests isolation). - auto s2 = s1; - REQUIRE(s2.getLedgerSeq() == seq1); - // S1 and S2 return identical data for entries1. - requireEntries(s1, entries1); - requireEntries(s2, entries1); + requireEntries(*s1, entries1); + requireEntries(*s2, entries1); // S1 and S2 must not see entries from seq2. for (auto const& entry : entries2) { - CHECK(s1.loadLiveEntry(LedgerEntryKey(entry)) == nullptr); - CHECK(s2.loadLiveEntry(LedgerEntryKey(entry)) == nullptr); + CHECK(!s1->load(LedgerEntryKey(entry))); + CHECK(!s2->load(LedgerEntryKey(entry))); } - // maybeUpdate S1: should refresh it to seq2 (jump +1). - lm.maybeUpdateImmutableLedgerView(s1); - REQUIRE(s1.getLedgerSeq() == seq2); - requireEntries(s1, entries1); - requireEntries(s1, entries2); + // Sync S1 with LCL: should refresh it to seq2 (jump +1). + lm.syncWithLCLView(s1); + REQUIRE(s1->getLedgerHeader().current().ledgerSeq == seq2); + requireEntries(*s1, entries1); + requireEntries(*s1, entries2); // maybeUpdate again with no new ledger close: no-op. - lm.maybeUpdateImmutableLedgerView(s1); - REQUIRE(s1.getLedgerSeq() == seq2); + lm.syncWithLCLView(s1); + REQUIRE(s1->getLedgerHeader().current().ledgerSeq == seq2); // Advance to seq3. uint32_t seq3 = startSeq + 3; @@ -993,18 +1002,20 @@ TEST_CASE("basic snapshot copy semantics and isolation", "[snapshot]") entries3, {}, {}); // maybeUpdate S2: still at seq1, jumps +2 (seq1 -> seq3). - REQUIRE(s2.getLedgerSeq() == seq1); - lm.maybeUpdateImmutableLedgerView(s2); - REQUIRE(s2.getLedgerSeq() == seq3); - requireEntries(s2, entries1); - requireEntries(s2, entries2); - requireEntries(s2, entries3); + REQUIRE(s2->getLedgerHeader().current().ledgerSeq == seq1); + lm.syncWithLCLView(s2); + REQUIRE(s2->getLedgerHeader().current().ledgerSeq == seq3); + requireEntries(*s2, entries1); + requireEntries(*s2, entries2); + requireEntries(*s2, entries3); // maybeUpdate S1: at seq2, jumps +1 (seq2 -> seq3). - REQUIRE(s1.getLedgerSeq() == seq2); - lm.maybeUpdateImmutableLedgerView(s1); - REQUIRE(s1.getLedgerSeq() == seq3); - requireEntries(s1, entries3); + REQUIRE(s1->getLedgerHeader().current().ledgerSeq == seq2); + lm.syncWithLCLView(s1); + REQUIRE(s1->getLedgerHeader().current().ledgerSeq == seq3); + requireEntries(*s1, entries1); + requireEntries(*s1, entries2); + requireEntries(*s1, entries3); } // --------------------------------------------------------------------------- diff --git a/src/main/AppConnector.cpp b/src/main/AppConnector.cpp index b03c4283bd..afa08bad57 100644 --- a/src/main/AppConnector.cpp +++ b/src/main/AppConnector.cpp @@ -175,25 +175,13 @@ AppConnector::getBatchExecutor() return mApp.getBatchExecutor(); } -ImmutableLedgerView -AppConnector::copyImmutableLedgerView() -{ - return mApp.getLedgerManager().copyImmutableLedgerView(); -} - ApplyLedgerView AppConnector::copyApplyLedgerView() { return mApp.getLedgerManager().copyApplyLedgerView(); } -void -AppConnector::maybeUpdateImmutableLedgerView(ImmutableLedgerView& ledgerView) -{ - mApp.getLedgerManager().maybeUpdateImmutableLedgerView(ledgerView); -} - -ImmutableLedgerView& +AbstractLedgerView const& AppConnector::getOverlayThreadSnapshot() { return mApp.getOverlayManager().getOverlayThreadSnapshot(); diff --git a/src/main/AppConnector.h b/src/main/AppConnector.h index cf3bb124de..2f3065d057 100644 --- a/src/main/AppConnector.h +++ b/src/main/AppConnector.h @@ -75,13 +75,11 @@ class AppConnector bool isStopping() const; - ImmutableLedgerView copyImmutableLedgerView(); ApplyLedgerView copyApplyLedgerView(); - void maybeUpdateImmutableLedgerView(ImmutableLedgerView& ledgerView); // Get a snapshot of ledger state for use by the overlay thread only. Must // only be called from the overlay thread. - ImmutableLedgerView& getOverlayThreadSnapshot(); + AbstractLedgerView const& getOverlayThreadSnapshot(); // Protocol 23 data corruption bug data verifier. This typically is null, // unless a path to a CSV file containing the corruption data was provided diff --git a/src/main/ApplicationImpl.cpp b/src/main/ApplicationImpl.cpp index 2d4b082997..a070c9fba8 100644 --- a/src/main/ApplicationImpl.cpp +++ b/src/main/ApplicationImpl.cpp @@ -34,6 +34,7 @@ #include "invariant/LedgerEntryIsValid.h" #include "invariant/LiabilitiesMatchOffers.h" #include "invariant/SponsorshipCountIsValid.h" +#include "ledger/ImmutableLedgerView.h" #include "ledger/LedgerHeaderUtils.h" #include "ledger/LedgerManager.h" #include "ledger/LedgerManagerImpl.h" diff --git a/src/main/ApplicationImpl.h b/src/main/ApplicationImpl.h index 9432c5457c..4e8ad10906 100644 --- a/src/main/ApplicationImpl.h +++ b/src/main/ApplicationImpl.h @@ -11,6 +11,7 @@ #include "util/MetricResetter.h" #include "util/Timer.h" #include "xdr/Stellar-ledger-entries.h" +#include #include #include @@ -251,7 +252,9 @@ class ApplicationImpl : public Application std::atomic mStopping; #ifdef BUILD_TESTS - bool mRunInOverlayOnlyMode; + // Read from the parallel transaction validation threads while the + // `toggleoverlayonlymode` command may flip it from the main thread. + std::atomic mRunInOverlayOnlyMode; #endif VirtualTimer mStoppingTimer; diff --git a/src/main/ApplicationUtils.cpp b/src/main/ApplicationUtils.cpp index 66f9702304..36e994033a 100644 --- a/src/main/ApplicationUtils.cpp +++ b/src/main/ApplicationUtils.cpp @@ -730,7 +730,7 @@ dumpLedger(Config cfg, std::string const& outputFile, auto& lm = app->getLedgerManager(); lm.partiallyLoadLastKnownLedgerForUtils(); - auto liveLedgerView = app->getAppConnector().copyImmutableLedgerView(); + auto liveLedgerView = app->getLedgerManager().getLCLView(); auto ttlGetter = [&liveLedgerView, includeAllStates, dumpHotArchive](LedgerKey const& key) -> uint32_t { if (includeAllStates || dumpHotArchive) @@ -739,13 +739,13 @@ dumpLedger(Config cfg, std::string const& outputFile, "TTL is undefined when `--include-all-states` or " "`--hot-archive` flag is set."); } - auto entry = liveLedgerView.loadLiveEntry(key); + auto entry = liveLedgerView->load(key); if (!entry) { throw std::runtime_error("No TTL entry found for key: " + xdrToCerealString(key, "key")); } - return entry->data.ttl().liveUntilLedgerSeq; + return entry.current().data.ttl().liveUntilLedgerSeq; }; HistoryArchiveState has = lm.getLastClosedLedgerHAS(); @@ -869,10 +869,10 @@ dumpWasmBlob(Config cfg, std::string const& hash, std::string const& dir) LOG_INFO(DEFAULT_LOG, "Wrote {} bytes to {}", entry.code.size(), filename); }; - auto ledgerView = app->getLedgerManager().copyImmutableLedgerView(); + auto& lmForScan = app->getLedgerManager(); if (hash == "ALL") { - ledgerView.scanLiveEntriesOfType( + lmForScan.scanLiveEntriesOfType( CONTRACT_CODE, [&](BucketEntry const& entry) { if (entry.type() == INITENTRY || entry.type() == LIVEENTRY) { @@ -888,10 +888,11 @@ dumpWasmBlob(Config cfg, std::string const& hash, std::string const& dir) LedgerKey key; key.type(LedgerEntryType::CONTRACT_CODE); key.contractCode().hash = hexToBin256(hash); - auto entry = ledgerView.loadLiveEntry(key); - if (entry && entry->data.type() == LedgerEntryType::CONTRACT_CODE) + auto entry = lmForScan.getLCLView()->load(key); + if (entry && + entry.current().data.type() == LedgerEntryType::CONTRACT_CODE) { - auto const& codeEntry = entry->data.contractCode(); + auto const& codeEntry = entry.current().data.contractCode(); writeBlob(codeEntry); } else diff --git a/src/main/CommandHandler.cpp b/src/main/CommandHandler.cpp index 8d99441728..6f3c484d34 100644 --- a/src/main/CommandHandler.cpp +++ b/src/main/CommandHandler.cpp @@ -657,9 +657,9 @@ CommandHandler::upgrades(std::string const& params, std::string& retStr) decoder::decode_b64(configXdrIter->second, buffer); ConfigUpgradeSetKey key; xdr::xdr_from_opaque(buffer, key); - auto ledgerView = CheckValidLedgerViewWrapper(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); - auto ptr = ConfigUpgradeSetFrame::makeFromKey(ledgerView, key); + auto ptr = ConfigUpgradeSetFrame::makeFromKey(*ledgerView, key); if (!ptr || ptr->isValidForApply(mApp.getConfig()) != Upgrades::UpgradeValidity::VALID) @@ -727,9 +727,9 @@ CommandHandler::dumpProposedSettings(std::string const& params, decoder::decode_b64(blob, buffer); ConfigUpgradeSetKey key; xdr::xdr_from_opaque(buffer, key); - auto ledgerView = CheckValidLedgerViewWrapper(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); - auto ptr = ConfigUpgradeSetFrame::makeFromKey(ledgerView, key); + auto ptr = ConfigUpgradeSetFrame::makeFromKey(*ledgerView, key); if (!ptr || ptr->isValidForApply(mApp.getConfig()) != Upgrades::UpgradeValidity::VALID) @@ -959,11 +959,11 @@ CommandHandler::sorobanInfo(std::string const& params, std::string& retStr) } else if (format == "detailed") { - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); xdr::xvector entries; for (auto c : xdr::xdr_traits::enum_values()) { - auto entry = ledgerView.load( + auto entry = ledgerView->load( configSettingKey(static_cast(c))); if (!entry) { @@ -976,7 +976,7 @@ CommandHandler::sorobanInfo(std::string const& params, std::string& retStr) } else if (format == "upgrade_xdr") { - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); ConfigUpgradeSet upgradeSet; for (auto c : xdr::xdr_traits::enum_values()) @@ -987,7 +987,8 @@ CommandHandler::sorobanInfo(std::string const& params, std::string& retStr) { continue; } - auto entry = ledgerView.load(configSettingKey(configSettingID)); + auto entry = + ledgerView->load(configSettingKey(configSettingID)); if (!entry) { continue; @@ -1455,8 +1456,8 @@ CommandHandler::testAcc(std::string const& params, std::string& retStr) key = getAccount(accName->second.c_str()); } - CheckValidLedgerViewWrapper ledgerView(mApp); - auto acc = ledgerView.load(accountKey(key.getPublicKey())); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + auto acc = ledgerView->load(accountKey(key.getPublicKey())); if (acc) { auto const& ae = acc.current().data.account(); diff --git a/src/main/test/CommandHandlerTests.cpp b/src/main/test/CommandHandlerTests.cpp index 30f228d8dd..7eb61ed43e 100644 --- a/src/main/test/CommandHandlerTests.cpp +++ b/src/main/test/CommandHandlerTests.cpp @@ -513,9 +513,9 @@ TEST_CASE("manualclose", "[commandhandler]") txFrame->addSignature(*root); { - LedgerTxn checkLtx(app->getLedgerTxnRoot()); auto valid = txFrame->checkValidForTesting( - app->getAppConnector(), checkLtx, 0, 0, 0); + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); REQUIRE(valid); } diff --git a/src/overlay/OverlayManager.h b/src/overlay/OverlayManager.h index a3f9bc1b6a..26240a1585 100644 --- a/src/overlay/OverlayManager.h +++ b/src/overlay/OverlayManager.h @@ -213,8 +213,8 @@ class OverlayManager virtual bool checkScheduledAndCache(std::shared_ptr tracker) = 0; - // Get a snapshot of ledger state for use by the overlay thread only. Caller - // is responsible for updating the snapshot as needed. - virtual ImmutableLedgerView& getOverlayThreadSnapshot() = 0; + // Get a snapshot of ledger state for use by the overlay thread only. + // The snapshot is guaranteed to be synchronized with the LCL. + virtual AbstractLedgerView const& getOverlayThreadSnapshot() = 0; }; } diff --git a/src/overlay/OverlayManagerImpl.cpp b/src/overlay/OverlayManagerImpl.cpp index 578ff82f46..eb240fecdc 100644 --- a/src/overlay/OverlayManagerImpl.cpp +++ b/src/overlay/OverlayManagerImpl.cpp @@ -1432,17 +1432,14 @@ OverlayManagerImpl::recordMessageMetric(StellarMessage const& stellarMsg, } } -ImmutableLedgerView& +AbstractLedgerView const& OverlayManagerImpl::getOverlayThreadSnapshot() { releaseAssert(mApp.threadIsType(Application::ThreadType::OVERLAY)); JITTER_INJECT_DELAY(); - if (!mOverlayThreadSnapshot) - { - // Create a new snapshot - mOverlayThreadSnapshot = - mApp.getLedgerManager().copyImmutableLedgerView(); - } + // Creates the snapshot on the first call, and refreshes it whenever the + // last closed ledger has moved on. + mApp.getLedgerManager().syncWithLCLView(mOverlayThreadSnapshot); return *mOverlayThreadSnapshot; } diff --git a/src/overlay/OverlayManagerImpl.h b/src/overlay/OverlayManagerImpl.h index 1bbec29c4d..9858e89a82 100644 --- a/src/overlay/OverlayManagerImpl.h +++ b/src/overlay/OverlayManagerImpl.h @@ -168,7 +168,7 @@ class OverlayManagerImpl : public OverlayManager void recordMessageMetric(StellarMessage const& stellarMsg, Peer::pointer peer) override; - ImmutableLedgerView& getOverlayThreadSnapshot() override; + AbstractLedgerView const& getOverlayThreadSnapshot() override; private: struct ResolvedPeers @@ -186,7 +186,7 @@ class OverlayManagerImpl : public OverlayManager mScheduledMessages; // Snapshot of ledger state for use ONLY by the overlay thread - std::optional mOverlayThreadSnapshot; + std::unique_ptr mOverlayThreadSnapshot; void triggerPeerResolution(); std::pair, bool> diff --git a/src/overlay/Peer.cpp b/src/overlay/Peer.cpp index 860efe1b00..4f66893a01 100644 --- a/src/overlay/Peer.cpp +++ b/src/overlay/Peer.cpp @@ -71,19 +71,17 @@ populateSignatureCache(AppConnector& app, TransactionFrameBaseConstPtr tx) app.threadIsType(Application::ThreadType::OVERLAY)); auto& overlayView = app.getOverlayThreadSnapshot(); - app.maybeUpdateImmutableLedgerView(overlayView); - CheckValidLedgerViewWrapper ledgerView(overlayView); - // Use ledgerView to check all transactions in `tx`. We use a lambda to + // Use overlayView to check all transactions in `tx`. We use a lambda to // simplify checking of both outer and inner transactions in the case of fee // bumps. - auto const checkTxSignatures = [&ledgerView]( + auto const checkTxSignatures = [&overlayView]( TransactionFrameBaseConstPtr tx) { auto const& hash = tx->getContentsHash(); auto const& signatures = txbridge::getSignatures(tx->getEnvelope()); SignatureChecker signatureChecker( - ledgerView.getLedgerHeader().current().ledgerVersion, hash, + overlayView.getLedgerHeader().current().ledgerVersion, hash, signatures, true); // Do not report signature cache metrics during background validation. @@ -94,7 +92,7 @@ populateSignatureCache(AppConnector& app, TransactionFrameBaseConstPtr tx) // NOTE: Use getFeeSourceID so that this works for both TransactionFrame // and FeeBumpTransactionFrame - auto const sourceAccount = ledgerView.getAccount(tx->getFeeSourceID()); + auto const sourceAccount = overlayView.getAccount(tx->getFeeSourceID()); if (!sourceAccount) { @@ -115,10 +113,10 @@ populateSignatureCache(AppConnector& app, TransactionFrameBaseConstPtr tx) // Check all transaction signatures tx->checkAllTransactionSignatures( signatureChecker, sourceAccount, - ledgerView.getLedgerHeader().current().ledgerVersion); + overlayView.getLedgerHeader().current().ledgerVersion); // Check all operation signatures. - tx->checkOperationSignatures(signatureChecker, ledgerView, nullptr); + tx->checkOperationSignatures(signatureChecker, overlayView, nullptr); }; checkTxSignatures(tx); diff --git a/src/overlay/test/OverlayTests.cpp b/src/overlay/test/OverlayTests.cpp index 079be7b8cb..53dddf9e26 100644 --- a/src/overlay/test/OverlayTests.cpp +++ b/src/overlay/test/OverlayTests.cpp @@ -3469,8 +3469,8 @@ TEST_CASE("populateSignatureCache tests", "[overlay]") REQUIRE(misses == 1); // Call to checkValid should experience only cache hits - LedgerTxn ltx(app->getLedgerTxnRoot()); - tx->checkValid(app->getAppConnector(), ltx, 0, 0, 0); + tx->checkValid(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); PubKeyUtils::flushVerifySigCacheCounts(hits, misses); REQUIRE(hits > 0); REQUIRE(misses == 0); @@ -3493,8 +3493,8 @@ TEST_CASE("populateSignatureCache tests", "[overlay]") REQUIRE(misses == 2); // Call to checkValid should experience only cache hits - LedgerTxn ltx(app->getLedgerTxnRoot()); - feeBumpTx->checkValid(app->getAppConnector(), ltx, 0, 0, 0); + feeBumpTx->checkValid(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); PubKeyUtils::flushVerifySigCacheCounts(hits, misses); REQUIRE(hits > 0); REQUIRE(misses == 0); @@ -3583,11 +3583,10 @@ TEST_CASE("populateSignatureCache tests", "[overlay]") // checkValid now sees the cache already populated: both tx-level // and op-level signed-payload lookups are pure cache hits. - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto ls = CheckValidLedgerViewWrapper(ltx); + auto ls = app->getLedgerManager().getLCLView(); auto diagnostics = DiagnosticEventManager::createDisabled(); - auto result = - payTx->checkValid(app->getAppConnector(), ls, 0, 0, 0, diagnostics); + auto result = payTx->checkValid(app->getAppConnector(), *ls, 0, 0, 0, + diagnostics); REQUIRE(result->isSuccess()); PubKeyUtils::flushVerifySigCacheCounts(hits, misses); @@ -3638,16 +3637,15 @@ TEST_CASE("populateSignatureCache tests", "[overlay]") txtest::applyTx(removeSignerTx, *app); // Now check that the cached transaction is invalid due to bad auth - LedgerTxn ltx(app->getLedgerTxnRoot()); + auto ledgerView = app->getLedgerManager().getLCLView(); bool isValid = paymentTx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0); + *ledgerView, 0, 0, 0); REQUIRE(!isValid); // Verify it fails with bad auth, not other reasons - auto ledgerView = CheckValidLedgerViewWrapper(ltx); auto diagnostics = DiagnosticEventManager::createDisabled(); - auto result = paymentTx->checkValid(app->getAppConnector(), ledgerView, + auto result = paymentTx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0, diagnostics); REQUIRE(result->getResultCode() == txBAD_AUTH); diff --git a/src/simulation/ApplyLoad.cpp b/src/simulation/ApplyLoad.cpp index d57b7da66b..b287ad1d56 100644 --- a/src/simulation/ApplyLoad.cpp +++ b/src/simulation/ApplyLoad.cpp @@ -1165,11 +1165,11 @@ ApplyLoad::applyConfigUpgrade(SorobanUpgradeConfig const& upgradeConfig) upgradeBytes, mUpgradeCodeKey, mUpgradeInstanceKey, std::nullopt, resources); { - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); auto diagnostics = DiagnosticEventManager::createForValidation(mApp.getConfig()); auto validationRes = invokeTx->checkValid( - mApp.getAppConnector(), ledgerView, 0, 0, 0, diagnostics); + mApp.getAppConnector(), *ledgerView, 0, 0, 0, diagnostics); if (!validationRes->isSuccess()) { if (validationRes->getResultCode() == txSOROBAN_INVALID) @@ -1680,14 +1680,14 @@ ApplyLoad::benchmarkLimitsIteration() stellar::shuffle(std::begin(shuffledAccounts), std::end(shuffledAccounts), getGlobalRandomEngine()); - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); auto appConnector = mApp.getAppConnector(); auto addTx = [&ledgerView, &appConnector, &txs](TransactionFrameBasePtr tx) { auto diagnostics = DiagnosticEventManager::createDisabled(); auto res = - tx->checkValid(appConnector, ledgerView, 0, 0, 0, diagnostics); + tx->checkValid(appConnector, *ledgerView, 0, 0, 0, diagnostics); releaseAssert(res && res->isSuccess()); txs.emplace_back(tx); }; @@ -2002,7 +2002,7 @@ ApplyLoad::generateClassicPayments(std::vector& txs, releaseAssert(accounts.size() >= startAccountIdx + classicTxCount()); - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); auto appConnector = mApp.getAppConnector(); auto diagnostics = DiagnosticEventManager::createDisabled(); @@ -2021,7 +2021,7 @@ ApplyLoad::generateClassicPayments(std::vector& txs, mNumAccounts, 0, lm.getLastClosedLedgerNum() + 1, it->first, 1, std::nullopt, memo); auto res = - tx->checkValid(appConnector, ledgerView, 0, 0, 0, diagnostics); + tx->checkValid(appConnector, *ledgerView, 0, 0, 0, diagnostics); releaseAssert(res && res->isSuccess()); txs.emplace_back(tx); } @@ -2103,7 +2103,7 @@ ApplyLoad::generateSacPayments(std::vector& txs, txs.push_back(tx.second); } } - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); auto diag = DiagnosticEventManager::createDisabled(); // Validate all the generated transactions. This serves 2 purposes: // - ensure that the tx generator works as expected @@ -2115,7 +2115,7 @@ ApplyLoad::generateSacPayments(std::vector& txs, for (auto const& tx : txs) { releaseAssert( - tx->checkValid(mApp.getAppConnector(), ledgerView, 0, 0, 0, diag) + tx->checkValid(mApp.getAppConnector(), *ledgerView, 0, 0, 0, diag) ->isSuccess()); } } @@ -2304,12 +2304,12 @@ ApplyLoad::generateTokenTransfers(std::vector& txs, txs.push_back(tx.second); } - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); auto diag = DiagnosticEventManager::createDisabled(); for (auto const& tx : txs) { releaseAssert( - tx->checkValid(mApp.getAppConnector(), ledgerView, 0, 0, 0, diag) + tx->checkValid(mApp.getAppConnector(), *ledgerView, 0, 0, 0, diag) ->isSuccess()); } } @@ -3069,11 +3069,11 @@ ApplyLoad::generateSoroswapSwaps(std::vector& txs, txs.push_back(tx.second); } - CheckValidLedgerViewWrapper ls(mApp); + auto ls = mApp.getLedgerManager().getLCLView(); auto diag = DiagnosticEventManager::createDisabled(); for (auto const& tx : txs) { - releaseAssert(tx->checkValid(mApp.getAppConnector(), ls, 0, 0, 0, diag) + releaseAssert(tx->checkValid(mApp.getAppConnector(), *ls, 0, 0, 0, diag) ->isSuccess()); } } diff --git a/src/simulation/LoadGenerator.cpp b/src/simulation/LoadGenerator.cpp index 7ea9efc28e..bccdc6e3e9 100644 --- a/src/simulation/LoadGenerator.cpp +++ b/src/simulation/LoadGenerator.cpp @@ -1268,16 +1268,16 @@ LoadGenerator::checkSorobanStateSynced(Application& app, } std::vector result; - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); for (auto const& lk : mContractInstanceKeys) { - if (!ledgerView.load(lk)) + if (!ledgerView->load(lk)) { result.emplace_back(lk); } } - if (mCodeKey && !ledgerView.load(*mCodeKey)) + if (mCodeKey && !ledgerView->load(*mCodeKey)) { result.emplace_back(*mCodeKey); } diff --git a/src/simulation/TxGenerator.cpp b/src/simulation/TxGenerator.cpp index 374011fec9..0664fcc5da 100644 --- a/src/simulation/TxGenerator.cpp +++ b/src/simulation/TxGenerator.cpp @@ -50,11 +50,11 @@ sampleDiscrete(std::vector const& values, uint64_t footprintSize(Application& app, xdr::xvector const& keys) { - CheckValidLedgerViewWrapper ledgerView(app); + auto ledgerView = app.getLedgerManager().getLCLView(); uint64_t total = 0; for (auto const& key : keys) { - auto entry = ledgerView.load(key); + auto entry = ledgerView->load(key); if (entry) { total += xdr::xdr_size(entry.current()); @@ -88,8 +88,8 @@ TxGenerator::updateMinBalance() bool TxGenerator::isLive(LedgerKey const& lk, uint32_t ledgerNum) const { - CheckValidLedgerViewWrapper ledgerView(mApp); - auto ttlEntryPtr = ledgerView.load(getTTLKey(lk)); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + auto ttlEntryPtr = ledgerView->load(getTTLKey(lk)); return ttlEntryPtr && stellar::isLive(ttlEntryPtr.current(), ledgerNum); } @@ -129,8 +129,8 @@ TxGenerator::generateFee(std::optional maxGeneratedFeeRate, bool TxGenerator::loadAccount(TestAccount& account) { - CheckValidLedgerViewWrapper ledgerView(mApp); - auto const entry = ledgerView.getAccount(account.getPublicKey()); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + auto const entry = ledgerView->getAccount(account.getPublicKey()); if (!entry) { return false; @@ -1180,7 +1180,7 @@ TxGenerator::getConfigUpgradeSetFromLoadConfig( { xdr::xvector updatedEntries; - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); for (auto t : xdr::xdr_traits::enum_values()) { auto type = static_cast(t); @@ -1219,7 +1219,7 @@ TxGenerator::getConfigUpgradeSetFromLoadConfig( continue; } - auto entryPtr = ledgerView.load(configSettingKey(type)); + auto entryPtr = ledgerView->load(configSettingKey(type)); // This could happen if we have not yet upgraded if ((t == CONFIG_SETTING_CONTRACT_PARALLEL_COMPUTE_V0 || t == CONFIG_SETTING_CONTRACT_LEDGER_COST_EXT_V0 || diff --git a/src/test/TestAccount.cpp b/src/test/TestAccount.cpp index 2cfd7912c8..5523ec5750 100644 --- a/src/test/TestAccount.cpp +++ b/src/test/TestAccount.cpp @@ -33,8 +33,8 @@ TestAccount::updateSequenceNumber() { if (mSn == 0) { - CheckValidLedgerViewWrapper ledgerView(mApp); - auto const entry = ledgerView.load(accountKey(getPublicKey())); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + auto const entry = ledgerView->load(accountKey(getPublicKey())); if (entry) { mSn = entry.current().data.account().seqNum; @@ -45,8 +45,8 @@ TestAccount::updateSequenceNumber() uint32_t TestAccount::getTrustlineFlags(Asset const& asset) const { - CheckValidLedgerViewWrapper ledgerView(mApp); - auto const trust = ledgerView.load(trustlineKey(getPublicKey(), asset)); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + auto const trust = ledgerView->load(trustlineKey(getPublicKey(), asset)); REQUIRE(trust); return trust.current().data.trustLine().flags; } @@ -63,10 +63,11 @@ TestAccount::getTrustlineBalance(Asset const& asset) const int64_t TestAccount::getTrustlineBalance(PoolID const& poolID) const { - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); TrustLineAsset asset(ASSET_TYPE_POOL_SHARE); asset.liquidityPoolID() = poolID; - auto const trustLine = ledgerView.load(trustlineKey(getPublicKey(), asset)); + auto const trustLine = + ledgerView->load(trustlineKey(getPublicKey(), asset)); REQUIRE(trustLine); return trustLine.current().data.trustLine().balance; } @@ -74,25 +75,25 @@ TestAccount::getTrustlineBalance(PoolID const& poolID) const int64_t TestAccount::getBalance() const { - CheckValidLedgerViewWrapper ledgerView(mApp); - auto const entry = ledgerView.getAccount(getPublicKey()); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + auto const entry = ledgerView->getAccount(getPublicKey()); return entry.current().data.account().balance; } int64_t TestAccount::getAvailableBalance() const { - CheckValidLedgerViewWrapper ledgerView(mApp); - auto const entry = ledgerView.getAccount(getPublicKey()); - return stellar::getAvailableBalance(ledgerView.getLedgerHeader().current(), + auto ledgerView = mApp.getLedgerManager().getLCLView(); + auto const entry = ledgerView->getAccount(getPublicKey()); + return stellar::getAvailableBalance(ledgerView->getLedgerHeader().current(), entry.current()); } uint32_t TestAccount::getNumSubEntries() const { - CheckValidLedgerViewWrapper ledgerView(mApp); - auto const entry = ledgerView.getAccount(getPublicKey()); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + auto const entry = ledgerView->getAccount(getPublicKey()); return entry.current().data.account().numSubEntries; } @@ -146,8 +147,8 @@ TestAccount::create(SecretKey const& secretKey, uint64_t initialBalance) std::unique_ptr destBefore; { - CheckValidLedgerViewWrapper ledgerView(mApp); - auto const entry = ledgerView.getAccount(publicKey); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + auto const entry = ledgerView->getAccount(publicKey); if (entry) { destBefore = std::make_unique(entry.current()); @@ -160,8 +161,8 @@ TestAccount::create(SecretKey const& secretKey, uint64_t initialBalance) } catch (...) { - CheckValidLedgerViewWrapper ledgerView(mApp); - auto const destAfter = ledgerView.getAccount(publicKey); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + auto const destAfter = ledgerView->getAccount(publicKey); // check that the target account didn't change REQUIRE(!!destBefore == !!destAfter); if (destBefore && destAfter) @@ -172,8 +173,8 @@ TestAccount::create(SecretKey const& secretKey, uint64_t initialBalance) } { - CheckValidLedgerViewWrapper ledgerView(mApp); - REQUIRE(ledgerView.getAccount(publicKey)); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + REQUIRE(ledgerView->getAccount(publicKey)); } return TestAccount{mApp, secretKey}; } @@ -190,10 +191,10 @@ TestAccount::createBatch(std::vector const& secretKeys, } applyOpsBatch(ops); std::vector accounts; - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); for (auto const& secretKey : secretKeys) { - REQUIRE(ledgerView.getAccount(secretKey.getPublicKey())); + REQUIRE(ledgerView->getAccount(secretKey.getPublicKey())); accounts.emplace_back(mApp, secretKey); } return accounts; @@ -222,9 +223,9 @@ TestAccount::merge(PublicKey const& into) { applyTx(tx({accountMerge(into)}), mApp); - CheckValidLedgerViewWrapper ledgerView(mApp); - REQUIRE(ledgerView.getAccount(into)); - REQUIRE(!ledgerView.getAccount(getPublicKey())); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + REQUIRE(ledgerView->getAccount(into)); + REQUIRE(!ledgerView->getAccount(getPublicKey())); } void @@ -339,11 +340,11 @@ TestAccount::loadTrustLine(Asset const& asset) const TrustLineEntry TestAccount::loadTrustLine(TrustLineAsset const& asset) const { - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); LedgerKey key(TRUSTLINE); key.trustLine().accountID = getPublicKey(); key.trustLine().asset = asset; - return ledgerView.load(key).current().data.trustLine(); + return ledgerView->load(key).current().data.trustLine(); } bool @@ -355,11 +356,11 @@ TestAccount::hasTrustLine(Asset const& asset) const bool TestAccount::hasTrustLine(TrustLineAsset const& asset) const { - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); LedgerKey key(TRUSTLINE); key.trustLine().accountID = getPublicKey(); key.trustLine().asset = asset; - return static_cast(ledgerView.load(key)); + return static_cast(ledgerView->load(key)); } void @@ -391,20 +392,20 @@ TestAccount::bumpSequence(SequenceNumber to) { applyTx(tx({txtest::bumpSequence(to)}), mApp, false); - CheckValidLedgerViewWrapper ledgerView(mApp); + auto ledgerView = mApp.getLedgerManager().getLCLView(); if (protocolVersionStartsFrom( - ledgerView.getLedgerHeader().current().ledgerVersion, + ledgerView->getLedgerHeader().current().ledgerVersion, ProtocolVersion::V_19)) { - auto const account = ledgerView.getAccount(getPublicKey()); + auto const account = ledgerView->getAccount(getPublicKey()); REQUIRE(account); auto const& v3 = getAccountEntryExtensionV3(account.current().data.account()); REQUIRE(v3.seqLedger == - ledgerView.getLedgerHeader().current().ledgerSeq); + ledgerView->getLedgerHeader().current().ledgerSeq); REQUIRE(v3.seqTime == - ledgerView.getLedgerHeader().current().scpValue.closeTime); + ledgerView->getLedgerHeader().current().scpValue.closeTime); } } @@ -504,8 +505,8 @@ TestAccount::pay(PublicKey const& destination, int64_t amount) { std::unique_ptr toAccount; { - CheckValidLedgerViewWrapper ledgerView(mApp); - auto const toAccountEntry = ledgerView.getAccount(destination); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + auto const toAccountEntry = ledgerView->getAccount(destination); toAccount = toAccountEntry ? std::make_unique(toAccountEntry.current()) @@ -516,7 +517,7 @@ TestAccount::pay(PublicKey const& destination, int64_t amount) } else { - REQUIRE(ledgerView.getAccount(getPublicKey())); + REQUIRE(ledgerView->getAccount(getPublicKey())); } } @@ -528,8 +529,8 @@ TestAccount::pay(PublicKey const& destination, int64_t amount) } catch (...) { - CheckValidLedgerViewWrapper ledgerView(mApp); - auto const toAccountAfter = ledgerView.getAccount(destination); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + auto const toAccountAfter = ledgerView->getAccount(destination); // check that the target account didn't change REQUIRE(!!toAccount == !!toAccountAfter); if (toAccount && toAccountAfter && @@ -541,8 +542,8 @@ TestAccount::pay(PublicKey const& destination, int64_t amount) throw; } - CheckValidLedgerViewWrapper ledgerView(mApp); - auto const toAccountAfter = ledgerView.getAccount(destination); + auto ledgerView = mApp.getLedgerManager().getLCLView(); + auto const toAccountAfter = ledgerView->getAccount(destination); REQUIRE(toAccount); REQUIRE(toAccountAfter); } diff --git a/src/test/TxTests.cpp b/src/test/TxTests.cpp index cd5b853a2d..cab72f1458 100644 --- a/src/test/TxTests.cpp +++ b/src/test/TxTests.cpp @@ -180,9 +180,16 @@ applyCheck(TransactionTestFramePtr tx, Application& app, bool checkSeqNum) bool checkedTxApplyRes = false; { LedgerTxn ltxFeeProc(ltx); - // use checkedTx here for validity check as to keep tx untouched - check = checkedTx->checkValidForTesting(app.getAppConnector(), - ltxFeeProc, 0, 0, 0); + { + // use checkedTx here for validity check as to keep tx untouched. + // Before protocol 8 the account loading code writes through to the + // LedgerTxn, so validate against a nested one that is rolled back, + // leaving ltxFeeProc with the fee processing change alone. + LedgerTxn ltxCheck(ltxFeeProc); + LedgerTxnView feeProcView(ltxCheck); + check = checkedTx->checkValidForTesting(app.getAppConnector(), + feeProcView, 0, 0, 0); + } checkResult = checkedTx->getResult(); REQUIRE((!check || checkResult.result.code() == txSUCCESS)); @@ -489,9 +496,9 @@ validateTxResults(TransactionTestFramePtr const& tx, Application& app, TransactionFrameBase::makeTransactionFromWire(app.getNetworkID(), tx->getEnvelope())); { - LedgerTxn ltx(app.getLedgerTxnRoot()); - REQUIRE(checkedTx->checkValidForTesting(app.getAppConnector(), ltx, 0, - 0, 0) == shouldValidateOk); + REQUIRE(checkedTx->checkValidForTesting( + app.getAppConnector(), *app.getLedgerManager().getLCLView(), + 0, 0, 0) == shouldValidateOk); } REQUIRE(checkedTx->getResult().result.code() == validationResult.code); REQUIRE(checkedTx->getResult().feeCharged == validationResult.fee); @@ -711,15 +718,15 @@ loadAccount(AbstractLedgerTxn& ltx, PublicKey const& k, bool mustExist) bool doesAccountExist(Application& app, PublicKey const& k) { - CheckValidLedgerViewWrapper lss(app); - return (bool)lss.getAccount(k); + auto lss = app.getLedgerManager().getLCLView(); + return (bool)lss->getAccount(k); } xdr::xvector getAccountSigners(PublicKey const& k, Application& app) { - CheckValidLedgerViewWrapper lss(app); - auto account = lss.getAccount(k); + auto lss = app.getLedgerManager().getLCLView(); + auto account = lss->getAccount(k); return account.current().data.account().signers; } @@ -2054,7 +2061,7 @@ makeConfigUpgradeSet(AbstractLedgerTxn& ltx, ConfigUpgradeSet configUpgradeSet, ltx.create(InternalLedgerEntry(ttl)); auto upgradeKey = ConfigUpgradeSetKey{contractID, hashOfUpgradeSet}; - CheckValidLedgerViewWrapper ledgerView(ltx); + LedgerTxnView ledgerView(ltx); return ConfigUpgradeSetFrame::makeFromKey(ledgerView, upgradeKey); } diff --git a/src/test/fuzz/targets/TxFuzzTarget.cpp b/src/test/fuzz/targets/TxFuzzTarget.cpp index c43b3c879f..c571a64287 100644 --- a/src/test/fuzz/targets/TxFuzzTarget.cpp +++ b/src/test/fuzz/targets/TxFuzzTarget.cpp @@ -127,15 +127,23 @@ FuzzTransactionFrame::attemptApplication(Application& app, AlwaysValidSignatureChecker signatureChecker{ ltx.loadHeader().current().ledgerVersion, getContentsHash(), mEnvelope.v1().signatures}; - CheckValidLedgerViewWrapper ltxStmt(ltx); + + std::optional sorobanNetworkConfig; + if (protocolVersionStartsFrom(ltx.loadHeader().current().ledgerVersion, + SOROBAN_PROTOCOL_VERSION)) + { + sorobanNetworkConfig.emplace( + app.getAppConnector().getLastClosedSorobanNetworkConfig()); + } + auto const* sorobanConfigPtr = + sorobanNetworkConfig ? &(*sorobanNetworkConfig) : nullptr; + LedgerTxnView ltxStmt(ltx, sorobanConfigPtr); // if any ill-formed Operations, do not attempt transaction application auto isInvalidOperation = [&](auto const& op, auto& opResult) { auto diagnostics = DiagnosticEventManager::createForValidation(app.getConfig()); - return !op->checkValid( - app.getAppConnector(), signatureChecker, - &app.getAppConnector().getLastClosedSorobanNetworkConfig(), ltxStmt, - false, opResult, diagnostics); + return !op->checkValid(app.getAppConnector(), signatureChecker, ltxStmt, + false, opResult, diagnostics); }; auto const& ops = getOperations(); @@ -158,7 +166,7 @@ FuzzTransactionFrame::attemptApplication(Application& app, TransactionMetaBuilder tm(true, *this, ltx.loadHeader().current().ledgerVersion, app.getAppConnector()); - std::optional sorobanNetworkConfig; + Hash sorobanRngSeed; applyOperations(signatureChecker, app.getAppConnector(), ltx, tm, *mTxResult, sorobanNetworkConfig, sorobanRngSeed); diff --git a/src/transactions/FeeBumpTransactionFrame.cpp b/src/transactions/FeeBumpTransactionFrame.cpp index 3513237f7b..c3f958c486 100644 --- a/src/transactions/FeeBumpTransactionFrame.cpp +++ b/src/transactions/FeeBumpTransactionFrame.cpp @@ -84,15 +84,13 @@ FeeBumpTransactionFrame::FeeBumpTransactionFrame( void FeeBumpTransactionFrame::preParallelApplyReadOnly( - AppConnector& app, CheckValidLedgerViewWrapper const& ls, - TransactionMetaBuilder& meta, MutableTransactionResultBase& txResult, - SorobanNetworkConfig const& sorobanConfig) const + AppConnector& app, AbstractLedgerView const& ls, + TransactionMetaBuilder& meta, MutableTransactionResultBase& txResult) const { try { mInnerTx->preParallelApplyReadOnlyWithOptionallyChargedFee( - /*chargeFee=*/false, app, ls, meta, txResult, sorobanConfig, - getContentsHash()); + /*chargeFee=*/false, app, ls, meta, txResult, getContentsHash()); } catch (std::exception& e) { @@ -258,8 +256,7 @@ FeeBumpTransactionFrame::checkSignature(SignatureChecker& signatureChecker, bool FeeBumpTransactionFrame::checkOperationSignatures( - SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, + SignatureChecker& signatureChecker, AbstractLedgerView const& ledgerView, MutableTransactionResultBase* txResult) const { // Fee bumps do not contain explicit operations, so this check trivially @@ -284,7 +281,7 @@ FeeBumpTransactionFrame::checkAllTransactionSignatures( MutableTxResultPtr FeeBumpTransactionFrame::checkValidImpl( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, DiagnosticEventManager& diagnosticEvents, bool isOverlayValidation, @@ -319,8 +316,9 @@ FeeBumpTransactionFrame::checkValidImpl( if (protocolVersionStartsFrom(ledgerVersion, SOROBAN_PROTOCOL_VERSION)) { // CAP-77: Check if fee bump source account is frozen - auto const& sorobanConfig = - app.getLedgerManager().getLastClosedSorobanNetworkConfig(); + auto const* sorobanConfigPtr = ledgerView.getSorobanNetworkConfig(); + releaseAssertOrThrow(sorobanConfigPtr != nullptr); + auto const& sorobanConfig = *sorobanConfigPtr; if (sorobanConfig.hasFrozenKeys()) { auto feeAcctKey = accountKey(getFeeSourceID()); @@ -349,7 +347,7 @@ FeeBumpTransactionFrame::checkValidImpl( MutableTxResultPtr FeeBumpTransactionFrame::checkValid( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, DiagnosticEventManager& diagnosticEvents, @@ -362,7 +360,7 @@ FeeBumpTransactionFrame::checkValid( MutableTxResultPtr FeeBumpTransactionFrame::checkValidForOverlay( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, DiagnosticEventManager& diagnosticEvents, @@ -384,7 +382,7 @@ FeeBumpTransactionFrame::checkSorobanResources( std::optional FeeBumpTransactionFrame::commonValidPreSeqNum( - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, MutableTransactionResultBase& txResult) const { // this function does validations that are independent of the account state @@ -463,9 +461,8 @@ FeeBumpTransactionFrame::commonValidPreSeqNum( FeeBumpTransactionFrame::ValidationType FeeBumpTransactionFrame::commonValid( - SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, bool applying, - MutableTransactionResultBase& txResult) const + SignatureChecker& signatureChecker, AbstractLedgerView const& ledgerView, + bool applying, MutableTransactionResultBase& txResult) const { ValidationType res = ValidationType::kInvalid; diff --git a/src/transactions/FeeBumpTransactionFrame.h b/src/transactions/FeeBumpTransactionFrame.h index 98134acaef..2d06150be3 100644 --- a/src/transactions/FeeBumpTransactionFrame.h +++ b/src/transactions/FeeBumpTransactionFrame.h @@ -35,7 +35,7 @@ class FeeBumpTransactionFrame : public TransactionFrameBase bool checkOperationSignatures( SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, MutableTransactionResultBase* txResult) const override; bool checkAllTransactionSignatures(SignatureChecker& signatureChecker, @@ -45,7 +45,7 @@ class FeeBumpTransactionFrame : public TransactionFrameBase // If check passes, returns the fee source account. Otherwise returns // nullopt. std::optional - commonValidPreSeqNum(CheckValidLedgerViewWrapper const& ledgerView, + commonValidPreSeqNum(AbstractLedgerView const& ledgerView, MutableTransactionResultBase& txResult) const; enum ValidationType @@ -57,18 +57,19 @@ class FeeBumpTransactionFrame : public TransactionFrameBase }; ValidationType commonValid(SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, bool applying, MutableTransactionResultBase& txResult) const; void removeOneTimeSignerKeyFromFeeSource(AbstractLedgerTxn& ltx) const; - MutableTxResultPtr checkValidImpl( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, - SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, - uint64_t upperBoundCloseTimeOffset, - DiagnosticEventManager& diagnosticEvents, bool isOverlayValidation, - std::optional validationLedgerSeq = std::nullopt) const; + MutableTxResultPtr + checkValidImpl(AppConnector& app, AbstractLedgerView const& ledgerView, + SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, + uint64_t upperBoundCloseTimeOffset, + DiagnosticEventManager& diagnosticEvents, + bool isOverlayValidation, + std::optional validationLedgerSeq) const; public: FeeBumpTransactionFrame(Hash const& networkID, @@ -91,9 +92,9 @@ class FeeBumpTransactionFrame : public TransactionFrameBase ~FeeBumpTransactionFrame() override = default; void preParallelApplyReadOnly( - AppConnector& app, CheckValidLedgerViewWrapper const& ls, - TransactionMetaBuilder& meta, MutableTransactionResultBase& txResult, - SorobanNetworkConfig const& sorobanConfig) const override; + AppConnector& app, AbstractLedgerView const& ls, + TransactionMetaBuilder& meta, + MutableTransactionResultBase& txResult) const override; void preParallelApplyWrite( AppConnector& app, AbstractLedgerTxn& ltx, TransactionMetaBuilder& meta, @@ -122,7 +123,7 @@ class FeeBumpTransactionFrame : public TransactionFrameBase TxEventManager& txEventManager) const override; MutableTxResultPtr checkValid(AppConnector& app, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, @@ -130,7 +131,7 @@ class FeeBumpTransactionFrame : public TransactionFrameBase std::optional validationLedgerSeq = std::nullopt) const override; MutableTxResultPtr checkValidForOverlay( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, DiagnosticEventManager& diagnosticEvents, diff --git a/src/transactions/OperationFrame.cpp b/src/transactions/OperationFrame.cpp index d70bcea9ed..80d2ad1300 100644 --- a/src/transactions/OperationFrame.cpp +++ b/src/transactions/OperationFrame.cpp @@ -148,10 +148,26 @@ OperationFrame::apply( ZoneScoped; CLOG_TRACE(Tx, "{}", xdrToCerealString(mOperation, "Operation")); - CheckValidLedgerViewWrapper ltxState(ltx); - bool applyRes = checkValid( - app, signatureChecker, sorobanConfig ? &sorobanConfig.value() : nullptr, - ltxState, true, res, opMeta.getDiagnosticEventManager()); + auto const* cfg = sorobanConfig ? &sorobanConfig.value() : nullptr; + bool applyRes = [&] { + // Older protocol versions contain buggy account loading code that + // writes through to the LedgerTxn, so validate against a nested + // LedgerTxn that is rolled back. + std::optional maybeLegacyNestedLtx; + if (protocolVersionIsBefore(ltx.loadHeader().current().ledgerVersion, + ProtocolVersion::V_8)) + { + maybeLegacyNestedLtx.emplace(ltx); + } + LedgerTxnView ledgerView( + maybeLegacyNestedLtx + ? static_cast(*maybeLegacyNestedLtx) + : ltx, + cfg); + return checkValid(app, signatureChecker, ledgerView, + /*forApply=*/true, res, + opMeta.getDiagnosticEventManager()); + }(); if (applyRes) { if (isSoroban()) @@ -215,11 +231,11 @@ OperationFrame::isOpSupported(LedgerHeader const&) const bool OperationFrame::checkSignature(SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, + LedgerHeaderWrapper const& header, OperationResult* res, bool forApply) const { ZoneScoped; - auto header = ledgerView.getLedgerHeader(); auto sourceAccount = ledgerView.getAccount(header, mParentTx, getSourceID()); if (sourceAccount) @@ -282,84 +298,55 @@ OperationFrame::getSourceAccount() const bool OperationFrame::checkValid(AppConnector& app, SignatureChecker& signatureChecker, - SorobanNetworkConfig const* cfg, - CheckValidLedgerViewWrapper const& ledgerView, - bool forApply, OperationResult& res, + AbstractLedgerView const& ledgerView, bool forApply, + OperationResult& res, DiagnosticEventManager& diagnosticEvents) const { ZoneScoped; - bool validationResult = false; - auto validate = [this, &res, forApply, &signatureChecker, &app, - &diagnosticEvents, &validationResult, - &cfg](CheckValidLedgerViewWrapper const& ledgerView) { - if (!isOpSupported(ledgerView.getLedgerHeader().current())) - { - res.code(opNOT_SUPPORTED); - validationResult = false; - return; - } - - auto ledgerVersion = - ledgerView.getLedgerHeader().current().ledgerVersion; - if (!forApply || - protocolVersionIsBefore(ledgerVersion, ProtocolVersion::V_10)) - { - if (!checkSignature(signatureChecker, ledgerView, &res, forApply)) - { - validationResult = false; - return; - } - } - else - { - // for ledger versions >= 10 we need to load account here, as for - // previous versions it is done in checkSignature call - // If we get to operation checkvalid, we know the tx source account - // has already been checked for existence. If we're not applying, - // it's guaranteed that the tx source account exists, since ledger - // state hasn't changed, so we can skip this redundant check. - // If we're applying, it's possible an earlier op modified the TX - // source, so we need to check again. - if ((mOperation.sourceAccount || forApply) && - !ledgerView.getAccount(ledgerView.getLedgerHeader(), mParentTx, - getSourceID())) - { - res.code(opNO_ACCOUNT); - validationResult = false; - return; - } - } + auto header = ledgerView.getLedgerHeader(); + if (!isOpSupported(header.current())) + { + res.code(opNOT_SUPPORTED); + return false; + } - if (protocolVersionStartsFrom(ledgerVersion, - SOROBAN_PROTOCOL_VERSION) && - isSoroban()) - { - releaseAssertOrThrow(cfg); - validationResult = doCheckValidForSoroban( - *cfg, app.getConfig(), ledgerVersion, res, diagnosticEvents); - } - else + auto ledgerVersion = header.current().ledgerVersion; + if (!forApply || + protocolVersionIsBefore(ledgerVersion, ProtocolVersion::V_10)) + { + if (!checkSignature(signatureChecker, ledgerView, header, &res, + forApply)) { - validationResult = doCheckValid(ledgerVersion, res); + return false; } - }; - - // Older protocol versions contain buggy account loading code, - // so preserve nested LedgerTxn to avoid writing to the ledger - if (protocolVersionIsBefore( - ledgerView.getLedgerHeader().current().ledgerVersion, - ProtocolVersion::V_8) && - forApply) - { - ledgerView.executeWithMaybeInnerSnapshot(validate); } else { - // Validate using read-only snapshot - validate(ledgerView); + // for ledger versions >= 10 we need to load account here, as for + // previous versions it is done in checkSignature call + // If we get to operation checkvalid, we know the tx source account + // has already been checked for existence. If we're not applying, + // it's guaranteed that the tx source account exists, since ledger + // state hasn't changed, so we can skip this redundant check. + // If we're applying, it's possible an earlier op modified the TX + // source, so we need to check again. + if ((mOperation.sourceAccount || forApply) && + !ledgerView.getAccount(header, mParentTx, getSourceID())) + { + res.code(opNO_ACCOUNT); + return false; + } } - return validationResult; + if (protocolVersionStartsFrom(ledgerVersion, SOROBAN_PROTOCOL_VERSION) && + isSoroban()) + { + auto const* cfg = ledgerView.getSorobanNetworkConfig(); + releaseAssertOrThrow(cfg); + return doCheckValidForSoroban(*cfg, app.getConfig(), ledgerVersion, res, + diagnosticEvents); + } + return doCheckValid(ledgerVersion, res); } bool diff --git a/src/transactions/OperationFrame.h b/src/transactions/OperationFrame.h index a4028de87c..d25c46100e 100644 --- a/src/transactions/OperationFrame.h +++ b/src/transactions/OperationFrame.h @@ -15,6 +15,7 @@ namespace stellar { +class AbstractLedgerView; class AbstractLedgerTxn; class LedgerManager; class LedgerTxnEntry; @@ -97,16 +98,16 @@ class OperationFrame // to `nullptr` if they do not directly need the result of signature // validation (such as in the case of background signature validation). bool checkSignature(SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, - OperationResult* res, bool forApply) const; + AbstractLedgerView const& ledgerView, + LedgerHeaderWrapper const& header, OperationResult* res, + bool forApply) const; AccountID getSourceID() const; MuxedAccount getSourceAccount() const; bool checkValid(AppConnector& app, SignatureChecker& signatureChecker, - SorobanNetworkConfig const* cfg, - CheckValidLedgerViewWrapper const& ledgerView, - bool forApply, OperationResult& res, + AbstractLedgerView const& ledgerView, bool forApply, + OperationResult& res, DiagnosticEventManager& diagnosticEvents) const; bool apply(AppConnector& app, SignatureChecker& signatureChecker, diff --git a/src/transactions/ParallelApplyUtils.cpp b/src/transactions/ParallelApplyUtils.cpp index 08435ed03a..9ca02aadf5 100644 --- a/src/transactions/ParallelApplyUtils.cpp +++ b/src/transactions/ParallelApplyUtils.cpp @@ -450,15 +450,14 @@ GlobalParallelApplyLedgerState::readOnlyParallelPreApply( [&](size_t begin, size_t end, size_t) { // NB: mLCLApplyView is not thread-safe, so we need to copy it into // a thread-local view. - CheckValidLedgerViewWrapper ledgerView( - std::make_unique( - header, getUpdatedEntry, mLCLApplyView)); + SorobanPreApplyLedgerView ledgerView(header, getUpdatedEntry, + mLCLApplyView); for (size_t i = begin; i < end; ++i) { auto const* txBundle = txBundles[i]; txBundle->getTx()->preParallelApplyReadOnly( app, ledgerView, txBundle->getEffects().getMeta(), - txBundle->getResPayload(), mSorobanConfig); + txBundle->getResPayload()); } }); } diff --git a/src/transactions/TransactionFrame.cpp b/src/transactions/TransactionFrame.cpp index 4abc9f40ec..1a93e5b439 100644 --- a/src/transactions/TransactionFrame.cpp +++ b/src/transactions/TransactionFrame.cpp @@ -481,17 +481,18 @@ TransactionFrame::checkExtraSigners(SignatureChecker& signatureChecker) const bool TransactionFrame::checkOperationSignatures( - SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, + SignatureChecker& signatureChecker, AbstractLedgerView const& ledgerView, MutableTransactionResultBase* txResult) const { ZoneScoped; + auto header = ledgerView.getLedgerHeader(); bool allOpsValid = true; for (size_t i = 0; i < mOperations.size(); ++i) { auto const& op = mOperations[i]; auto opResult = txResult ? &txResult->getOpResultAt(i) : nullptr; - if (!op->checkSignature(signatureChecker, ledgerView, opResult, false)) + if (!op->checkSignature(signatureChecker, ledgerView, header, opResult, + /*forApply=*/false)) { allOpsValid = false; } @@ -1245,8 +1246,7 @@ TransactionFrame::isTooEarlyForAccount(uint32_t ledgerVersion, std::optional TransactionFrame::commonValidPreSeqNum( - AppConnector& app, SorobanNetworkConfig const* cfg, - CheckValidLedgerViewWrapper const& ledgerView, bool chargeFee, + AppConnector& app, AbstractLedgerView const& ledgerView, bool chargeFee, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, Hash const& envelopeContentsHash, std::optional sorobanResourceFee, MutableTransactionResultBase& txResult, @@ -1257,8 +1257,9 @@ TransactionFrame::commonValidPreSeqNum( // this function does validations that are independent of the account state // (stay true regardless of other side effects) - uint32_t ledgerVersion = - ledgerView.getLedgerHeader().current().ledgerVersion; + auto const* cfg = ledgerView.getSorobanNetworkConfig(); + auto header = ledgerView.getLedgerHeader(); + uint32_t ledgerVersion = header.current().ledgerVersion; if ((protocolVersionIsBefore(ledgerVersion, ProtocolVersion::V_13) && (mEnvelope.type() == ENVELOPE_TYPE_TX || hasMuxedAccount(mEnvelope))) || @@ -1318,9 +1319,7 @@ TransactionFrame::commonValidPreSeqNum( return std::nullopt; } - if (protocolVersionStartsFrom( - ledgerView.getLedgerHeader().current().ledgerVersion, - ProtocolVersion::V_25)) + if (protocolVersionStartsFrom(ledgerVersion, ProtocolVersion::V_25)) { if (!validateSorobanMemo()) { @@ -1442,8 +1441,6 @@ TransactionFrame::commonValidPreSeqNum( } } - auto header = ledgerView.getLedgerHeader(); - // If we have an overriding ledger sequence for validation (like when tx // queue is accepting TXs for the next ledger), use that for time-based // checks instead of the ledgerSeq from the header. @@ -1520,11 +1517,11 @@ TransactionFrame::processSeqNum(AbstractLedgerTxn& ltx) const } bool -TransactionFrame::processSignatures( - ValidationType cv, SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, - MutableTransactionResultBase& txResult, - AbstractLedgerTxn* ltxForWrites) const +TransactionFrame::processSignatures(ValidationType cv, + SignatureChecker& signatureChecker, + AbstractLedgerView const& ledgerView, + MutableTransactionResultBase& txResult, + AbstractLedgerTxn* ltxForWrites) const { ZoneScoped; bool maybeValid = (cv == ValidationType::kMaybeValid); @@ -1612,10 +1609,9 @@ TransactionFrame::isBadSeq(LedgerHeaderWrapper const& header, TransactionFrame::ValidationType TransactionFrame::commonValid( - AppConnector& app, SorobanNetworkConfig const* cfg, - SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, SequenceNumber current, - bool applying, bool chargeFee, uint64_t lowerBoundCloseTimeOffset, + AppConnector& app, SignatureChecker& signatureChecker, + AbstractLedgerView const& ledgerView, SequenceNumber current, bool applying, + bool chargeFee, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, Hash const& envelopeContentsHash, std::optional sorobanResourceFee, MutableTransactionResultBase& txResult, @@ -1625,112 +1621,88 @@ TransactionFrame::commonValid( ZoneScoped; ValidationType res = ValidationType::kInvalid; - auto validate = [this, &signatureChecker, applying, - lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset, &app, - chargeFee, sorobanResourceFee, &txResult, - &diagnosticEvents, ¤t, &res, &cfg, - &envelopeContentsHash, validationLedgerSeq]( - CheckValidLedgerViewWrapper const& ledgerView) { - if (applying && - (lowerBoundCloseTimeOffset != 0 || upperBoundCloseTimeOffset != 0)) - { - throw std::logic_error( - "Applying transaction with non-current closeTime"); - } + if (applying && + (lowerBoundCloseTimeOffset != 0 || upperBoundCloseTimeOffset != 0)) + { + throw std::logic_error( + "Applying transaction with non-current closeTime"); + } - // Get the source account during commonValidPreSeqNum to avoid - // redundant account loading - auto sourceAccount = commonValidPreSeqNum( - app, cfg, ledgerView, chargeFee, lowerBoundCloseTimeOffset, - upperBoundCloseTimeOffset, envelopeContentsHash, sorobanResourceFee, - txResult, diagnosticEvents, validationLedgerSeq); + auto sourceAccount = commonValidPreSeqNum( + app, ledgerView, chargeFee, lowerBoundCloseTimeOffset, + upperBoundCloseTimeOffset, envelopeContentsHash, sorobanResourceFee, + txResult, diagnosticEvents, validationLedgerSeq); - if (!sourceAccount) - { - return; - } + if (!sourceAccount) + { + return res; + } - auto header = ledgerView.getLedgerHeader(); + auto header = ledgerView.getLedgerHeader(); - // in older versions, the account's sequence number is updated when - // taking fees - if (protocolVersionStartsFrom(header.current().ledgerVersion, - ProtocolVersion::V_10) || - !applying) + // in older versions, the account's sequence number is updated when + // taking fees + if (protocolVersionStartsFrom(header.current().ledgerVersion, + ProtocolVersion::V_10) || + !applying) + { + if (current == 0) { - if (current == 0) - { - current = sourceAccount->current().data.account().seqNum; - } - bool skipCheck = false; + current = sourceAccount->current().data.account().seqNum; + } #ifdef BUILD_TESTS - skipCheck = ledgerView.mSkipSeqNumCheck; + // Overlay-only mode freezes the on-disk seqnums at genesis while + // LoadGenerator keeps advancing its local counters, so every tx after + // the first would otherwise fail isBadSeq. + if (!app.getRunInOverlayOnlyMode() && isBadSeq(header, current)) +#else + if (isBadSeq(header, current)) #endif - if (!skipCheck && isBadSeq(header, current)) - { - txResult.setInnermostError(txBAD_SEQ); - return; - } - } - - res = ValidationType::kInvalidUpdateSeqNum; - - auto ledgerSeq = - validationLedgerSeq.value_or(header.current().ledgerSeq); - auto closeTime = header.current().scpValue.closeTime; - if (isTooEarlyForAccount(header.current().ledgerVersion, closeTime, - ledgerSeq, *sourceAccount, - lowerBoundCloseTimeOffset)) { - txResult.setInnermostError(txBAD_MIN_SEQ_AGE_OR_GAP); - return; - } - - if (!checkAllTransactionSignatures(signatureChecker, *sourceAccount, - header.current().ledgerVersion)) - { - txResult.setInnermostError(txBAD_AUTH); - return; + txResult.setInnermostError(txBAD_SEQ); + return res; } + } - res = ValidationType::kInvalidPostAuth; + res = ValidationType::kInvalidUpdateSeqNum; - // if we are in applying mode fee was already deduced from signing - // account balance, if not, we need to check if after that deduction - // this account will still have minimum balance - uint32_t feeToPay = (applying && protocolVersionStartsFrom( - header.current().ledgerVersion, - ProtocolVersion::V_9)) - ? 0 - : static_cast(getFullFee()); - // don't let the account go below the reserve after accounting for - // liabilities - if (chargeFee && - getAvailableBalance(header.current(), sourceAccount->current()) < - feeToPay) - { - txResult.setInnermostError(txINSUFFICIENT_BALANCE); - return; - } - - res = ValidationType::kMaybeValid; - }; + auto ledgerSeq = validationLedgerSeq.value_or(header.current().ledgerSeq); + auto closeTime = header.current().scpValue.closeTime; + if (isTooEarlyForAccount(header.current().ledgerVersion, closeTime, + ledgerSeq, *sourceAccount, + lowerBoundCloseTimeOffset)) + { + txResult.setInnermostError(txBAD_MIN_SEQ_AGE_OR_GAP); + return res; + } - // Older protocol versions contain buggy account loading code, - // so preserve nested LedgerTxn to avoid writing to the ledger - if (protocolVersionIsBefore( - ledgerView.getLedgerHeader().current().ledgerVersion, - ProtocolVersion::V_8) && - applying) + if (!checkAllTransactionSignatures(signatureChecker, *sourceAccount, + header.current().ledgerVersion)) { - ledgerView.executeWithMaybeInnerSnapshot(validate); + txResult.setInnermostError(txBAD_AUTH); + return res; } - else + + res = ValidationType::kInvalidPostAuth; + + // if we are in applying mode fee was already deduced from signing + // account balance, if not, we need to check if after that deduction + // this account will still have minimum balance + uint32_t feeToPay = + (applying && protocolVersionStartsFrom(header.current().ledgerVersion, + ProtocolVersion::V_9)) + ? 0 + : static_cast(getFullFee()); + // don't let the account go below the reserve after accounting for + // liabilities + if (chargeFee && getAvailableBalance(header.current(), + sourceAccount->current()) < feeToPay) { - // Validate using read-only snapshot - validate(ledgerView); + txResult.setInnermostError(txINSUFFICIENT_BALANCE); + return res; } - return res; + + return ValidationType::kMaybeValid; } MutableTxResultPtr @@ -1852,7 +1824,7 @@ TransactionFrame::removeAccountSigner(AbstractLedgerTxn& ltxOuter, void TransactionFrame::checkValidWithOptionallyChargedFee( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, bool chargeFee, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, Hash const& envelopeContentsHash, MutableTransactionResultBase& txResult, @@ -1872,16 +1844,16 @@ TransactionFrame::checkValidWithOptionallyChargedFee( // Load sorobanConfig for all transactions at protocol >= V20. if (protocolVersionStartsFrom(ledgerVersion, SOROBAN_PROTOCOL_VERSION)) { - sorobanConfig = - &app.getLedgerManager().getLastClosedSorobanNetworkConfig(); + sorobanConfig = ledgerView.getSorobanNetworkConfig(); + releaseAssertOrThrow(sorobanConfig != nullptr); if (isSoroban()) { sorobanResourceFee = computePreApplySorobanResourceFee( ledgerVersion, *sorobanConfig, app.getConfig()); } } - if (commonValid(app, sorobanConfig, signatureChecker, ledgerView, current, - false, chargeFee, lowerBoundCloseTimeOffset, + if (commonValid(app, signatureChecker, ledgerView, current, + /*applying=*/false, chargeFee, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset, envelopeContentsHash, sorobanResourceFee, txResult, diagnosticEvents, validationLedgerSeq) != ValidationType::kMaybeValid) @@ -1894,8 +1866,8 @@ TransactionFrame::checkValidWithOptionallyChargedFee( auto const& op = mOperations[i]; auto& opResult = txResult.getOpResultAt(i); - if (!op->checkValid(app, signatureChecker, sorobanConfig, ledgerView, - false, opResult, diagnosticEvents)) + if (!op->checkValid(app, signatureChecker, ledgerView, + /*forApply=*/false, opResult, diagnosticEvents)) { // it's OK to just fast fail here and not try to call // checkValid on all operations as the resulting object @@ -1913,7 +1885,7 @@ TransactionFrame::checkValidWithOptionallyChargedFee( MutableTxResultPtr TransactionFrame::checkValidImpl( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, DiagnosticEventManager& diagnosticEvents, bool isOverlayValidation, @@ -1951,7 +1923,7 @@ TransactionFrame::checkValidImpl( MutableTxResultPtr TransactionFrame::checkValid(AppConnector& app, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, @@ -1965,7 +1937,7 @@ TransactionFrame::checkValid(AppConnector& app, MutableTxResultPtr TransactionFrame::checkValidForOverlay( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, DiagnosticEventManager& diagnosticEvents, @@ -2026,21 +1998,20 @@ maybeTriggerTestInternalError(TransactionEnvelope const& env) std::unique_ptr TransactionFrame::commonPreApply(bool chargeFee, AppConnector& app, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, TransactionMetaBuilder& meta, MutableTransactionResultBase& txResult, - SorobanNetworkConfig const* sorobanConfig, Hash const& envelopeContentsHash, AbstractLedgerTxn* ltxForWrites) const { mCachedAccountPreProtocol8.reset(); uint32_t ledgerVersion = ledgerView.getLedgerHeader().current().ledgerVersion; + auto const* sorobanConfig = ledgerView.getSorobanNetworkConfig(); std::unique_ptr signatureChecker; #ifdef BUILD_TESTS - // If the txResult has a replay result (catchup in skip mode is - // enabled), - // we do not perform signature verification. + // If the txResult has a replay result (catchup in skip mode is enabled), + // we do not perform signature verification. if (txResult.hasReplayTransactionResult()) { signatureChecker = std::make_unique( @@ -2055,13 +2026,11 @@ TransactionFrame::commonPreApply(bool chargeFee, AppConnector& app, } #endif // BUILD_TESTS - // when applying, a failure during tx validation means that - // we'll skip trying to apply operations but we'll still - // process the sequence number if needed std::optional sorobanResourceFee; if (protocolVersionStartsFrom(ledgerVersion, SOROBAN_PROTOCOL_VERSION) && isSoroban()) { + releaseAssertOrThrow(sorobanConfig); sorobanResourceFee = computePreApplySorobanResourceFee( ledgerVersion, *sorobanConfig, app.getConfig()); @@ -2072,13 +2041,32 @@ TransactionFrame::commonPreApply(bool chargeFee, AppConnector& app, txResult.initializeRefundableFeeTracker(initialFeeRefund); } - // Pass in nullopt, we always use the header ledgerSeq in the apply path for - // validation. - auto cv = - commonValid(app, sorobanConfig, *signatureChecker, ledgerView, 0, true, - chargeFee, 0, 0, envelopeContentsHash, sorobanResourceFee, - txResult, meta.getDiagnosticEventManager(), - /*validationLedgerSeq=*/std::nullopt); + ValidationType cv; + { + // Older protocol versions contain buggy account loading code that + // writes through to the LedgerTxn, so validate against a nested + // LedgerTxn that is rolled back. + std::optional maybeLegacyNestedLtx; + std::optional maybeLegacyValidationView; + if (ltxForWrites && + protocolVersionIsBefore( + ltxForWrites->loadHeader().current().ledgerVersion, + ProtocolVersion::V_8)) + { + maybeLegacyNestedLtx.emplace(*ltxForWrites); + maybeLegacyValidationView.emplace(*maybeLegacyNestedLtx, + sorobanConfig); + } + // Pass in nullopt, we always use the header ledgerSeq in the apply + // path for validation. + cv = commonValid( + app, *signatureChecker, + maybeLegacyValidationView ? *maybeLegacyValidationView : ledgerView, + 0, + /*applying=*/true, chargeFee, 0, 0, envelopeContentsHash, + sorobanResourceFee, txResult, meta.getDiagnosticEventManager(), + /*validationLedgerSeq=*/std::nullopt); + } if (ltxForWrites && cv >= ValidationType::kInvalidUpdateSeqNum) { processSeqNum(*ltxForWrites); @@ -2086,28 +2074,22 @@ TransactionFrame::commonPreApply(bool chargeFee, AppConnector& app, bool signaturesValid = processSignatures(cv, *signatureChecker, ledgerView, txResult, ltxForWrites); - if (signaturesValid && cv == ValidationType::kMaybeValid) { return signatureChecker; } - else - { - return nullptr; - } + return nullptr; } void TransactionFrame::preParallelApplyReadOnly( - AppConnector& app, CheckValidLedgerViewWrapper const& ls, - TransactionMetaBuilder& meta, MutableTransactionResultBase& txResult, - SorobanNetworkConfig const& sorobanConfig) const + AppConnector& app, AbstractLedgerView const& ls, + TransactionMetaBuilder& meta, MutableTransactionResultBase& txResult) const { try { preParallelApplyReadOnlyWithOptionallyChargedFee( - /*chargeFee=*/true, app, ls, meta, txResult, sorobanConfig, - getContentsHash()); + /*chargeFee=*/true, app, ls, meta, txResult, getContentsHash()); } catch (std::exception& e) { @@ -2123,10 +2105,8 @@ TransactionFrame::preParallelApplyReadOnly( void TransactionFrame::preParallelApplyReadOnlyWithOptionallyChargedFee( - bool chargeFee, AppConnector& app, - CheckValidLedgerViewWrapper const& ledgerView, TransactionMetaBuilder& meta, - MutableTransactionResultBase& txResult, - SorobanNetworkConfig const& sorobanConfig, + bool chargeFee, AppConnector& app, AbstractLedgerView const& ledgerView, + TransactionMetaBuilder& meta, MutableTransactionResultBase& txResult, Hash const& envelopeContentsHash) const { ZoneScoped; @@ -2135,17 +2115,16 @@ TransactionFrame::preParallelApplyReadOnlyWithOptionallyChargedFee( auto signatureChecker = commonPreApply(chargeFee, app, ledgerView, meta, txResult, - &sorobanConfig, envelopeContentsHash, - /*ltxForWrites=*/nullptr); + envelopeContentsHash, /*ltxForWrites=*/nullptr); bool ok = signatureChecker != nullptr; if (ok) { updateSorobanMetrics(app); auto& opResult = txResult.getOpResultAt(0); - ok = mOperations.front()->checkValid( - app, *signatureChecker, &sorobanConfig, ledgerView, true, opResult, - meta.getDiagnosticEventManager()); + ok = mOperations.front()->checkValid(app, *signatureChecker, ledgerView, + /*forApply=*/true, opResult, + meta.getDiagnosticEventManager()); if (!ok) { txResult.setInnermostError(txFAILED); @@ -2506,10 +2485,10 @@ TransactionFrame::apply( { auto signatureChecker = [&] { LedgerTxn ltxTx(ltx); - CheckValidLedgerViewWrapper lsTx(ltxTx); + LedgerTxnView ltxView(ltxTx, sorobanConfig ? &sorobanConfig.value() + : nullptr); auto checker = - commonPreApply(chargeFee, app, lsTx, meta, txResult, - sorobanConfig ? &sorobanConfig.value() : nullptr, + commonPreApply(chargeFee, app, ltxView, meta, txResult, envelopeContentsHash, <xTx); meta.pushTxChangesBefore(ltxTx); ltxTx.commit(); diff --git a/src/transactions/TransactionFrame.h b/src/transactions/TransactionFrame.h index 73022d2511..676062742d 100644 --- a/src/transactions/TransactionFrame.h +++ b/src/transactions/TransactionFrame.h @@ -56,12 +56,13 @@ class TransactionFrame : public TransactionFrameBase bool maybeAdoptFailedReplayResult(MutableTransactionResultBase& txResult) const; - MutableTxResultPtr checkValidImpl( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, - SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, - uint64_t upperBoundCloseTimeOffset, - DiagnosticEventManager& diagnosticEvents, bool isOverlayValidation, - std::optional validationLedgerSeq = std::nullopt) const; + MutableTxResultPtr + checkValidImpl(AppConnector& app, AbstractLedgerView const& ledgerView, + SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, + uint64_t upperBoundCloseTimeOffset, + DiagnosticEventManager& diagnosticEvents, + bool isOverlayValidation, + std::optional validationLedgerSeq) const; protected: #ifdef BUILD_TESTS @@ -83,7 +84,7 @@ class TransactionFrame : public TransactionFrameBase LedgerTxnEntry loadSourceAccount(AbstractLedgerTxn& ltx, LedgerTxnHeader const& header) const; - friend class LedgerTxnReadOnly; + friend class LedgerTxnView; enum ValidationType { @@ -108,24 +109,21 @@ class TransactionFrame : public TransactionFrameBase uint64_t lowerBoundCloseTimeOffset) const; // If check passes, returns the source account. Otherwise returns nullopt. - std::optional - commonValidPreSeqNum(AppConnector& app, SorobanNetworkConfig const* cfg, - CheckValidLedgerViewWrapper const& ledgerView, - bool chargeFee, uint64_t lowerBoundCloseTimeOffset, - uint64_t upperBoundCloseTimeOffset, - Hash const& envelopeContentsHash, - std::optional sorobanResourceFee, - MutableTransactionResultBase& txResult, - DiagnosticEventManager& diagnosticEvents, - std::optional validationLedgerSeq) const; + std::optional commonValidPreSeqNum( + AppConnector& app, AbstractLedgerView const& ledgerView, bool chargeFee, + uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, + Hash const& envelopeContentsHash, + std::optional sorobanResourceFee, + MutableTransactionResultBase& txResult, + DiagnosticEventManager& diagnosticEvents, + std::optional validationLedgerSeq) const; virtual bool isBadSeq(LedgerHeaderWrapper const& header, int64_t seqNum) const; ValidationType commonValid( - AppConnector& app, SorobanNetworkConfig const* cfg, - SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, SequenceNumber current, + AppConnector& app, SignatureChecker& signatureChecker, + AbstractLedgerView const& ledgerView, SequenceNumber current, bool applying, bool chargeFee, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, Hash const& envelopeContentsHash, std::optional sorobanResourceFee, @@ -153,7 +151,7 @@ class TransactionFrame : public TransactionFrameBase // `processSignatures` call. bool processSignatures(ValidationType cv, SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, MutableTransactionResultBase& txResult, AbstractLedgerTxn* ltxForWrites) const; @@ -250,18 +248,18 @@ class TransactionFrame : public TransactionFrameBase bool checkOperationSignatures( SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, MutableTransactionResultBase* txResult) const override; void checkValidWithOptionallyChargedFee( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, bool chargeFee, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, Hash const& envelopeContentsHash, MutableTransactionResultBase& result, DiagnosticEventManager& diagnosticEvents, bool isOverlayValidation, std::optional validationLedgerSeq = std::nullopt) const; MutableTxResultPtr checkValid(AppConnector& app, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, @@ -269,7 +267,7 @@ class TransactionFrame : public TransactionFrameBase std::optional validationLedgerSeq = std::nullopt) const override; MutableTxResultPtr checkValidForOverlay( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, DiagnosticEventManager& diagnosticEvents, @@ -307,28 +305,24 @@ class TransactionFrame : public TransactionFrameBase // // If all of this succeeds, it returns a non-nullptr pointer to the // signature checker, to be used elsewhere in the txn. If anything - // fails it returns nullptr. `ltxForWrites` will contain the changes made - // up to the failure point in that case. - std::unique_ptr - commonPreApply(bool chargeFee, AppConnector& app, - CheckValidLedgerViewWrapper const& ledgerView, - TransactionMetaBuilder& meta, - MutableTransactionResultBase& txResult, - SorobanNetworkConfig const* sorobanConfig, - Hash const& envelopeContentsHash, - AbstractLedgerTxn* ltxForWrites) const; + // fails it returns nullptr. + // The writes go directly to `ltxForWrites` and are not rolled back on + // failure. + std::unique_ptr commonPreApply( + bool chargeFee, AppConnector& app, AbstractLedgerView const& ledgerView, + TransactionMetaBuilder& meta, MutableTransactionResultBase& txResult, + Hash const& envelopeContentsHash, + AbstractLedgerTxn* ltxForWrites) const; void preParallelApplyReadOnlyWithOptionallyChargedFee( - bool chargeFee, AppConnector& app, - CheckValidLedgerViewWrapper const& ls, TransactionMetaBuilder& meta, - MutableTransactionResultBase& txResult, - SorobanNetworkConfig const& sorobanConfig, + bool chargeFee, AppConnector& app, AbstractLedgerView const& ls, + TransactionMetaBuilder& meta, MutableTransactionResultBase& txResult, Hash const& envelopeContentsHash) const; void preParallelApplyReadOnly( - AppConnector& app, CheckValidLedgerViewWrapper const& ls, - TransactionMetaBuilder& meta, MutableTransactionResultBase& txResult, - SorobanNetworkConfig const& sorobanConfig) const override; + AppConnector& app, AbstractLedgerView const& ls, + TransactionMetaBuilder& meta, + MutableTransactionResultBase& txResult) const override; void preParallelApplyWrite( AppConnector& app, AbstractLedgerTxn& ltx, TransactionMetaBuilder& meta, diff --git a/src/transactions/TransactionFrameBase.h b/src/transactions/TransactionFrameBase.h index 22cfbd7f67..e1e3c05de2 100644 --- a/src/transactions/TransactionFrameBase.h +++ b/src/transactions/TransactionFrameBase.h @@ -160,10 +160,10 @@ class TransactionFrameBase // and the operation's checkValid. Performs no writes. Safe to run // concurrently for distinct transactions, provided `ls` supports concurrent // reads. - virtual void preParallelApplyReadOnly( - AppConnector& app, CheckValidLedgerViewWrapper const& ls, - TransactionMetaBuilder& meta, MutableTransactionResultBase& txResult, - SorobanNetworkConfig const& sorobanConfig) const = 0; + virtual void + preParallelApplyReadOnly(AppConnector& app, AbstractLedgerView const& ls, + TransactionMetaBuilder& meta, + MutableTransactionResultBase& txResult) const = 0; // The write half of the Soroban pre-apply. Has to run on the thread that // owns `ltx`, serially across transactions, in canonical transaction order. @@ -187,13 +187,13 @@ class TransactionFrameBase // validation where the snapshot reflects LCL but checks must evaluate // against the next ledger. virtual MutableTxResultPtr checkValid( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, DiagnosticEventManager& diagnosticEvents, std::optional validationLedgerSeq = std::nullopt) const = 0; virtual MutableTxResultPtr checkValidForOverlay( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, DiagnosticEventManager& diagnosticEvents, @@ -219,7 +219,7 @@ class TransactionFrameBase // populating signature cache in the background). virtual bool checkOperationSignatures(SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, MutableTransactionResultBase* txResult) const = 0; // Validate all transaction-level signatures diff --git a/src/transactions/test/BeginSponsoringFutureReservesTests.cpp b/src/transactions/test/BeginSponsoringFutureReservesTests.cpp index ebc07cef8f..1774f071cb 100644 --- a/src/transactions/test/BeginSponsoringFutureReservesTests.cpp +++ b/src/transactions/test/BeginSponsoringFutureReservesTests.cpp @@ -47,10 +47,9 @@ TEST_CASE_VERSIONS("sponsor future reserves", "[tx][sponsorship]") app->getNetworkID(), *root, {root->op(beginSponsoringFutureReserves(a1))}, {}); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, - 0)); - ltx.commit(); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); REQUIRE(getOperationResultCode(*tx, 0) == opNOT_SUPPORTED); }); @@ -63,10 +62,9 @@ TEST_CASE_VERSIONS("sponsor future reserves", "[tx][sponsorship]") app->getNetworkID(), *root, {root->op(beginSponsoringFutureReserves(*root))}, {}); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, - 0)); - ltx.commit(); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); REQUIRE(getBeginSponsoringFutureReservesResultCode(*tx, 0) == BEGIN_SPONSORING_FUTURE_RESERVES_MALFORMED); @@ -251,18 +249,18 @@ TEST_CASE_VERSIONS("sponsor future reserves", "[tx][sponsorship]") auto tx = transactionWithV2Precondition(*app, a1, 1, 100, cond); { - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txBAD_MIN_SEQ_AGE_OR_GAP); } { // this increments ledgerSeq closeLedger(*app); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txSUCCESS); } } diff --git a/src/transactions/test/ChangeTrustTests.cpp b/src/transactions/test/ChangeTrustTests.cpp index 9ca7cbfe05..b08a86e159 100644 --- a/src/transactions/test/ChangeTrustTests.cpp +++ b/src/transactions/test/ChangeTrustTests.cpp @@ -697,12 +697,13 @@ TEST_CASE_VERSIONS("change trust pool share trustline", acc1.op(endSponsoringFutureReserves())}, {acc1}); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); REQUIRE(tx->apply(app->getAppConnector(), ltx, txm)); REQUIRE(tx->getResultCode() == txSUCCESS); ltx.commit(); @@ -729,12 +730,13 @@ TEST_CASE_VERSIONS("change trust pool share trustline", revokeSponsorship(trustlineKey(acc1, tlAsset)))}, {}); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); REQUIRE(!tx->apply(app->getAppConnector(), ltx, txm)); REQUIRE(tx->getResultCode() == txFAILED); @@ -756,12 +758,13 @@ TEST_CASE_VERSIONS("change trust pool share trustline", gateway.op(endSponsoringFutureReserves())}, {acc2}); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); REQUIRE(!tx->apply(app->getAppConnector(), ltx, txm)); REQUIRE(tx->getResultCode() == txFAILED); @@ -782,12 +785,13 @@ TEST_CASE_VERSIONS("change trust pool share trustline", revokeSponsorship(trustlineKey(acc1, tlAsset)))}, {}); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); REQUIRE(tx->apply(app->getAppConnector(), ltx, txm)); REQUIRE(tx->getResultCode() == txSUCCESS); } @@ -806,12 +810,13 @@ TEST_CASE_VERSIONS("change trust pool share trustline", gateway.op(endSponsoringFutureReserves())}, {acc2}); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); REQUIRE(tx->apply(app->getAppConnector(), ltx, txm)); REQUIRE(tx->getResultCode() == txSUCCESS); } @@ -836,12 +841,13 @@ TEST_CASE_VERSIONS("change trust pool share trustline", app->getNetworkID(), *root, {acc1.op(changeTrust(idrUsd, 0))}, {acc1}); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); REQUIRE(tx->apply(app->getAppConnector(), ltx, txm)); ltx.commit(); }; @@ -887,12 +893,13 @@ TEST_CASE_VERSIONS("change trust pool share trustline", acc1.op(endSponsoringFutureReserves())}, {acc1}); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); REQUIRE(tx->apply(app->getAppConnector(), ltx, txm)); ltx.commit(); } diff --git a/src/transactions/test/ClaimableBalanceTests.cpp b/src/transactions/test/ClaimableBalanceTests.cpp index 2e72bd7d25..ec030c82bc 100644 --- a/src/transactions/test/ClaimableBalanceTests.cpp +++ b/src/transactions/test/ClaimableBalanceTests.cpp @@ -175,11 +175,13 @@ validateBalancesOnCreateAndClaim(TestAccount& createAcc, TestAccount& claimAcc, createAcc.op(endSponsoringFutureReserves())}, {createAcc}); + REQUIRE(tx->checkValidForTesting(app.getAppConnector(), + *app.getLedgerManager().getLCLView(), + 0, 0, 0)); LedgerTxn ltx(app.getLedgerTxnRoot()); TransactionMetaBuilder txm(true, *tx, ltx.loadHeader().current().ledgerVersion, app.getAppConnector()); - REQUIRE(tx->checkValidForTesting(app.getAppConnector(), ltx, 0, 0, 0)); REQUIRE(tx->apply(app.getAppConnector(), ltx, txm)); REQUIRE(tx->getResultCode() == txSUCCESS); @@ -236,11 +238,13 @@ validateBalancesOnCreateAndClaim(TestAccount& createAcc, TestAccount& claimAcc, createAcc.op(endSponsoringFutureReserves())}, {createAcc}); + REQUIRE(tx->checkValidForTesting(app.getAppConnector(), + *app.getLedgerManager().getLCLView(), + 0, 0, 0)); LedgerTxn ltx(app.getLedgerTxnRoot()); TransactionMetaBuilder txm(true, *tx, ltx.loadHeader().current().ledgerVersion, app.getAppConnector()); - REQUIRE(tx->checkValidForTesting(app.getAppConnector(), ltx, 0, 0, 0)); REQUIRE(tx->apply(app.getAppConnector(), ltx, txm)); ltx.commit(); @@ -259,11 +263,13 @@ validateBalancesOnCreateAndClaim(TestAccount& createAcc, TestAccount& claimAcc, claimAcc.op(endSponsoringFutureReserves())}, {claimAcc}); + REQUIRE(tx->checkValidForTesting(app.getAppConnector(), + *app.getLedgerManager().getLCLView(), + 0, 0, 0)); LedgerTxn ltx(app.getLedgerTxnRoot()); TransactionMetaBuilder txm(true, *tx, ltx.loadHeader().current().ledgerVersion, app.getAppConnector()); - REQUIRE(tx->checkValidForTesting(app.getAppConnector(), ltx, 0, 0, 0)); REQUIRE(tx->apply(app.getAppConnector(), ltx, txm)); ltx.commit(); @@ -1168,12 +1174,13 @@ TEST_CASE_VERSIONS("claimableBalance", "[tx][claimablebalance]") acc1.op(endSponsoringFutureReserves())}, {acc1}); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm(true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE( - tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, 0)); REQUIRE(tx->apply(app->getAppConnector(), ltx, txm)); REQUIRE(tx->getResultCode() == txSUCCESS); @@ -1188,8 +1195,8 @@ TEST_CASE_VERSIONS("claimableBalance", "[tx][claimablebalance]") TransactionMetaBuilder txm2( true, *tx2, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx2->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, - 0)); + REQUIRE(tx2->checkValidForTesting(app->getAppConnector(), + LedgerTxnView(ltx), 0, 0, 0)); REQUIRE(!tx2->apply(app->getAppConnector(), ltx, txm2)); REQUIRE(tx2->getResultCode() == txFAILED); @@ -1251,12 +1258,13 @@ TEST_CASE_VERSIONS("claimableBalance", "[tx][claimablebalance]") acc2.op(endSponsoringFutureReserves())}, {acc2}); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); REQUIRE(tx->apply(app->getAppConnector(), ltx, txm)); REQUIRE(tx->getResultCode() == txSUCCESS); ltx.commit(); @@ -1283,12 +1291,13 @@ TEST_CASE_VERSIONS("claimableBalance", "[tx][claimablebalance]") app->getNetworkID(), *root, {claimAccount.op(claimOp)}, {claimAccount}); + REQUIRE(tx2->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm2( true, *tx2, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx2->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); REQUIRE(tx2->apply(app->getAppConnector(), ltx, txm2)); REQUIRE(tx2->getResultCode() == txSUCCESS); diff --git a/src/transactions/test/CreateAccountTests.cpp b/src/transactions/test/CreateAccountTests.cpp index d45d173c73..46ddf7d3ec 100644 --- a/src/transactions/test/CreateAccountTests.cpp +++ b/src/transactions/test/CreateAccountTests.cpp @@ -52,14 +52,15 @@ TEST_CASE_VERSIONS("create account", "[tx][createaccount]") app->getNetworkID(), *root, {root->op(createAccount(key.getPublicKey(), 1))}, {}); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx1->checkValidForTesting(app->getAppConnector(), ltx, 0, - 0, 0)); + REQUIRE(!tx1->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); REQUIRE(getCreateAccountResultCode(tx1, 0) == CREATE_ACCOUNT_MALFORMED); - REQUIRE(tx2->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, - 0)); + REQUIRE(tx2->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); }); for_versions_from(14, *app, [&] { @@ -72,14 +73,15 @@ TEST_CASE_VERSIONS("create account", "[tx][createaccount]") app->getNetworkID(), *root, {root->op(createAccount(key.getPublicKey(), 0))}, {}); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx1->checkValidForTesting(app->getAppConnector(), ltx, 0, - 0, 0)); + REQUIRE(!tx1->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); REQUIRE(getCreateAccountResultCode(tx1, 0) == CREATE_ACCOUNT_MALFORMED); - REQUIRE(tx2->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, - 0)); + REQUIRE(tx2->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); }); } @@ -90,9 +92,9 @@ TEST_CASE_VERSIONS("create account", "[tx][createaccount]") app->getNetworkID(), *root, {root->op(createAccount(*root, -1))}, {}); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, - 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); REQUIRE(getCreateAccountResultCode(tx, 0) == CREATE_ACCOUNT_MALFORMED); }); diff --git a/src/transactions/test/EndSponsoringFutureReservesTests.cpp b/src/transactions/test/EndSponsoringFutureReservesTests.cpp index 724778e07d..8c704098a8 100644 --- a/src/transactions/test/EndSponsoringFutureReservesTests.cpp +++ b/src/transactions/test/EndSponsoringFutureReservesTests.cpp @@ -40,9 +40,9 @@ TEST_CASE_VERSIONS("confirm and clear sponsor", "[tx][sponsorship]") app->getNetworkID(), *root, {root->op(endSponsoringFutureReserves())}, {}); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, - 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); REQUIRE(getOperationResultCode(tx, 0) == opNOT_SUPPORTED); }); diff --git a/src/transactions/test/FeeBumpTransactionTests.cpp b/src/transactions/test/FeeBumpTransactionTests.cpp index 00081b3b24..3957bd1223 100644 --- a/src/transactions/test/FeeBumpTransactionTests.cpp +++ b/src/transactions/test/FeeBumpTransactionTests.cpp @@ -96,9 +96,9 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") for_versions({12}, *app, [&] { auto fb = feeBump(app->getNetworkID(), *root, *root, *root, 2 * fee, fee, 1); - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto result = - fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0); + auto result = fb->checkValid( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txNOT_SUPPORTED); REQUIRE(result->getFeeCharged() == 2 * fee); @@ -110,9 +110,9 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") for_versions_from(13, *app, [&] { auto fb = feeBump(app->getNetworkID(), *root, *root, *root, 2 * fee - 1, 1, 1); - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto result = - fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0); + auto result = fb->checkValid( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txINSUFFICIENT_FEE); REQUIRE(result->getFeeCharged() == 2 * fee); @@ -124,9 +124,9 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") for_versions_from(13, *app, [&] { auto fb = feeBump(app->getNetworkID(), *root, *root, *root, 2 * fee + 1, 101, 1); - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto result = - fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0); + auto result = fb->checkValid( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txINSUFFICIENT_FEE); REQUIRE(result->getFeeCharged() == 2 * 101); @@ -139,9 +139,9 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") for_versions_from(13, *app, [&] { auto fb = feeBump(app->getNetworkID(), acc, *root, *root, 2 * fee, fee, 1); - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto result = - fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0); + auto result = fb->checkValid( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txNO_ACCOUNT); REQUIRE(result->getFeeCharged() == 2 * fee); @@ -159,9 +159,9 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") auto fb = TransactionTestFrame::fromTxFrame( TransactionFrameBase::makeTransactionFromWire( app->getNetworkID(), fbXDR)); - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto result = - fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0); + auto result = fb->checkValid( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txBAD_AUTH); REQUIRE(result->getFeeCharged() == 2 * fee); @@ -182,9 +182,9 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") auto fb = TransactionTestFrame::fromTxFrame( TransactionFrameBase::makeTransactionFromWire( app->getNetworkID(), fbXDR)); - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto result = - fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0); + auto result = fb->checkValid( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txBAD_AUTH); REQUIRE(result->getFeeCharged() == 2 * fee); @@ -197,9 +197,9 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") for_versions_from(13, *app, [&] { auto fb = feeBump(app->getNetworkID(), acc, *root, *root, 2 * fee, fee, 1); - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto result = - fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0); + auto result = fb->checkValid( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txINSUFFICIENT_BALANCE); REQUIRE(result->getFeeCharged() == 2 * fee); @@ -219,9 +219,9 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") auto fb = TransactionTestFrame::fromTxFrame( TransactionFrameBase::makeTransactionFromWire( app->getNetworkID(), fbXDR)); - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto result = - fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0); + auto result = fb->checkValid( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txBAD_AUTH_EXTRA); REQUIRE(result->getFeeCharged() == 2 * fee); @@ -238,9 +238,9 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") auto fb = TransactionTestFrame::fromTxFrame( TransactionFrameBase::makeTransactionFromWire( app->getNetworkID(), fbXDR)); - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto result = - fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0); + auto result = fb->checkValid( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txFEE_BUMP_INNER_FAILED); auto const& fbRes = result->getXDR(); @@ -257,9 +257,9 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") for_versions_from(13, *app, [&] { auto fb = feeBump(app->getNetworkID(), acc, *root, *root, 2 * fee, fee, -1); - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto result = - fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0); + auto result = fb->checkValid( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txFEE_BUMP_INNER_FAILED); auto const& fbRes = result->getXDR(); @@ -279,9 +279,9 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") for_versions_from(13, *app, [&] { auto fb = feeBump(app->getNetworkID(), acc, *root, *root, 2 * fee, fee, 1); - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto result = - fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0); + auto result = fb->checkValid( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0); REQUIRE(result->isSuccess()); REQUIRE(result->getResultCode() == txFEE_BUMP_INNER_SUCCESS); auto const& fbRes = result->getXDR(); @@ -329,9 +329,11 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") auto fb = feeBump(app->getNetworkID(), acc, *root, *root, 2 * fee, fee, 1); { - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0) - ->isSuccess()); + REQUIRE( + fb->checkValid(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, + 0, 0) + ->isSuccess()); } MutableTxResultPtr result; @@ -363,9 +365,11 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") auto fb = feeBump(app->getNetworkID(), acc, *root, *root, 2 * fee, fee, 1); { - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0) - ->isSuccess()); + REQUIRE( + fb->checkValid(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, + 0, 0) + ->isSuccess()); } acc.setOptions(setMasterWeight(0)); { @@ -391,9 +395,11 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") auto fb = feeBump(app->getNetworkID(), acc, *root, *root, 2 * fee, fee, 1); { - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0) - ->isSuccess()); + REQUIRE( + fb->checkValid(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, + 0, 0) + ->isSuccess()); } acc.pay(*root, 2 * fee); { @@ -428,9 +434,11 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") app->getNetworkID(), fbXDR); auto fb = TransactionTestFrame::fromTxFrame(rawTx); { - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0) - ->isSuccess()); + REQUIRE( + fb->checkValid(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, + 0, 0) + ->isSuccess()); } auto setOptionsTx = acc.tx({setOptions(setLowThreshold(1))}); @@ -460,9 +468,11 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") auto fb = feeBump(app->getNetworkID(), acc, *root, acc, 2 * fee, fee, 1); { - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0) - ->isSuccess()); + REQUIRE( + fb->checkValid(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, + 0, 0) + ->isSuccess()); } auto setOptionsOp = setOptions(setMasterWeight(0)); @@ -496,9 +506,11 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") auto fb = feeBump(app->getNetworkID(), acc, *root, acc, 2 * fee, fee, INT64_MAX); { - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0) - ->isSuccess()); + REQUIRE( + fb->checkValid(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, + 0, 0) + ->isSuccess()); } { LedgerTxn ltx(app->getLedgerTxnRoot()); @@ -557,12 +569,15 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") acc.op(endSponsoringFutureReserves())}, {sponsoring}); + REQUIRE( + tx->checkValid(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, + 0, 0) + ->isSuccess()); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValid(app->getAppConnector(), ltx, 0, 0, 0) - ->isSuccess()); auto result = tx->processFeeSeqNum(ltx, fee); REQUIRE( tx->apply(app->getAppConnector(), ltx, txm, *result)); @@ -578,9 +593,11 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]") } { - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(fb->checkValid(app->getAppConnector(), ltx, 0, 0, 0) - ->isSuccess()); + REQUIRE( + fb->checkValid(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, + 0, 0) + ->isSuccess()); } { LedgerTxn ltx(app->getLedgerTxnRoot()); diff --git a/src/transactions/test/FrozenLedgerKeysTests.cpp b/src/transactions/test/FrozenLedgerKeysTests.cpp index c5d1166c22..acb85e29a5 100644 --- a/src/transactions/test/FrozenLedgerKeysTests.cpp +++ b/src/transactions/test/FrozenLedgerKeysTests.cpp @@ -97,9 +97,9 @@ bypassAndUnbypassTxHashes(Application& app, std::vector const& toBypass, UnorderedSet loadFrozenKeysFromLedger(Application& app) { - CheckValidLedgerViewWrapper ledgerView(app); + auto ledgerView = app.getLedgerManager().getLCLView(); auto configKey = configSettingKey(CONFIG_SETTING_FROZEN_LEDGER_KEYS); - auto entry = ledgerView.load(configKey); + auto entry = ledgerView->load(configKey); REQUIRE(entry); auto const& frozenKeys = @@ -122,9 +122,9 @@ loadFrozenKeysFromLedger(Application& app) UnorderedSet loadFreezeBypassTxsFromLedger(Application& app) { - CheckValidLedgerViewWrapper ledgerView(app); + auto ledgerView = app.getLedgerManager().getLCLView(); auto configKey = configSettingKey(CONFIG_SETTING_FREEZE_BYPASS_TXS); - auto entry = ledgerView.load(configKey); + auto entry = ledgerView->load(configKey); REQUIRE(entry); auto const& bypassTxs = @@ -158,9 +158,9 @@ TEST_CASE("frozen ledger keys config setting does not exist prior to p26", } auto app = createTestApplication(clock, cfg); auto root = app->getRoot(); - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto configKey = configSettingKey(CONFIG_SETTING_FROZEN_LEDGER_KEYS); - auto entry = ledgerView.load(configKey); + auto entry = ledgerView->load(configKey); REQUIRE(!entry); } @@ -180,9 +180,9 @@ TEST_CASE("freeze bypass txs config setting does not exist prior to p26", } auto app = createTestApplication(clock, cfg); auto root = app->getRoot(); - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto configKey = configSettingKey(CONFIG_SETTING_FREEZE_BYPASS_TXS); - auto entry = ledgerView.load(configKey); + auto entry = ledgerView->load(configKey); REQUIRE(!entry); } @@ -551,9 +551,9 @@ TEST_CASE("freeze bypass tx hash allows frozen key access at validation time", auto checkFrozen = [&](TransactionTestFramePtr& tx, bool expectInnerFrozenResult) { - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto result = - tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0); + tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0); REQUIRE(!result->isSuccess()); if (expectInnerFrozenResult) { @@ -569,9 +569,9 @@ TEST_CASE("freeze bypass tx hash allows frozen key access at validation time", }; auto checkValid = [&](TransactionTestFramePtr& tx) { - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto result = - tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0); + tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0); REQUIRE(result->isSuccess()); }; @@ -723,9 +723,9 @@ TEST_CASE("frozen ledger keys in Soroban footprint", auto tx = createUploadWasmTx(*app, a1, 1000, DEFAULT_TEST_RESOURCE_FEE, resources); - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto result = - tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0); + tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txFROZEN_KEY_ACCESSED); @@ -747,9 +747,9 @@ TEST_CASE("source account frozen", "[frozenledgerkeys][tx]") root->create("A2", lm.getLastMinBalance(10) + 10 * lm.getLastTxFee()); auto checkTx = [&](TransactionTestFramePtr& tx) { - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto result = - tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0); + tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txFROZEN_KEY_ACCESSED); }; @@ -883,9 +883,9 @@ TEST_CASE("source account frozen", "[frozenledgerkeys][tx]") unfreezeKey(*app, a1Key); - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto result = - tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0); + tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0); REQUIRE(result->isSuccess()); } } @@ -920,9 +920,9 @@ TEST_CASE("source trustline frozen", "[frozenledgerkeys][tx]") auto checkAccessesFrozenKey = [&](Operation const& op) { auto tx = transactionFromOperations(*app, a1.getSecretKey(), a1.nextSequenceNumber(), {op}); - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto result = - tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0); + tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txFROZEN_KEY_ACCESSED); }; @@ -1036,9 +1036,9 @@ TEST_CASE("operation destination frozen", "[frozenledgerkeys][tx]") auto tx = transactionFromOperations(*app, sourceAccount.getSecretKey(), sourceAccount.nextSequenceNumber(), {op}); - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto result = - tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0); + tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0); REQUIRE(!result->isSuccess()); REQUIRE(result->getResultCode() == txFROZEN_KEY_ACCESSED); }; @@ -2246,9 +2246,9 @@ TEST_CASE("frozen offers are transparent to DEX matching - randomized", tx->addSignature(taker.getSecretKey()); { - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); auto result = tx->checkValid(app->getAppConnector(), - ledgerView, 0, 0, 0); + *ledgerView, 0, 0, 0); REQUIRE(result->isSuccess()); } diff --git a/src/transactions/test/InvokeHostFunctionTests.cpp b/src/transactions/test/InvokeHostFunctionTests.cpp index db84d1ff21..6f2e2a629e 100644 --- a/src/transactions/test/InvokeHostFunctionTests.cpp +++ b/src/transactions/test/InvokeHostFunctionTests.cpp @@ -1177,9 +1177,9 @@ TEST_CASE("Soroban footprint validation", "[tx][soroban]") MutableTxResultPtr result; { auto diagnostics = DiagnosticEventManager::createDisabled(); - LedgerTxn ltx(test.getApp().getLedgerTxnRoot()); - result = tx->checkValid(test.getApp().getAppConnector(), ltx, 0, - 0, 0, diagnostics); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); + result = tx->checkValid(test.getApp().getAppConnector(), + *ledgerView, 0, 0, 0, diagnostics); } REQUIRE(result->isSuccess() == shouldBeValid); @@ -1195,9 +1195,9 @@ TEST_CASE("Soroban footprint validation", "[tx][soroban]") MutableTxResultPtr result; { auto diagnostics = DiagnosticEventManager::createDisabled(); - LedgerTxn ltx(test.getApp().getLedgerTxnRoot()); - result = tx->checkValid(test.getApp().getAppConnector(), ltx, 0, 0, - 0, diagnostics); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); + result = tx->checkValid(test.getApp().getAppConnector(), + *ledgerView, 0, 0, 0, diagnostics); } REQUIRE(result->isSuccess() == shouldBeValid); if (!shouldBeValid) @@ -1225,9 +1225,9 @@ TEST_CASE("Soroban footprint validation", "[tx][soroban]") MutableTxResultPtr result; { auto diagnostics = DiagnosticEventManager::createDisabled(); - LedgerTxn ltx(test.getApp().getLedgerTxnRoot()); - result = tx->checkValid(test.getApp().getAppConnector(), ltx, 0, 0, - 0, diagnostics); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); + result = tx->checkValid(test.getApp().getAppConnector(), + *ledgerView, 0, 0, 0, diagnostics); } REQUIRE(result->isSuccess() == shouldBeValid); if (!shouldBeValid) @@ -2245,10 +2245,10 @@ TEST_CASE("resource fee exceeds uint32", "[tx][soroban][feebump]") txEnvelope.v1()); auto tx = TransactionFrameBase::makeTransactionFromWire( test.getApp().getNetworkID(), txEnvelope); - CheckValidLedgerViewWrapper ledgerView(test.getApp()); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); auto diagnostics = DiagnosticEventManager::createDisabled(); auto innerCheckValidResult = tx->checkValid( - test.getApp().getAppConnector(), ledgerView, 0, 0, 0, diagnostics); + test.getApp().getAppConnector(), *ledgerView, 0, 0, 0, diagnostics); int64_t feeBumpFullFee = resourceFee + inclusionFee; auto feeBumpTx = feeBump(test.getApp(), feeBumper, tx, feeBumpFullFee, @@ -2257,7 +2257,7 @@ TEST_CASE("resource fee exceeds uint32", "[tx][soroban][feebump]") REQUIRE(feeBumpTx->getInclusionFee() == inclusionFee); auto checkValidResult = feeBumpTx->checkValid( - test.getApp().getAppConnector(), ledgerView, 0, 0, 0, diagnostics); + test.getApp().getAppConnector(), *ledgerView, 0, 0, 0, diagnostics); if (!checkValidResult->isSuccess()) { return checkValidResult->getResultCode(); @@ -2479,9 +2479,9 @@ TEST_CASE("transaction validation diagnostics", "[tx][soroban]") auto diagnosticEvents = DiagnosticEventManager::createForValidation(cfg); { - LedgerTxn ltx(test.getApp().getLedgerTxnRoot()); - auto result = tx->checkValid(test.getApp().getAppConnector(), ltx, 0, 0, - 0, diagnosticEvents); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); + auto result = tx->checkValid(test.getApp().getAppConnector(), + *ledgerView, 0, 0, 0, diagnosticEvents); } REQUIRE(!test.isTxValid(tx)); @@ -9234,8 +9234,8 @@ TEST_CASE_VERSIONS("merge account then SAC payment scenarios", checkTx(1, r, txFAILED); // Verify that a1 no longer exists after the merge - CheckValidLedgerViewWrapper ledgerView(test.getApp()); - REQUIRE(!ledgerView.getAccount(a1.getPublicKey())); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); + REQUIRE(!ledgerView->getAccount(a1.getPublicKey())); // Verify that b1 received a1's balance (minus merge fee) auto expectedBalance = @@ -9266,8 +9266,8 @@ TEST_CASE_VERSIONS("merge account then SAC payment scenarios", checkTx(1, r, txFAILED); // Verify that a1 no longer exists after the merge - CheckValidLedgerViewWrapper ledgerView(test.getApp()); - REQUIRE(!ledgerView.getAccount(a1.getPublicKey())); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); + REQUIRE(!ledgerView->getAccount(a1.getPublicKey())); // Verify that b1 received a1's balance (minus merge fee) auto expectedBalance = @@ -9304,8 +9304,8 @@ TEST_CASE_VERSIONS("merge account then SAC payment scenarios", checkTx(1, r, txNO_ACCOUNT); // Verify that a1 no longer exists after the merge - CheckValidLedgerViewWrapper ledgerView(test.getApp()); - REQUIRE(!ledgerView.getAccount(a1.getPublicKey())); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); + REQUIRE(!ledgerView->getAccount(a1.getPublicKey())); // Verify that b1 received a1's balance (minus the soroban // transactions fee which a1 paid before it was merged). @@ -9566,7 +9566,7 @@ TEST_CASE("apply generated parallel tx sets", "[soroban][parallelapply]") { std::vector sorobanTxs; auto resources = lm.maxLedgerResources(true); - CheckValidLedgerViewWrapper ledgerView(app); + auto ledgerView = app.getLedgerManager().getLCLView(); for (int txId = 0; txId < MAX_TRANSACTIONS_PER_LEDGER; ++txId) { auto account = txtest::getGenesisAccount(app, accountId++); @@ -9607,7 +9607,7 @@ TEST_CASE("apply generated parallel tx sets", "[soroban][parallelapply]") auto tx = invocation.withExactNonRefundableResourceFee().createTx( &account); - REQUIRE(tx->checkValid(app.getAppConnector(), ledgerView, 0, 0, 0) + REQUIRE(tx->checkValid(app.getAppConnector(), *ledgerView, 0, 0, 0) ->isSuccess()); if (!anyGreater(tx->getResources(false, test.getLedgerVersion()), resources)) @@ -9997,8 +9997,8 @@ TEST_CASE("in-memory state size tracking", "[soroban]") { auto ledgerKey = client.getContract().getDataKey( makeSymbolSCVal(key), durability); - CheckValidLedgerViewWrapper ledgerView(test.getApp()); - auto le = ledgerView.load(ledgerKey); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); + auto le = ledgerView->load(ledgerKey); if (le) { // We only deal with the data entries here, so no need to @@ -10210,9 +10210,9 @@ TEST_CASE("readonly ttl bumps across threads and stages", auto startingTTL = test.getTTL(lk); // Capture the TTL entry's lastModifiedLedgerSeq before tx execution - CheckValidLedgerViewWrapper ledgerView(test.getApp()); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); auto ttlKey = getTTLKey(lk); - auto ttlEntry = ledgerView.load(ttlKey); + auto ttlEntry = ledgerView->load(ttlKey); REQUIRE(ttlEntry); uint32_t ttlLastModifiedBeforeTx = ttlEntry.current().lastModifiedLedgerSeq; @@ -10259,9 +10259,9 @@ TEST_CASE("readonly ttl bumps across threads and stages", auto startingTTL = test.getTTL(lk); // Capture the TTL entry's lastModifiedLedgerSeq before tx execution - CheckValidLedgerViewWrapper ledgerView(test.getApp()); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); auto ttlKey = getTTLKey(lk); - auto ttlEntry = ledgerView.load(ttlKey); + auto ttlEntry = ledgerView->load(ttlKey); REQUIRE(ttlEntry); uint32_t ttlLastModifiedBeforeTx = ttlEntry.current().lastModifiedLedgerSeq; @@ -10313,9 +10313,9 @@ TEST_CASE("readonly ttl bumps across threads and stages", auto startingTTL = test.getTTL(lk); // Capture the TTL entry's lastModifiedLedgerSeq before tx execution - CheckValidLedgerViewWrapper ledgerView(test.getApp()); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); auto ttlKey = getTTLKey(lk); - auto ttlEntry = ledgerView.load(ttlKey); + auto ttlEntry = ledgerView->load(ttlKey); REQUIRE(ttlEntry); uint32_t ttlLastModifiedBeforeTx = ttlEntry.current().lastModifiedLedgerSeq; @@ -10704,8 +10704,8 @@ TEST_CASE_VERSIONS("fee bump inner account merged then used as inner account " REQUIRE(innerRes.result.code() == txNO_ACCOUNT); // Verify that innerAccount no longer exists after the merge - CheckValidLedgerViewWrapper ledgerView(test.getApp()); - REQUIRE(!ledgerView.getAccount(innerAccount.getPublicKey())); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); + REQUIRE(!ledgerView->getAccount(innerAccount.getPublicKey())); auto expectedDestinationBalance = startingBalance + startingBalance; REQUIRE(destination.getBalance() == expectedDestinationBalance); @@ -11171,9 +11171,8 @@ TEST_CASE("create and invoke external ref contract", "[tx][soroban]") REQUIRE(isSuccessResult(test.invokeTx(createTx))); { - auto ledgerView = - test.getApp().getLedgerManager().copyImmutableLedgerView(); - auto le = ledgerView.load(contractInstanceKey); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); + auto le = ledgerView->load(contractInstanceKey); REQUIRE(le); REQUIRE(le.current().data.contractData().val.instance().executable == executable); @@ -11238,13 +11237,12 @@ TEST_CASE_VERSIONS("Soroban pre-apply removes pre-auth tx signers", if (sponsored) { - auto ledgerView = - app.getLedgerManager().copyImmutableLedgerView(); + auto ledgerView = app.getLedgerManager().getLCLView(); auto accountEntry = - ledgerView.load(accountKey(account.getPublicKey())); + ledgerView->load(accountKey(account.getPublicKey())); REQUIRE(getNumSponsored(accountEntry.current()) == 1); auto sponsorEntry = - ledgerView.load(accountKey(sponsor.getPublicKey())); + ledgerView->load(accountKey(sponsor.getPublicKey())); REQUIRE(getNumSponsoring(sponsorEntry.current()) == 1); } }; @@ -11321,14 +11319,14 @@ TEST_CASE_VERSIONS("Soroban pre-apply removes pre-auth tx signers", REQUIRE(isSuccessResult( closeLedger(*app, {signerTx}).results.front().result)); { - auto ledgerView = app->getLedgerManager().copyImmutableLedgerView(); + auto ledgerView = app->getLedgerManager().getLCLView(); auto entry = - ledgerView.load(accountKey(sharedSigner.getPublicKey())); + ledgerView->load(accountKey(sharedSigner.getPublicKey())); REQUIRE(entry.current().data.account().signers.size() == SHARED_SIGNER_COUNT); REQUIRE(getNumSponsored(entry.current()) == sponsoredCount); auto sponsorEntry = - ledgerView.load(accountKey(sponsor.getPublicKey())); + ledgerView->load(accountKey(sponsor.getPublicKey())); REQUIRE(getNumSponsoring(sponsorEntry.current()) == sponsoredCount); } @@ -11369,14 +11367,15 @@ TEST_CASE_VERSIONS("Soroban pre-apply removes pre-auth tx signers", REQUIRE(isSuccessResult(resultFor(r, tx))); } - auto ledgerView = app->getLedgerManager().copyImmutableLedgerView(); + auto ledgerView = app->getLedgerManager().getLCLView(); // Every one-time signer is gone for the sharedSigner, and all the // sponsorships are removed. auto sharedEntry = - ledgerView.load(accountKey(sharedSigner.getPublicKey())); + ledgerView->load(accountKey(sharedSigner.getPublicKey())); REQUIRE(sharedEntry.current().data.account().signers.empty()); REQUIRE(getNumSponsored(sharedEntry.current()) == 0); - auto sponsorEntry = ledgerView.load(accountKey(sponsor.getPublicKey())); + auto sponsorEntry = + ledgerView->load(accountKey(sponsor.getPublicKey())); REQUIRE(getNumSponsoring(sponsorEntry.current()) == 0); // Every one-time signer is gone from the tx sources, and all the @@ -11384,13 +11383,13 @@ TEST_CASE_VERSIONS("Soroban pre-apply removes pre-auth tx signers", for (int i = 0; i < txs.size(); ++i) { auto& source = txSources[i]; - auto entry = ledgerView.load(accountKey(source.getPublicKey())); + auto entry = ledgerView->load(accountKey(source.getPublicKey())); REQUIRE(entry.current().data.account().signers.empty()); REQUIRE(getNumSponsored(entry.current()) == 0); if (i >= SHARED_SIGNER_COUNT) { - auto ownSponsorEntry = ledgerView.load(accountKey( + auto ownSponsorEntry = ledgerView->load(accountKey( ownSponsors[i - SHARED_SIGNER_COUNT].getPublicKey())); REQUIRE(getNumSponsoring(ownSponsorEntry.current()) == 0); } diff --git a/src/transactions/test/ManageBuyOfferTests.cpp b/src/transactions/test/ManageBuyOfferTests.cpp index b3bf745968..c5bf309d84 100644 --- a/src/transactions/test/ManageBuyOfferTests.cpp +++ b/src/transactions/test/ManageBuyOfferTests.cpp @@ -373,8 +373,9 @@ TEST_CASE_VERSIONS("manage buy offer liabilities", "[tx][offers]") *app, SecretKey::pseudoRandomForTesting(), 1, {op}); { - LedgerTxn ltx(app->getLedgerTxnRoot()); - tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, 0); + tx->checkValidForTesting(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), + 0, 0, 0); } auto buyOp = std::static_pointer_cast( diff --git a/src/transactions/test/ParallelApplyTest.cpp b/src/transactions/test/ParallelApplyTest.cpp index d7915eda36..89047bb527 100644 --- a/src/transactions/test/ParallelApplyTest.cpp +++ b/src/transactions/test/ParallelApplyTest.cpp @@ -1053,16 +1053,16 @@ applyTestTransactions(TestConfig const& testConfig, uint32_t protocolVersion, auto allTxs = classicTxs; allTxs.insert(allTxs.end(), sorobanTxs.begin(), sorobanTxs.end()); { - CheckValidLedgerViewWrapper ledgerView(test.getApp()); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); auto diag = DiagnosticEventManager::createDisabled(); for (auto const& tx : allTxs) { bool isValid = tx->checkValid(test.getApp().getAppConnector(), - ledgerView, 0, 0, 0, diag) + *ledgerView, 0, 0, 0, diag) ->isSuccess(); if (!isValid) { - tx->checkValid(test.getApp().getAppConnector(), ledgerView, 0, + tx->checkValid(test.getApp().getAppConnector(), *ledgerView, 0, 0, 0, diag); } REQUIRE(isValid); @@ -1121,14 +1121,14 @@ applyTestTransactions(TestConfig const& testConfig, uint32_t protocolVersion, std::vector, std::optional>>> finalEntries; - CheckValidLedgerViewWrapper ledgerView(test.getApp()); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); auto archiveSnap = test.getApp().getLedgerManager().copyImmutableLedgerView(); for (auto const& k : allKeys) { std::optional liveEntry; std::optional archivedEntry; - if (auto e = ledgerView.load(k)) + if (auto e = ledgerView->load(k)) { liveEntry = e.current(); // All the entries that were in the live state and were @@ -1155,7 +1155,7 @@ applyTestTransactions(TestConfig const& testConfig, uint32_t protocolVersion, { LedgerKey ttlKey = getTTLKey(k); std::optional liveTtlEntry; - if (auto e = ledgerView.load(ttlKey)) + if (auto e = ledgerView->load(ttlKey)) { liveTtlEntry = e.current(); } @@ -1494,11 +1494,11 @@ runPreApplyScenario(int64_t seed, int multiplier, size_t preApplyTaskCount) // Validate every transaction manually, as we skip validation due to fixed // Soroban apply order. { - CheckValidLedgerViewWrapper ledgerView(test.getApp()); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); auto diag = DiagnosticEventManager::createDisabled(); for (auto const& tx : txs) { - REQUIRE(tx->checkValid(test.getApp().getAppConnector(), ledgerView, + REQUIRE(tx->checkValid(test.getApp().getAppConnector(), *ledgerView, 0, 0, 0, diag) ->isSuccess()); } @@ -1573,13 +1573,12 @@ runPreApplyScenario(int64_t seed, int multiplier, size_t preApplyTaskCount) observedAccounts.push_back(feeBumper.getPublicKey()); observedAccounts.push_back(root.getPublicKey()); - auto ledgerView = - test.getApp().getLedgerManager().copyImmutableLedgerView(); + auto ledgerView = test.getApp().getLedgerManager().getLCLView(); for (auto const& accountID : observedAccounts) { auto key = accountKey(accountID); std::optional entry; - if (auto e = ledgerView.load(key)) + if (auto e = ledgerView->load(key)) { entry = e.current(); } diff --git a/src/transactions/test/PaymentTests.cpp b/src/transactions/test/PaymentTests.cpp index 3ab8c4ae2d..7fa6768b2e 100644 --- a/src/transactions/test/PaymentTests.cpp +++ b/src/transactions/test/PaymentTests.cpp @@ -1510,11 +1510,11 @@ TEST_CASE_VERSIONS("payment", "[tx][payment]") // Since a1 has a trustline, and there is only 1 trustline, we know // that gateway has no trustlines. - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); LedgerKey trustKey(TRUSTLINE); trustKey.trustLine().accountID = gateway.getPublicKey(); trustKey.trustLine().asset = assetToTrustLineAsset(idr); - REQUIRE(!ledgerView.load(trustKey)); + REQUIRE(!ledgerView->load(trustKey)); }); } SECTION("authorize flag") diff --git a/src/transactions/test/RevokeSponsorshipTests.cpp b/src/transactions/test/RevokeSponsorshipTests.cpp index e03959a16a..1405f54d1b 100644 --- a/src/transactions/test/RevokeSponsorshipTests.cpp +++ b/src/transactions/test/RevokeSponsorshipTests.cpp @@ -1202,12 +1202,9 @@ TEST_CASE_VERSIONS("update sponsorship", "[tx][sponsorship]") app->getNetworkID(), *root, {root->op(revokeSponsorship(trustlineKey(*root, Asset{})))}, {}); - LedgerTxn ltx(app->getLedgerTxnRoot()); - TransactionMetaBuilder txm(true, *tx, - ltx.loadHeader().current().ledgerVersion, - app->getAppConnector()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, - 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); REQUIRE(getRevokeSponsorshipResultCode(tx, 0) == REVOKE_SPONSORSHIP_MALFORMED); }); @@ -1220,12 +1217,9 @@ TEST_CASE_VERSIONS("update sponsorship", "[tx][sponsorship]") auto tx = transactionFrameFromOps( app->getNetworkID(), *root, {root->op(revokeSponsorship(trustlineKey(*root, cur1)))}, {}); - LedgerTxn ltx(app->getLedgerTxnRoot()); - TransactionMetaBuilder txm(true, *tx, - ltx.loadHeader().current().ledgerVersion, - app->getAppConnector()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, - 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); REQUIRE(getRevokeSponsorshipResultCode(tx, 0) == REVOKE_SPONSORSHIP_MALFORMED); }); @@ -1239,10 +1233,9 @@ TEST_CASE_VERSIONS("update sponsorship", "[tx][sponsorship]") app->getNetworkID(), a1, {a1.op(revokeSponsorship(ledgerKey))}, {}); - LedgerTxn ltx(app->getLedgerTxnRoot()); - - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, - 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); REQUIRE(getRevokeSponsorshipResultCode(tx, 0) == REVOKE_SPONSORSHIP_MALFORMED); diff --git a/src/transactions/test/SetTrustLineFlagsTests.cpp b/src/transactions/test/SetTrustLineFlagsTests.cpp index 4d3ce07e8e..7755c87c32 100644 --- a/src/transactions/test/SetTrustLineFlagsTests.cpp +++ b/src/transactions/test/SetTrustLineFlagsTests.cpp @@ -1148,13 +1148,15 @@ TEST_CASE_VERSIONS("revoke from pool", {acc1}); { + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, + 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, 0)); REQUIRE(tx->apply(app->getAppConnector(), ltx, txm)); REQUIRE(tx->getResultCode() == txSUCCESS); @@ -1250,12 +1252,13 @@ TEST_CASE_VERSIONS("revoke from pool", bool trustlineIsSponsored = getNumSponsored(*app, acc1) > 0; { + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); REQUIRE(tx->apply(app->getAppConnector(), ltx, txm) == success); @@ -1518,12 +1521,13 @@ TEST_CASE_VERSIONS("revoke from pool", acc1.op(endSponsoringFutureReserves())}, {acc1}); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); REQUIRE(tx->apply(app->getAppConnector(), ltx, txm)); REQUIRE(tx->getResultCode() == txSUCCESS); @@ -1700,11 +1704,13 @@ TEST_CASE("pool share revocation order test", "[tx][settrustlineflags]") trustor.op(changeTrust(shareAC, INT64_MAX)), trustor.op(endSponsoringFutureReserves())}, {trustor}); + REQUIRE(tx->checkValidForTesting(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), + 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm(true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, 0)); REQUIRE(tx->apply(app->getAppConnector(), ltx, txm)); REQUIRE(tx->getResultCode() == txSUCCESS); ltx.commit(); @@ -1718,11 +1724,13 @@ TEST_CASE("pool share revocation order test", "[tx][settrustlineflags]") trustor.op(changeTrust(shareAD, INT64_MAX)), trustor.op(endSponsoringFutureReserves())}, {trustor}); + REQUIRE(tx->checkValidForTesting(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), + 0, 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm(true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, 0)); REQUIRE(tx->apply(app->getAppConnector(), ltx, txm)); REQUIRE(tx->getResultCode() == txSUCCESS); ltx.commit(); @@ -1763,11 +1771,13 @@ TEST_CASE("pool share revocation order test", "[tx][settrustlineflags]") root->op(revokeOp), sponsor2.op(endSponsoringFutureReserves())}, {sponsor1, sponsor2}); + REQUIRE(tx->checkValidForTesting(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, + 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm(true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, 0)); bool success = tx->apply(app->getAppConnector(), ltx, txm); // PS2 (balance>0, backer=B, sandwich sponsor=Y) first: @@ -1855,11 +1865,13 @@ TEST_CASE("revocation result test across validators", "[tx][settrustlineflags]") trustor.op(changeTrust(share, INT64_MAX)), trustor.op(endSponsoringFutureReserves())}, {trustor}); + tx->checkValidForTesting(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, + 0); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm(true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, 0); tx->apply(app->getAppConnector(), ltx, txm); ltx.commit(); }; @@ -1892,11 +1904,13 @@ TEST_CASE("revocation result test across validators", "[tx][settrustlineflags]") root->op(revokeOp), sponsor2.op(endSponsoringFutureReserves())}, {sponsor1, sponsor2}); + REQUIRE(tx->checkValidForTesting(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, + 0, 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm(true, *tx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, 0)); bool success = tx->apply(app->getAppConnector(), ltx, txm); // A deterministic implementation must always produce the same result. diff --git a/src/transactions/test/SorobanTxTestUtils.cpp b/src/transactions/test/SorobanTxTestUtils.cpp index 7e8b3eace9..182a35160e 100644 --- a/src/transactions/test/SorobanTxTestUtils.cpp +++ b/src/transactions/test/SorobanTxTestUtils.cpp @@ -1109,9 +1109,9 @@ SorobanTest::invokeArchivalOp(TransactionFrameBaseConstPtr tx, MutableTxResultPtr result; { auto diagnostics = DiagnosticEventManager::createDisabled(); - LedgerTxn ltx(getApp().getLedgerTxnRoot()); - result = tx->checkValid(getApp().getAppConnector(), ltx, 0, 0, 0, - diagnostics); + auto ledgerView = getApp().getLedgerManager().getLCLView(); + result = tx->checkValid(getApp().getAppConnector(), *ledgerView, 0, 0, + 0, diagnostics); } REQUIRE(result->isSuccess()); @@ -1383,9 +1383,9 @@ SorobanTest::createRestoreTx(SorobanResources const& resources, bool SorobanTest::isTxValid(TransactionFrameBaseConstPtr tx) { - CheckValidLedgerViewWrapper ledgerView(getApp()); + auto ledgerView = getApp().getLedgerManager().getLCLView(); auto diagnostics = DiagnosticEventManager::createDisabled(); - auto ret = tx->checkValid(getApp().getAppConnector(), ledgerView, 0, 0, 0, + auto ret = tx->checkValid(getApp().getAppConnector(), *ledgerView, 0, 0, 0, diagnostics); return ret->isSuccess(); } @@ -1396,8 +1396,8 @@ SorobanTest::invokeTx(TransactionFrameBaseConstPtr tx) auto diagnostics = DiagnosticEventManager::createForValidation(mApp->getConfig()); { - CheckValidLedgerViewWrapper ledgerView(getApp()); - auto result = tx->checkValid(getApp().getAppConnector(), ledgerView, 0, + auto ledgerView = getApp().getLedgerManager().getLCLView(); + auto result = tx->checkValid(getApp().getAppConnector(), *ledgerView, 0, 0, 0, diagnostics); if (!result->isSuccess()) { @@ -1456,8 +1456,8 @@ ExpirationStatus SorobanTest::getEntryExpirationStatus(LedgerKey const& key) { auto ttlKey = getTTLKey(key); - CheckValidLedgerViewWrapper ledgerView(getApp()); - if (auto le = ledgerView.load(ttlKey)) + auto ledgerView = getApp().getLedgerManager().getLCLView(); + if (auto le = ledgerView->load(ttlKey)) { if (le.current().data.ttl().liveUntilLedgerSeq <= getLCLSeq()) { diff --git a/src/transactions/test/TransactionTestFrame.cpp b/src/transactions/test/TransactionTestFrame.cpp index b239a996f0..9e9e2b117b 100644 --- a/src/transactions/test/TransactionTestFrame.cpp +++ b/src/transactions/test/TransactionTestFrame.cpp @@ -3,6 +3,7 @@ // of this distribution or at http://www.apache.org/licenses/LICENSE-2.0 #include "transactions/test/TransactionTestFrame.h" +#include "main/AppConnector.h" #include "transactions/EventManager.h" #include "transactions/MutableTransactionResult.h" #include "transactions/SignatureUtils.h" @@ -12,6 +13,28 @@ namespace stellar { +namespace +{ +// Some legacy tests are not passing the Soroban config into functions that are +// expected to have access to it, so we load the config from LTX when not +// provided. +// Ideally this should be cleaned up in the future (preferably by getting rid +// of TransactionTestFrame altogether). +std::optional +maybeLoadSorobanConfig( + AbstractLedgerTxn& ltx, + std::optional const& providedConfig) +{ + if (providedConfig || + protocolVersionIsBefore(ltx.loadHeader().current().ledgerVersion, + SOROBAN_PROTOCOL_VERSION)) + { + return providedConfig; + } + return SorobanNetworkConfig::loadFromLedger(ltx); +} +} // namespace + class ThreadParallelApplyLedgerState; TransactionTestFrame::TransactionTestFrame(TransactionFrameBasePtr tx) : mTransactionFrame(tx) @@ -36,8 +59,9 @@ TransactionTestFrame::apply( std::optional const& sorobanConfig, Hash const& sorobanBasePrngSeed) { + auto config = maybeLoadSorobanConfig(ltx, sorobanConfig); return mTransactionFrame->apply(app, ltx, meta, *mTransactionTxResult, - sorobanConfig, sorobanBasePrngSeed); + config, sorobanBasePrngSeed); } void @@ -73,30 +97,16 @@ TransactionTestFrame::apply( std::optional const& sorobanConfig, Hash const& sorobanBasePrngSeed) const { - auto ret = mTransactionFrame->apply(app, ltx, meta, txResult, sorobanConfig, + auto config = maybeLoadSorobanConfig(ltx, sorobanConfig); + auto ret = mTransactionFrame->apply(app, ltx, meta, txResult, config, sorobanBasePrngSeed); mTransactionTxResult = txResult.clone(); return ret; } -MutableTxResultPtr -TransactionTestFrame::checkValid(AppConnector& app, AbstractLedgerTxn& ltxOuter, - SequenceNumber current, - uint64_t lowerBoundCloseTimeOffset, - uint64_t upperBoundCloseTimeOffset) const -{ - LedgerTxn ltx(ltxOuter); - auto ledgerView = CheckValidLedgerViewWrapper(ltx); - auto diagnostics = DiagnosticEventManager::createDisabled(); - mTransactionTxResult = mTransactionFrame->checkValid( - app, ledgerView, current, lowerBoundCloseTimeOffset, - upperBoundCloseTimeOffset, diagnostics); - return mTransactionTxResult->clone(); -} - MutableTxResultPtr TransactionTestFrame::checkValid( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, std::optional validationLedgerSeq) const @@ -109,7 +119,7 @@ TransactionTestFrame::checkValid( MutableTxResultPtr TransactionTestFrame::checkValid( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, DiagnosticEventManager& diagnosticEvents, @@ -123,7 +133,7 @@ TransactionTestFrame::checkValid( MutableTxResultPtr TransactionTestFrame::checkValidForOverlay( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, DiagnosticEventManager& diagnosticEvents, @@ -137,13 +147,13 @@ TransactionTestFrame::checkValidForOverlay( bool TransactionTestFrame::checkValidForTesting(AppConnector& app, - AbstractLedgerTxn& ltxOuter, + AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) { mTransactionTxResult = - checkValid(app, ltxOuter, current, lowerBoundCloseTimeOffset, + checkValid(app, ledgerView, current, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); return mTransactionTxResult->isSuccess(); } @@ -247,8 +257,7 @@ TransactionTestFrame::checkSignature(SignatureChecker& signatureChecker, bool TransactionTestFrame::checkOperationSignatures( - SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, + SignatureChecker& signatureChecker, AbstractLedgerView const& ledgerView, MutableTransactionResultBase* txResult) const { return mTransactionFrame->checkOperationSignatures(signatureChecker, @@ -365,12 +374,11 @@ TransactionTestFrame::insertKeysForTxApply(UnorderedSet& keys) const void TransactionTestFrame::preParallelApplyReadOnly( - AppConnector& app, CheckValidLedgerViewWrapper const& ls, - TransactionMetaBuilder& meta, MutableTransactionResultBase& resPayload, - SorobanNetworkConfig const& sorobanConfig) const + AppConnector& app, AbstractLedgerView const& ls, + TransactionMetaBuilder& meta, + MutableTransactionResultBase& resPayload) const { - mTransactionFrame->preParallelApplyReadOnly(app, ls, meta, resPayload, - sorobanConfig); + mTransactionFrame->preParallelApplyReadOnly(app, ls, meta, resPayload); } void diff --git a/src/transactions/test/TransactionTestFrame.h b/src/transactions/test/TransactionTestFrame.h index 168c0c6990..5fa8cb9123 100644 --- a/src/transactions/test/TransactionTestFrame.h +++ b/src/transactions/test/TransactionTestFrame.h @@ -40,7 +40,8 @@ class TransactionTestFrame : public TransactionFrameBase std::nullopt, Hash const& sorobanBasePrngSeed = Hash{}); - bool checkValidForTesting(AppConnector& app, AbstractLedgerTxn& ltxOuter, + bool checkValidForTesting(AppConnector& app, + AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset); @@ -63,18 +64,13 @@ class TransactionTestFrame : public TransactionFrameBase std::nullopt, Hash const& sorobanBasePrngSeed = Hash{}) const override; - MutableTxResultPtr checkValid(AppConnector& app, - AbstractLedgerTxn& ltxOuter, - SequenceNumber current, - uint64_t lowerBoundCloseTimeOffset, - uint64_t upperBoundCloseTimeOffset) const; MutableTxResultPtr checkValid( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, std::optional validationLedgerSeq = std::nullopt) const; MutableTxResultPtr checkValid(AppConnector& app, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, @@ -82,7 +78,7 @@ class TransactionTestFrame : public TransactionFrameBase std::optional validationLedgerSeq = std::nullopt) const override; MutableTxResultPtr checkValidForOverlay( - AppConnector& app, CheckValidLedgerViewWrapper const& ledgerView, + AppConnector& app, AbstractLedgerView const& ledgerView, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, DiagnosticEventManager& diagnosticEvents, @@ -123,7 +119,7 @@ class TransactionTestFrame : public TransactionFrameBase bool checkOperationSignatures( SignatureChecker& signatureChecker, - CheckValidLedgerViewWrapper const& ledgerView, + AbstractLedgerView const& ledgerView, MutableTransactionResultBase* txResult) const override; bool checkAllTransactionSignatures(SignatureChecker& signatureChecker, @@ -156,9 +152,9 @@ class TransactionTestFrame : public TransactionFrameBase void insertKeysForTxApply(UnorderedSet& keys) const override; void preParallelApplyReadOnly( - AppConnector& app, CheckValidLedgerViewWrapper const& ls, - TransactionMetaBuilder& meta, MutableTransactionResultBase& resPayload, - SorobanNetworkConfig const& sorobanConfig) const override; + AppConnector& app, AbstractLedgerView const& ls, + TransactionMetaBuilder& meta, + MutableTransactionResultBase& resPayload) const override; void preParallelApplyWrite( AppConnector& app, AbstractLedgerTxn& ltx, TransactionMetaBuilder& meta, diff --git a/src/transactions/test/TxEnvelopeTests.cpp b/src/transactions/test/TxEnvelopeTests.cpp index 3058a88cdf..8cb54375c0 100644 --- a/src/transactions/test/TxEnvelopeTests.cpp +++ b/src/transactions/test/TxEnvelopeTests.cpp @@ -825,9 +825,10 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") for_versions(3, 9, *app, [&] { setup(); { - LedgerTxn ltx(app->getLedgerTxnRoot()); REQUIRE(!tx->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, 0)); + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, + 0)); } REQUIRE(tx->getResultCode() == txBAD_SEQ); REQUIRE(getAccountSigners(a1, *app).size() == 1); @@ -1429,13 +1430,15 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") a1.op(endSponsoringFutureReserves())}, {a1}); { + REQUIRE(insideSignerTx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, + 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *insideSignerTx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(insideSignerTx->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, 0)); REQUIRE(insideSignerTx->apply( app->getAppConnector(), ltx, txm)); REQUIRE(insideSignerTx->getResultCode() == @@ -1451,13 +1454,15 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") a1.op(endSponsoringFutureReserves())}, {a1}); { + REQUIRE(outsideSignerTx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, + 0)); LedgerTxn ltx(app->getLedgerTxnRoot()); TransactionMetaBuilder txm( true, *outsideSignerTx, ltx.loadHeader().current().ledgerVersion, app->getAppConnector()); - REQUIRE(outsideSignerTx->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, 0)); REQUIRE(outsideSignerTx->apply( app->getAppConnector(), ltx, txm)); REQUIRE(outsideSignerTx->getResultCode() == @@ -1684,9 +1689,9 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") setFullFee(tx, 1000); { - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); } applyCheck(tx, *app); @@ -1709,9 +1714,10 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") tx->addSignature(a1); { - LedgerTxn ltx(app->getLedgerTxnRoot()); REQUIRE(!tx->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, 0)); + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, + 0)); } applyCheck(tx, *app); REQUIRE(tx->getResultCode() == txFAILED); @@ -1723,9 +1729,10 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") tx->addSignature(a1); { - LedgerTxn ltx(app->getLedgerTxnRoot()); REQUIRE(tx->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, 0)); + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, + 0)); } applyCheck(tx, *app); REQUIRE(tx->getResultCode() == txSUCCESS); @@ -1741,9 +1748,10 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") tx->addSignature(b1); { - LedgerTxn ltx(app->getLedgerTxnRoot()); REQUIRE(tx->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, 0)); + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, + 0)); } applyCheck(tx, *app); REQUIRE(tx->getResultCode() == txSUCCESS); @@ -1767,9 +1775,10 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") tx->addSignature(b1); { - LedgerTxn ltx(app->getLedgerTxnRoot()); REQUIRE(!tx->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, 0)); + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, + 0)); } applyCheck(tx, *app); @@ -1795,9 +1804,10 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") tx->addSignature(b1); { - LedgerTxn ltx(app->getLedgerTxnRoot()); REQUIRE(tx->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, 0)); + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, + 0)); } applyCheck(tx, *app); @@ -1822,9 +1832,10 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") tx->addSignature(b1); { - LedgerTxn ltx(app->getLedgerTxnRoot()); REQUIRE(tx->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, 0)); + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, + 0)); } applyCheck(tx, *app); @@ -1909,9 +1920,9 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") for_versions_to(9, *app, [&] { setup(); { - LedgerTxn ltx(app->getLedgerTxnRoot()); REQUIRE(!txFrame->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, 0)); + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); } REQUIRE(txFrame->getResultCode() == txBAD_SEQ); }); @@ -1998,10 +2009,11 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") getSignatures(txFrame).clear(); txFrame->addSignature(*root); { - LedgerTxn ltx(app->getLedgerTxnRoot()); REQUIRE(txFrame->checkValidForTesting( - app->getAppConnector(), ltx, 0, - lowerBound, + app->getAppConnector(), + *app->getLedgerManager() + .getLCLView(), + 0, lowerBound, 0) == expectSuccess); } REQUIRE( @@ -2080,10 +2092,10 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") txFrame->addSignature(*root); { - LedgerTxn ltx(app->getLedgerTxnRoot()); REQUIRE(txFrame->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, - offset)); + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), + 0, 0, offset)); } REQUIRE(txFrame->getResultCode() == txSUCCESS); @@ -2094,10 +2106,10 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") setMaxTime(txFrame, upperBoundCloseTime - 1); { - LedgerTxn ltx(app->getLedgerTxnRoot()); REQUIRE(!txFrame->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, - offset)); + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), + 0, 0, offset)); } REQUIRE(txFrame->getResultCode() == txTOO_LATE); @@ -2124,9 +2136,9 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") root->tx({payment(a1.getPublicKey(), paymentAmount)}); setSeqNum(txFrame, txFrame->getSeqNum() - 1); { - LedgerTxn ltx(app->getLedgerTxnRoot()); REQUIRE(!txFrame->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, 0)); + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); } REQUIRE(txFrame->getResultCode() == txBAD_SEQ); @@ -2517,10 +2529,9 @@ TEST_CASE_VERSIONS("overlay validation handles ed25519 signed payload signers", SECTION("checkValid accepts ed25519 signed payload signer") { // Normal checkValid should succeed — the payload signer is valid - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto ls = CheckValidLedgerViewWrapper(ltx); + auto ls = app->getLedgerManager().getLCLView(); auto diagnostics = DiagnosticEventManager::createDisabled(); - auto result = tx->checkValid(app->getAppConnector(), ls, 0, 0, 0, + auto result = tx->checkValid(app->getAppConnector(), *ls, 0, 0, 0, diagnostics); REQUIRE(result->isSuccess()); } @@ -2534,10 +2545,9 @@ TEST_CASE_VERSIONS("overlay validation handles ed25519 signed payload signers", SECTION("checkValidForOverlay accepts ed25519 signed payload signer") { - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto ls = CheckValidLedgerViewWrapper(ltx); + auto ls = app->getLedgerManager().getLCLView(); auto diagnostics = DiagnosticEventManager::createDisabled(); - auto result = tx->checkValidForOverlay(app->getAppConnector(), ls, + auto result = tx->checkValidForOverlay(app->getAppConnector(), *ls, 0, 0, 0, diagnostics); REQUIRE(result->isSuccess()); } @@ -2550,21 +2560,19 @@ TEST_CASE_VERSIONS("overlay validation handles ed25519 signed payload signers", SECTION("checkValid accepts fee bump") { - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto ls = CheckValidLedgerViewWrapper(ltx); + auto ls = app->getLedgerManager().getLCLView(); auto diagnostics = DiagnosticEventManager::createDisabled(); - auto result = feeBumpTx->checkValid(app->getAppConnector(), ls, + auto result = feeBumpTx->checkValid(app->getAppConnector(), *ls, 0, 0, 0, diagnostics); REQUIRE(result->isSuccess()); } SECTION("checkValidForOverlay accepts fee bump") { - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto ls = CheckValidLedgerViewWrapper(ltx); + auto ls = app->getLedgerManager().getLCLView(); auto diagnostics = DiagnosticEventManager::createDisabled(); auto result = feeBumpTx->checkValidForOverlay( - app->getAppConnector(), ls, 0, 0, 0, diagnostics); + app->getAppConnector(), *ls, 0, 0, 0, diagnostics); REQUIRE(result->isSuccess()); } } @@ -2586,8 +2594,9 @@ TEST_CASE("soroban txs not allowed before protocol upgrade", auto tx = sorobanTransactionFrameFromOps(app->getNetworkID(), *root, {op}, {}, SorobanResources(), 1000, 1'000'000); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, + 0, 0)); REQUIRE(tx->getResultCode() == txMALFORMED); } TEST_CASE_VERSIONS("Soroban extension for non-Soroban tx", @@ -2607,20 +2616,21 @@ TEST_CASE_VERSIONS("Soroban extension for non-Soroban tx", payment.asset.type(ASSET_TYPE_NATIVE); auto tx = sorobanTransactionFrameFromOps(app->getNetworkID(), *root, {op}, {}, resources, 100, 100); - LedgerTxn ltx(app->getLedgerTxnRoot()); if (protocolVersionStartsFrom(app->getLedgerManager() .getLastClosedLedgerHeader() .header.ledgerVersion, ProtocolVersion::V_21)) { - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, - 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); REQUIRE(tx->getResultCode() == txMALFORMED); } else { - REQUIRE( - tx->checkValidForTesting(app->getAppConnector(), ltx, 0, 0, 0)); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), *app->getLedgerManager().getLCLView(), + 0, 0, 0)); } }); } @@ -2645,9 +2655,10 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = sorobanTransactionFrameFromOps( app->getNetworkID(), *root, {op0}, {}, resources, 100, 3'500'000); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), ltx, 0, - 0, 0) == valid); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, + 0) == valid); if (!valid) { REQUIRE(tx->getResultCode() == txSOROBAN_INVALID); @@ -2658,9 +2669,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") { auto tx = transactionFrameFromOps(app->getNetworkID(), *root, {op0}, {}); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), ltx, - 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txMALFORMED); } SorobanResources resources; @@ -2740,9 +2751,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = sorobanTransactionFrameFromOps( app->getNetworkID(), *root, {op}, {}, resources, 100, 4'000'000); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); } SECTION("limit exceeded") { @@ -2751,9 +2762,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = sorobanTransactionFrameFromOps( app->getNetworkID(), *root, {op}, {}, resources, 100, 4'000'000); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txSOROBAN_INVALID); } } @@ -2764,9 +2775,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = sorobanTransactionFrameFromOpsWithTotalFee( app->getNetworkID(), *root, {op0}, {}, resources, 1'000, 100'000); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txSOROBAN_INVALID); } SECTION("inclusion fee is too low") @@ -2774,9 +2785,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = sorobanTransactionFrameFromOpsWithTotalFee( app->getNetworkID(), *root, {op0}, {}, resources, 1'000'099, 1'000'000); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txINSUFFICIENT_FEE); } SECTION("required resource fee is lower than declared") @@ -2784,9 +2795,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = sorobanTransactionFrameFromOpsWithTotalFee( app->getNetworkID(), *root, {op0}, {}, resources, 1'000'000, 10); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txSOROBAN_INVALID); } SECTION("resource fee is negative") @@ -2794,9 +2805,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = sorobanTransactionFrameFromOpsWithTotalFee( app->getNetworkID(), *root, {op0}, {}, resources, 1'000'000, std::numeric_limits::min()); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); // Negative resource fee is handled before we get to // Soroban-specific checks. REQUIRE(tx->getResultCode() == txMALFORMED); @@ -2809,9 +2820,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") static_cast( std::numeric_limits::max()) + 1); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txSOROBAN_INVALID); } SECTION("resource fee is max int64") @@ -2820,9 +2831,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") app->getNetworkID(), *root, {op0}, {}, resources, std::numeric_limits::max(), std::numeric_limits::max()); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txMALFORMED); } SECTION("total fee is exactly uint32 max") @@ -2833,9 +2844,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") static_cast( std::numeric_limits::max()) - 100); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); } SECTION("total fee exceeds uint32 after adding base fee") { @@ -2845,9 +2856,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") static_cast( std::numeric_limits::max()) - 100 + 1); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); // This gets rejected due to insufficient inclusion fee, so // we have the respective error code (even though the fee is // insufficient due to Soroban resource fee). @@ -2862,9 +2873,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = feeBump(*app, *root, innerTx, 2 * 100 + resourceFee, /* useInclusionAsFullFee */ true); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); } SECTION("resource fee exceeds uint32 with fee bump") { @@ -2874,19 +2885,20 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") std::numeric_limits::max(), resourceFee); auto tx = feeBump(*app, *root, innerTx, resourceFee + 200, /* useInclusionAsFullFee */ true); - LedgerTxn ltx(app->getLedgerTxnRoot()); // This is allowed from protocol 23 - fee bump is sufficient // to cover the inner resource fee. if (protocolVersionStartsFrom(ledgerVersion, ProtocolVersion::V_23)) { - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); } else { REQUIRE(!tx->checkValidForTesting( - app->getAppConnector(), ltx, 0, 0, 0)); + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txFEE_BUMP_INNER_FAILED); } } @@ -2901,9 +2913,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = feeBump(*app, *root, innerTx, resourceFee + 200 - 1, /* useInclusionAsFullFee */ true); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txINSUFFICIENT_FEE); } SECTION("resource fee is negative with fee bump") @@ -2914,9 +2926,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = feeBump(*app, *root, innerTx, std::numeric_limits::max(), /* useInclusionAsFullFee */ true); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txMALFORMED); } SECTION("resource fee is max int64 with fee bump") @@ -2928,9 +2940,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = feeBump(*app, *root, innerTx, std::numeric_limits::max(), /* useInclusionAsFullFee */ true); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txMALFORMED); } } @@ -2940,9 +2952,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = sorobanTransactionFrameFromOps( app->getNetworkID(), *root, {op0, op0}, {}, resources, 100, 100'000); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), ltx, - 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); REQUIRE(tx->getResultCode() == txMALFORMED); } SECTION("contract size") @@ -2958,9 +2970,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = sorobanTransactionFrameFromOps( app->getNetworkID(), *root, {op}, {}, resources, 100, 3'500'000); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); } SECTION("over limit") { @@ -2969,9 +2981,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = sorobanTransactionFrameFromOps( app->getNetworkID(), *root, {op}, {}, resources, 100, 3'500'000); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); } } @@ -3000,9 +3012,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = sorobanTransactionFrameFromOps( app->getNetworkID(), *root, {op}, {}, resources, 100, 3'500'000); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); } auto keyBytes = makeBytes(InitialSorobanNetworkConfig:: @@ -3023,9 +3035,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = sorobanTransactionFrameFromOps( app->getNetworkID(), *root, {op}, {}, resources, 100, 3'500'000); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); } SECTION("read-write key over size limit") { @@ -3042,9 +3054,9 @@ TEST_CASE_VERSIONS("soroban transaction validation", "[tx][envelope][soroban]") auto tx = sorobanTransactionFrameFromOps( app->getNetworkID(), *root, {op}, {}, resources, 100, 3'500'000); - LedgerTxn ltx(app->getLedgerTxnRoot()); - REQUIRE(!tx->checkValidForTesting(app->getAppConnector(), - ltx, 0, 0, 0)); + REQUIRE(!tx->checkValidForTesting( + app->getAppConnector(), + *app->getLedgerManager().getLCLView(), 0, 0, 0)); } } }); @@ -3071,8 +3083,8 @@ TEST_CASE("XDR protocol 22 compatibility validation", "[tx][envelope]") auto tx = sorobanTransactionFrameFromOps(app->getNetworkID(), *root, {op}, {}, SorobanResources(), 1000, 1'000'000); - CheckValidLedgerViewWrapper ledgerView(*app); - return tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0); + auto ledgerView = app->getLedgerManager().getLCLView(); + return tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0); }; SECTION("not valid in protocol 21") { @@ -3104,7 +3116,7 @@ TEST_CASE("XDR protocol 23 compatibility validation", "[tx][envelope]") op.body.invokeHostFunctionOp().hostFunction.type( HOST_FUNCTION_TYPE_INVOKE_CONTRACT); - CheckValidLedgerViewWrapper ledgerView(*app); + auto ledgerView = app->getLedgerManager().getLCLView(); SECTION("muxed account ScAddress in function args") { auto& val = op.body.invokeHostFunctionOp() @@ -3118,7 +3130,7 @@ TEST_CASE("XDR protocol 23 compatibility validation", "[tx][envelope]") 1'000'000); auto res = - tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0); + tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0); REQUIRE(res->isSuccess() == expectSuccess); if (!expectSuccess) { @@ -3139,7 +3151,7 @@ TEST_CASE("XDR protocol 23 compatibility validation", "[tx][envelope]") app->getNetworkID(), *root, {op}, {}, SorobanResources(), 1000, 1'000'000); auto res = - tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0); + tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0); REQUIRE(res->isSuccess() == expectSuccess); if (!expectSuccess) { @@ -3162,7 +3174,7 @@ TEST_CASE("XDR protocol 23 compatibility validation", "[tx][envelope]") {ttlOp}, {}, resources, 1000, 1'000'000); auto res = - tx->checkValid(app->getAppConnector(), ledgerView, 0, 0, 0); + tx->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0); REQUIRE(res->isSuccess() == expectSuccess); if (!expectSuccess) { From ffe54116afd3bee0d16b88c0309c935d9f879078 Mon Sep 17 00:00:00 2001 From: dmkozh Date: Thu, 17 Sep 2026 18:26:03 -0400 Subject: [PATCH 2/2] Parallelize transaction validation for transaction sets. This affects both nominating a new transaction set, and validating the incoming transaction set. Thanks to the fact that most of the time we're either applying the ledger, or validating a transaction set, we can use the efficient CPU-pinned batch executor for this. This speeds up the invalid transaction trimming step by ~20ms on large benchmarks, and in general should increase the transaction validation step proportionally to the number of cores. --- src/herder/TxSetUtils.cpp | 128 +++++++++++++++------ src/herder/test/HerderTests.cpp | 197 +++++++++++++++++++------------- src/herder/test/TxSetTests.cpp | 74 ++++++++++++ 3 files changed, 279 insertions(+), 120 deletions(-) diff --git a/src/herder/TxSetUtils.cpp b/src/herder/TxSetUtils.cpp index 2153393fcf..80df5f4d63 100644 --- a/src/herder/TxSetUtils.cpp +++ b/src/herder/TxSetUtils.cpp @@ -12,11 +12,13 @@ #include "ledger/LedgerTxn.h" #include "ledger/LedgerTxnEntry.h" #include "ledger/LedgerTxnHeader.h" +#include "main/AppConnector.h" #include "main/Application.h" #include "main/Config.h" #include "main/ErrorMessages.h" #include "transactions/MutableTransactionResult.h" #include "transactions/TransactionUtils.h" +#include "util/BatchExecutor.h" #include "util/GlobalChecks.h" #include "util/Logging.h" #include "util/ProtocolVersion.h" @@ -165,12 +167,13 @@ TxSetUtils::buildAccountTxQueues(TxFrameList const& txs) template TxFrameListWithErrors TxSetUtils::getInvalidTxListWithErrors( - T const& txs, Application& app, + T const& inTxs, Application& app, UnorderedMap& accountFeeMap, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) { ZoneScoped; releaseAssert(threadIsMain()); + TxFrameList txs(inTxs.begin(), inTxs.end()); auto ledgerView = app.getLedgerManager().getLCLView(); // Validate minSeqLedgerGap and LedgerBounds against the next ledgerSeq, @@ -184,68 +187,117 @@ TxSetUtils::getInvalidTxListWithErrors( app.getLedgerManager().getLastClosedLedgerNum() + 1; } + // Parallelize transaction validation using the batch executor with + // `taskCount` batches. + auto taskCount = app.getBatchExecutor().preferredTaskCount(); + + // In some cases we might process a transaction set for ledger N while + // applying ledger N, in which case we won't be able to re-use the batch + // executor and thus need to fall back to single-threaded execution. + // This is really an edge case: it requires several tx sets to exist for the + // same ledger (which is rare), and the timing must be very specific for + // processing to happen after the vote and application start. Thus in + // practice we may consider this rare enough to not worry about the + // performance impact. + // NB: Both this method and `isApplying` must happen on the main thread, + // so there is no race condition risk. + if (app.getLedgerManager().isApplying()) + { + taskCount = 1; + } +#ifdef BUILD_TESTS + // In in-memory mode we use a raw LTX in validation, which is not safe to + // share across multiple threads. That's avoidable, but requires changes to + // LTX and validation logic, so it's not worth for fixing this just for + // tests. + if (app.getConfig().MODE_USES_IN_MEMORY_LEDGER) + { + taskCount = 1; + } +#endif + + std::vector> ledgerViews; + ledgerViews.emplace_back(std::move(ledgerView)); + + for (size_t i = 1; i < taskCount; ++i) + { + ledgerViews.emplace_back(app.getLedgerManager().getLCLView()); + } + + std::vector>> txValidationResult( + txs.size()); + auto& appConnector = app.getAppConnector(); + + app.getBatchExecutor().executeBatchOverRanges( + txs.size(), taskCount, + [&appConnector, &txs, &ledgerViews, &txValidationResult, + lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset, + validationLedgerSeq](size_t begin, size_t end, size_t rangeIndex) { + auto const& view = *ledgerViews.at(rangeIndex); + auto const header = view.getLedgerHeader().current(); + auto diagnostics = DiagnosticEventManager::createDisabled(); + for (size_t i = begin; i < end; ++i) + { + auto res = txs[i]->checkValid(appConnector, view, 0, + lowerBoundCloseTimeOffset, + upperBoundCloseTimeOffset, + diagnostics, validationLedgerSeq); + txValidationResult[i].first = res->isSuccess(); + if (!res->isSuccess()) + { + continue; + } + auto feeSource = view.getAccount(txs[i]->getFeeSourceID()); + if (feeSource) + { + txValidationResult[i].second = + getAvailableBalance(header, feeSource.current()); + } + } + }); + TxFrameListWithErrors invalidTxsWithError; auto& [invalidTxs, errorCode] = invalidTxsWithError; errorCode = TxSetValidationResult::VALID; - std::unordered_set seenInvalidTxs; - auto diagnostics = DiagnosticEventManager::createDisabled(); - for (auto const& tx : txs) + for (size_t i = 0; i < txs.size(); ++i) { - auto txResult = tx->checkValid( - app.getAppConnector(), *ledgerView, 0, lowerBoundCloseTimeOffset, - upperBoundCloseTimeOffset, diagnostics, validationLedgerSeq); - if (!txResult->isSuccess()) + auto const& tx = txs[i]; + auto const& [txIsValid, feeSourceBalance] = txValidationResult[i]; + if (!txIsValid) { invalidTxs.emplace_back(tx); - seenInvalidTxs.emplace(tx->getFullHash()); errorCode = TxSetValidationResult::TX_VALIDATION_FAILED; + continue; } - else + int64_t& accFee = accountFeeMap[tx->getFeeSourceID()]; + if (INT64_MAX - accFee < tx->getFullFee()) { - int64_t& accFee = accountFeeMap[tx->getFeeSourceID()]; - if (INT64_MAX - accFee < tx->getFullFee()) - { - accFee = INT64_MAX; - } - else - { - accFee += tx->getFullFee(); - } + accFee = INT64_MAX; } - } - - auto header = ledgerView->getLedgerHeader().current(); - for (auto const& tx : txs) - { - // Already added invalid tx - if (seenInvalidTxs.find(tx->getFullHash()) != seenInvalidTxs.end()) + else { - continue; + accFee += tx->getFullFee(); } - - auto feeSourceID = tx->getFeeSourceID(); - auto feeSource = ledgerView->getAccount(feeSourceID); - // feeSource should exist since we've already run checkValid, log - // internal bug - if (!feeSource) + // `feeSourceBalance` should exist as transaction must be valid, log + // an internal error and skip the transaction otherwise. + if (!feeSourceBalance) { CLOG_ERROR(Herder, "Account not found when checking TxSet validity"); CLOG_ERROR(Herder, "{}", REPORT_INTERNAL_BUG); + invalidTxs.emplace_back(tx); + errorCode = TxSetValidationResult::TX_VALIDATION_FAILED; continue; } - auto it = accountFeeMap.find(feeSourceID); - auto totFee = it->second; - if (getAvailableBalance(header, feeSource.current()) < totFee) + if (*feeSourceBalance < accFee) { invalidTxs.push_back(tx); - // Only override the error code if it wasn't already set + // Only override the error code if it wasn't already set. if (errorCode == TxSetValidationResult::VALID) { errorCode = TxSetValidationResult::ACCOUNT_CANT_PAY_FEE; } - releaseAssert(seenInvalidTxs.insert(tx->getFullHash()).second); CLOG_DEBUG( Herder, "Got bad txSet: account can't pay fee tx: {}", xdrToCerealString(tx->getEnvelope(), "TransactionEnvelope")); diff --git a/src/herder/test/HerderTests.cpp b/src/herder/test/HerderTests.cpp index 69bcb6da0b..c3f0f9b644 100644 --- a/src/herder/test/HerderTests.cpp +++ b/src/herder/test/HerderTests.cpp @@ -734,7 +734,7 @@ testTxSetWithFeeBumps(uint32 protocolVersion) TxFrameList invalidTxs; auto txSet = makeTxSetFromTransactions({fb1, fb2, fb3}, *app, 0, 0, invalidTxs); - compareTxs(invalidTxs, {fb1, fb2, fb3}); + compareTxs(invalidTxs, {fb2, fb3}); } SECTION("validate block") { @@ -777,9 +777,7 @@ testTxSetWithFeeBumps(uint32 protocolVersion) { auto txSet = makeTxSetFromTransactions({fb1, fb2}, *app, 0, 0, invalidTxs); - // Both are marked invalid because their combined fees exceed - // account2's balance - compareTxs(invalidTxs, {fb1, fb2}); + compareTxs(invalidTxs, {fb2}); } SECTION("validate block") { @@ -853,9 +851,7 @@ testTxSetWithFeeBumps(uint32 protocolVersion) { auto txSet = makeTxSetFromTransactions({{}, {fb1, fb2}}, *app, 0, 0, invalidPerPhase); - // Both are marked invalid because their combined fees exceed - // feeSourceAccount's balance - compareTxs(invalidPerPhase[1], {fb1, fb2}); + compareTxs(invalidPerPhase[1], {fb2}); } SECTION("validate block") { @@ -954,91 +950,128 @@ testTxSetWithFeeBumps(uint32 protocolVersion) } } -TEST_CASE("getInvalidTxListWithErrors returns no duplicates") +TEST_CASE("tx set validation rejects txs that fee source cannot pay for", + "[txset]") { Config cfg(getTestConfig()); VirtualClock clock; Application::pointer app = createTestApplication(clock, cfg); - auto const minBalance2 = app->getLedgerManager().getLastMinBalance(2); auto root = app->getRoot(); - // Create accounts for tx sources and fee source - auto account1 = root->create("a1", minBalance2); - auto account2 = root->create("a2", minBalance2); - auto account3 = root->create("a3", minBalance2); - auto account4 = root->create("a4", minBalance2); + auto feeSource = root->create( + "feeSource", app->getLedgerManager().getLastMinBalance(1) * 10); + auto account1 = + root->create("a1", app->getLedgerManager().getLastMinBalance(1) * 20); + auto account2 = + root->create("a2", app->getLedgerManager().getLastMinBalance(1) * 30); auto ledgerView = app->getLedgerManager().getLCLView(); - auto balanceOfFeeSource = getAvailableBalance( + int64_t const feeSourceBalance = getAvailableBalance( ledgerView->getLedgerHeader().current(), - ledgerView->getAccount(account2.getPublicKey()).current()); - - // Create three fee bumps from account2 (fee source): - // - fb1: fails checkValid (bad sequence number) - // - fb2: passes checkValid - // - fb3: passes checkValid - // Combined fees of fb2 + fb3 exceed balance, so both should be invalid - // fb1 is invalid due to checkValid failure - // This tests that fb1 doesn't appear twice (once from checkValid fail, - // once from fee check) - int64_t fee1 = 200; - int64_t fee2 = balanceOfFeeSource / 2 + 100; - int64_t fee3 = balanceOfFeeSource / 2 + 100; - - // fb1: Bad seqNum to ensure it fails checkValid - auto tx1 = transactionFromOperations( - *app, account1, 555, {payment(account1.getPublicKey(), 1)}, 100); - auto fb1 = feeBump(*app, account2, tx1, fee1); - - // fb2 and fb3: Valid transactions - auto tx2 = transactionFromOperations( - *app, account3, account3.getLastSequenceNumber() + 1, - {payment(account3.getPublicKey(), 1)}, 100); - auto fb2 = feeBump(*app, account2, tx2, fee2); - - auto tx3 = transactionFromOperations( - *app, account4, account4.getLastSequenceNumber() + 1, - {payment(account4.getPublicKey(), 1)}, 100); - auto fb3 = feeBump(*app, account2, tx3, fee3); - - // Verify fb1 fails checkValid - inner tx has bad sequence number - auto diagnostics = DiagnosticEventManager::createDisabled(); - REQUIRE(fb1->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0, - diagnostics) - ->getResultCode() == txFEE_BUMP_INNER_FAILED); - // Verify fb2 and fb3 pass checkValid individually - REQUIRE(fb2->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0, - diagnostics) - ->isSuccess()); - REQUIRE(fb3->checkValid(app->getAppConnector(), *ledgerView, 0, 0, 0, - diagnostics) - ->isSuccess()); - - // Verify combined fees of fb2 + fb3 exceed balance - REQUIRE(fb2->getFullFee() + fb3->getFullFee() > balanceOfFeeSource); - // But each individual fee is payable - REQUIRE(fb2->getFullFee() < balanceOfFeeSource); - REQUIRE(fb3->getFullFee() < balanceOfFeeSource); - - TxFrameList txs = {fb1, fb2, fb3}; - UnorderedMap accountFeeMap; - auto invalidTxs = - TxSetUtils::getInvalidTxListWithErrors(txs, *app, accountFeeMap, 0, 0) - .first; - - // Check for no duplicates by comparing size with unique count - std::unordered_set uniqueHashes; - for (auto const& tx : invalidTxs) - { - uniqueHashes.insert(tx->getFullHash()); - } - REQUIRE(invalidTxs.size() == uniqueHashes.size()); - - // All 3 txs should be invalid: - // - fb1: fails checkValid - // - fb2, fb3: can't pay combined fees - REQUIRE(invalidTxs.size() == 3); + ledgerView->getAccount(feeSource.getPublicKey()).current()); + + auto makePayment = [&](TestAccount& source, int64_t fee, + bool isValid = true) { + SequenceNumber seq = isValid ? source.getLastSequenceNumber() + 1 + : source.getLastSequenceNumber() + 100; + return transactionFromOperations(*app, source.getSecretKey(), seq, + {payment(source.getPublicKey(), 1)}, + static_cast(fee)); + }; + auto makeFeeBump = [&](TestAccount& source, int64_t fee, + bool isValid = true) { + return feeBump(*app, feeSource, makePayment(source, 100, isValid), fee); + }; + + // NB: the exact contents of the invalid transaction list are partially an + // implementation detail. Protocol only observes whether the list is + // non-empty, so any implementation that returns at least one invalid + // transaction for every invalid tx set is consistent (as long as it agrees + // on what an 'invalid' transaction is). + // Nomination logic requires `getInvalidTxListWithErrors` to return at least + // the minimum subset of invalid transactions (such that the rest of the + // transactions are valid), but returning more than that is not observable + // by the protocol. + auto checkInvalidTxs = [&](TxFrameList const& txs, + UnorderedMap accountFeeMap, + TxFrameList const& expectedInvalidTxs, + TxSetValidationResult expectedResult) { + auto const [invalidTxs, result] = + TxSetUtils::getInvalidTxListWithErrors(txs, *app, accountFeeMap, 0, + 0); + REQUIRE(result == expectedResult); + REQUIRE(invalidTxs.size() == expectedInvalidTxs.size()); + for (size_t i = 0; i < expectedInvalidTxs.size(); ++i) + { + REQUIRE(invalidTxs[i]->getFullHash() == + expectedInvalidTxs[i]->getFullHash()); + } + }; + + SECTION("txs that fit into the balance are accepted") + { + auto tx1 = makePayment(feeSource, feeSourceBalance / 2); + auto tx2 = makeFeeBump(account1, feeSourceBalance / 2); + checkInvalidTxs({tx1, tx2}, {}, {}, TxSetValidationResult::VALID); + } + + SECTION("tx that is both invalid and can't pay its fee is reported once") + { + auto tx = makePayment(feeSource, feeSourceBalance + 100, + /* isValid */ false); + checkInvalidTxs({tx}, {}, {tx}, + TxSetValidationResult::TX_VALIDATION_FAILED); + } + + SECTION("fee bumps with a shared fee source") + { + auto invalidFb = makeFeeBump(account1, 200, /* isValid */ false); + auto fb1 = makeFeeBump(account1, feeSourceBalance / 2 + 1); + // fb2 doesn't fit into the fee source balance. + auto fb2 = makeFeeBump(account2, feeSourceBalance / 2); + + checkInvalidTxs({fb1, fb2}, {}, {fb2}, + TxSetValidationResult::ACCOUNT_CANT_PAY_FEE); + checkInvalidTxs({invalidFb, fb1, fb2}, {}, {invalidFb, fb2}, + TxSetValidationResult::TX_VALIDATION_FAILED); + checkInvalidTxs({fb2, invalidFb, fb1}, {}, {invalidFb, fb1}, + TxSetValidationResult::TX_VALIDATION_FAILED); + checkInvalidTxs({fb1, fb2, invalidFb}, {}, {fb2, invalidFb}, + TxSetValidationResult::TX_VALIDATION_FAILED); + } + + SECTION("fee bumps mixed with regular txs") + { + auto tx = makePayment(feeSource, feeSourceBalance - 1000); + auto fb = makeFeeBump(account1, 1001); + REQUIRE(tx->getFeeSourceID() == fb->getFeeSourceID()); + + checkInvalidTxs({tx, fb}, {}, {fb}, + TxSetValidationResult::ACCOUNT_CANT_PAY_FEE); + checkInvalidTxs({fb, tx}, {}, {tx}, + TxSetValidationResult::ACCOUNT_CANT_PAY_FEE); + } + + SECTION("account fee map is accounted for") + { + auto tx = makePayment(feeSource, 200); + + SECTION("tx still fits into the balance") + { + UnorderedMap accountFeeMap = { + {feeSource.getPublicKey(), feeSourceBalance - 200}}; + checkInvalidTxs({tx}, accountFeeMap, {}, + TxSetValidationResult::VALID); + } + SECTION("tx no longer fits into the balance") + { + UnorderedMap accountFeeMap = { + {feeSource.getPublicKey(), feeSourceBalance - 199}}; + checkInvalidTxs({tx}, accountFeeMap, {tx}, + TxSetValidationResult::ACCOUNT_CANT_PAY_FEE); + } + } } TEST_CASE("txset", "[herder][txset]") diff --git a/src/herder/test/TxSetTests.cpp b/src/herder/test/TxSetTests.cpp index 6b95e131ca..91fb38bcba 100644 --- a/src/herder/test/TxSetTests.cpp +++ b/src/herder/test/TxSetTests.cpp @@ -3542,5 +3542,79 @@ TEST_CASE("parallel tx set building benchmark", std::cout << "===" << std::endl; } +TEST_CASE("parallel tx set validation matches sequential", "[txset]") +{ + Config cfg(getTestConfig()); + VirtualClock clock; + Application::pointer app = createTestApplication(clock, cfg); + auto root = app->getRoot(); + auto const minBalance = app->getLedgerManager().getLastMinBalance(2); + int const maxTxs = 1000; + + std::vector accounts; + for (int i = 0; i < maxTxs; ++i) + { + accounts.emplace_back( + root->create("account" + std::to_string(i), minBalance * 100)); + } + + auto makeTxs = [&](size_t count, TxFrameList& invalidTxs) { + TxFrameList txs; + for (size_t i = 0; i < count; ++i) + { + auto& account = accounts[i]; + SequenceNumber seq = account.getLastSequenceNumber() + 1; + bool valid = true; + if (rand_flip()) + { + seq += 1000; + valid = false; + } + auto tx = transactionFromOperations( + *app, account.getSecretKey(), seq, + {payment(account.getPublicKey(), static_cast(i) + 1)}, + 100); + txs.emplace_back(tx); + if (!valid) + { + invalidTxs.push_back(tx); + } + } + return txs; + }; + + for (size_t txCount : {0, 1, 2, 3, 4, 5, 6, 7, 500, 1000}) + { + INFO("txCount=" << txCount); + TxFrameList actualInvalidTxs; + auto const txs = makeTxs(txCount, actualInvalidTxs); + + for (size_t taskCount = 1; taskCount <= 8; ++taskCount) + { + INFO("taskCount=" << taskCount); + app->getBatchExecutor().setPreferredTaskCountForTesting(taskCount); + UnorderedMap accountFeeMap; + auto [invalidTxs, validationResult] = + TxSetUtils::getInvalidTxListWithErrors(txs, *app, accountFeeMap, + 0, 0); + if (actualInvalidTxs.empty()) + { + REQUIRE(validationResult == TxSetValidationResult::VALID); + } + else + { + REQUIRE(validationResult == + TxSetValidationResult::TX_VALIDATION_FAILED); + } + REQUIRE(actualInvalidTxs.size() == invalidTxs.size()); + for (size_t i = 0; i < actualInvalidTxs.size(); ++i) + { + REQUIRE(actualInvalidTxs[i]->getFullHash() == + invalidTxs[i]->getFullHash()); + } + } + } +} + } // namespace } // namespace stellar