From 464fa5b3f54022181cd379a12059e5cab60dc2ad Mon Sep 17 00:00:00 2001 From: marta-lokhova Date: Sat, 28 Dec 2024 15:54:32 -0800 Subject: [PATCH] Minor fixes --- src/bucket/BucketListSnapshotBase.cpp | 6 +++--- src/bucket/BucketSnapshotManager.cpp | 4 ++-- src/bucket/test/BucketTestUtils.cpp | 3 ++- src/catchup/CatchupWork.cpp | 2 +- src/catchup/DownloadApplyTxsWork.cpp | 2 +- src/catchup/DownloadApplyTxsWork.h | 4 ++-- src/herder/TransactionQueue.cpp | 2 ++ src/herder/test/HerderTests.cpp | 23 +++++++++++++++-------- src/herder/test/UpgradesTests.cpp | 4 ++-- src/ledger/LedgerManagerImpl.cpp | 6 ++++-- src/ledger/LedgerStateSnapshot.cpp | 5 ++--- src/ledger/LedgerStateSnapshot.h | 9 ++++++--- src/test/TestUtils.cpp | 8 ++++---- src/test/test.cpp | 1 + 14 files changed, 47 insertions(+), 32 deletions(-) diff --git a/src/bucket/BucketListSnapshotBase.cpp b/src/bucket/BucketListSnapshotBase.cpp index 5647a3b429..85a15ddaee 100644 --- a/src/bucket/BucketListSnapshotBase.cpp +++ b/src/bucket/BucketListSnapshotBase.cpp @@ -16,7 +16,8 @@ namespace stellar { template BucketListSnapshot::BucketListSnapshot( - BucketListBase const& bl, std::shared_ptr lastClosed) + BucketListBase const& bl, + std::shared_ptr lastClosed) : mLastClosedLedger(lastClosed) { releaseAssert(threadIsMain()); @@ -142,8 +143,7 @@ BucketLevelSnapshot::BucketLevelSnapshot( template SearchableBucketListSnapshotBase::SearchableBucketListSnapshotBase( BucketSnapshotManager const& snapshotManager) - : mSnapshotManager(snapshotManager) - , mHistoricalSnapshots() + : mSnapshotManager(snapshotManager), mHistoricalSnapshots() { mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots); } diff --git a/src/bucket/BucketSnapshotManager.cpp b/src/bucket/BucketSnapshotManager.cpp index f8086a5a1a..07bea53d98 100644 --- a/src/bucket/BucketSnapshotManager.cpp +++ b/src/bucket/BucketSnapshotManager.cpp @@ -56,7 +56,7 @@ std::shared_ptr BucketSnapshotManager::copySearchableLiveBucketListSnapshot() const { // Can't use std::make_shared due to private constructor - return std::shared_ptr( + return std::shared_ptr( new SearchableLiveBucketListSnapshot(*this)); } @@ -65,7 +65,7 @@ BucketSnapshotManager::copySearchableHotArchiveBucketListSnapshot() const { releaseAssert(mCurrHotArchiveSnapshot); // Can't use std::make_shared due to private constructor - return std::shared_ptr( + return std::shared_ptr( new SearchableHotArchiveBucketListSnapshot(*this)); } diff --git a/src/bucket/test/BucketTestUtils.cpp b/src/bucket/test/BucketTestUtils.cpp index 10e94ca4d1..78f8175218 100644 --- a/src/bucket/test/BucketTestUtils.cpp +++ b/src/bucket/test/BucketTestUtils.cpp @@ -46,7 +46,8 @@ addLiveBatchAndUpdateSnapshot(Application& app, LedgerHeader header, : nullptr; addLiveBatchAndUpdateSnapshot( app, - std::make_shared(lhe, app.getLedgerManager().getLastClosedLedgerHAS(), + std::make_shared( + lhe, app.getLedgerManager().getLastClosedLedgerHAS(), sorobanConfig), initEntries, liveEntries, deadEntries); } diff --git a/src/catchup/CatchupWork.cpp b/src/catchup/CatchupWork.cpp index 796c2f2120..f8aeedfc1b 100644 --- a/src/catchup/CatchupWork.cpp +++ b/src/catchup/CatchupWork.cpp @@ -143,7 +143,7 @@ CatchupWork::doReset() mGetBucketStateWork.reset(); mVerifyTxResults.reset(); mVerifyLedgers.reset(); - mLastApplied = mApp.getLedgerManager().getLastClosedLedgerHeader(); + mLastApplied = lcl; mCurrentWork.reset(); mHAS.reset(); mBucketHAS.reset(); diff --git a/src/catchup/DownloadApplyTxsWork.cpp b/src/catchup/DownloadApplyTxsWork.cpp index bdf924b19a..d06dab6c9a 100644 --- a/src/catchup/DownloadApplyTxsWork.cpp +++ b/src/catchup/DownloadApplyTxsWork.cpp @@ -22,7 +22,7 @@ namespace stellar DownloadApplyTxsWork::DownloadApplyTxsWork( Application& app, TmpDir const& downloadDir, LedgerRange const& range, - std::shared_ptr lastApplied, bool waitForPublish, + std::shared_ptr& lastApplied, bool waitForPublish, std::shared_ptr archive) : BatchWork(app, "download-apply-ledgers") , mRange(range) diff --git a/src/catchup/DownloadApplyTxsWork.h b/src/catchup/DownloadApplyTxsWork.h index be3c7e9818..22136c20f9 100644 --- a/src/catchup/DownloadApplyTxsWork.h +++ b/src/catchup/DownloadApplyTxsWork.h @@ -25,7 +25,7 @@ class DownloadApplyTxsWork : public BatchWork { LedgerRange const mRange; TmpDir const& mDownloadDir; - std::shared_ptr mLastApplied; + std::shared_ptr& mLastApplied; uint32_t mCheckpointToQueue; std::shared_ptr mLastYieldedWork; bool const mWaitForPublish; @@ -34,7 +34,7 @@ class DownloadApplyTxsWork : public BatchWork public: DownloadApplyTxsWork(Application& app, TmpDir const& downloadDir, LedgerRange const& range, - std::shared_ptr lastApplied, + std::shared_ptr& lastApplied, bool waitForPublish, std::shared_ptr archive = nullptr); diff --git a/src/herder/TransactionQueue.cpp b/src/herder/TransactionQueue.cpp index 783c20b763..ce9024ea1f 100644 --- a/src/herder/TransactionQueue.cpp +++ b/src/herder/TransactionQueue.cpp @@ -437,6 +437,8 @@ TransactionQueue::canAdd( ban({tx}); if (canAddRes.second != 0) { + CLOG_INFO(Tx, "Transaction rejected: {} insufficient fee", + hexAbbrev(tx->getFullHash())); AddResult result(TransactionQueue::AddResultCode::ADD_STATUS_ERROR, tx, txINSUFFICIENT_FEE); result.txResult->getResult().feeCharged = canAddRes.second; diff --git a/src/herder/test/HerderTests.cpp b/src/herder/test/HerderTests.cpp index abce3d6d4b..a229c0ff9c 100644 --- a/src/herder/test/HerderTests.cpp +++ b/src/herder/test/HerderTests.cpp @@ -2641,6 +2641,13 @@ TEST_CASE("SCP State", "[herder]") sim->addConnection(nodeIDs[0], nodeIDs[2]); sim->addConnection(nodeIDs[1], nodeIDs[2]); sim->addConnection(nodeIDs[0], nodeIDs[1]); + + REQUIRE(sim->getNode(nodeIDs[0]) + ->getLedgerManager() + .getLastClosedLedgerNum() == expectedLedger); + REQUIRE(sim->getNode(nodeIDs[1]) + ->getLedgerManager() + .getLastClosedLedgerNum() == expectedLedger); }; SECTION("Force SCP") @@ -2693,10 +2700,10 @@ TEST_CASE("SCP State", "[herder]") for (int i = 0; i <= 2; i++) { - auto const& actual = sim->getNode(nodeIDs[i]) - ->getLedgerManager() - .getLastClosedLedgerHeader() - ->header; + auto actual = sim->getNode(nodeIDs[i]) + ->getLedgerManager() + .getLastClosedLedgerHeader() + ->header; REQUIRE(actual == lcl.header); } @@ -3087,10 +3094,10 @@ TEST_CASE("soroban txs each parameter surge priced", "[soroban][herder]") bool hadSorobanSurgePricing = false; simulation->crankUntil( [&]() { - auto& lclHeader = nodes[0] - ->getLedgerManager() - .getLastClosedLedgerHeader() - ->header; + auto lclHeader = nodes[0] + ->getLedgerManager() + .getLastClosedLedgerHeader() + ->header; auto txSet = nodes[0]->getHerder().getTxSet( lclHeader.scpValue.txSetHash); GeneralizedTransactionSet xdrTxSet; diff --git a/src/herder/test/UpgradesTests.cpp b/src/herder/test/UpgradesTests.cpp index 5472032613..10c65a9ca0 100644 --- a/src/herder/test/UpgradesTests.cpp +++ b/src/herder/test/UpgradesTests.cpp @@ -294,8 +294,9 @@ testListUpgrades(VirtualClock::system_time_point preferredUpgradeDatetime, VirtualClock clock; auto app = createTestApplication(clock, cfg); + auto ls = LedgerSnapshot(*app); - auto header = LedgerHeader{}; + auto& header = ls.getLedgerHeader().currentToModify(); header.ledgerVersion = cfg.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION; header.baseFee = cfg.TESTING_UPGRADE_DESIRED_FEE; header.baseReserve = cfg.TESTING_UPGRADE_RESERVE; @@ -309,7 +310,6 @@ testListUpgrades(VirtualClock::system_time_point preferredUpgradeDatetime, makeTxCountUpgrade(cfg.TESTING_UPGRADE_MAX_TX_SET_SIZE); auto baseReserveUpgrade = makeBaseReserveUpgrade(cfg.TESTING_UPGRADE_RESERVE); - auto ls = LedgerSnapshot(*app); SECTION("protocol version upgrade needed") { diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp index 1ea1eee937..cd891f0a9e 100644 --- a/src/ledger/LedgerManagerImpl.cpp +++ b/src/ledger/LedgerManagerImpl.cpp @@ -513,7 +513,8 @@ std::shared_ptr LedgerManagerImpl::getLastClosedLedgerHeader() const { releaseAssert(threadIsMain()); - return std::make_shared(getLastClosedLedger(mApp)->lhhe); + return std::make_shared( + getLastClosedLedger(mApp)->lhhe); } HistoryArchiveState @@ -1299,7 +1300,8 @@ LedgerManagerImpl::advanceReadOnlyLedgerState(LedgerHeader const& header, lhhe.hash = ledgerHash; auto& bm = mApp.getBucketManager(); - auto lcl = std::make_shared(lhhe, getLastClosedLedgerHAS(), mSorobanNetworkConfig); + auto lcl = std::make_shared( + lhhe, getLastClosedLedgerHAS(), mSorobanNetworkConfig); auto liveSnapshot = std::make_unique>( bm.getLiveBucketList(), lcl); auto hotArchiveSnapshot = diff --git a/src/ledger/LedgerStateSnapshot.cpp b/src/ledger/LedgerStateSnapshot.cpp index b50f2c7e9e..a46c1d26b8 100644 --- a/src/ledger/LedgerStateSnapshot.cpp +++ b/src/ledger/LedgerStateSnapshot.cpp @@ -179,9 +179,8 @@ BucketSnapshotState::BucketSnapshotState( "BucketSnapshotState::BucketSnapshotState: snapshot is missing " "last closed ledger"); } - mLedgerHeader = LedgerHeaderWrapper( - std::make_shared(snapshot->getLastClosedLedger() - ->getLedgerHeaderHistoryEntry().header)); + mLedgerHeader = LedgerHeaderWrapper(std::make_shared( + snapshot->getLastClosedLedger()->getLedgerHeaderHistoryEntry().header)); } BucketSnapshotState::~BucketSnapshotState() diff --git a/src/ledger/LedgerStateSnapshot.h b/src/ledger/LedgerStateSnapshot.h index 8433225b9d..347a250853 100644 --- a/src/ledger/LedgerStateSnapshot.h +++ b/src/ledger/LedgerStateSnapshot.h @@ -65,7 +65,8 @@ class AbstractLedgerStateSnapshot { public: virtual ~AbstractLedgerStateSnapshot() = default; - virtual std::shared_ptr getLastClosedLedger() const = 0; + virtual std::shared_ptr + getLastClosedLedger() const = 0; virtual LedgerHeaderWrapper getLedgerHeader() const = 0; virtual LedgerEntryWrapper getAccount(AccountID const& account) const = 0; virtual LedgerEntryWrapper getAccount(LedgerHeaderWrapper const& header, @@ -92,7 +93,8 @@ class LedgerTxnReadOnly : public AbstractLedgerStateSnapshot public: LedgerTxnReadOnly(AbstractLedgerTxn& ltx); ~LedgerTxnReadOnly() override; - std::shared_ptr getLastClosedLedger() const override; + std::shared_ptr + getLastClosedLedger() const override; LedgerHeaderWrapper getLedgerHeader() const override; LedgerEntryWrapper getAccount(AccountID const& account) const override; LedgerEntryWrapper getAccount(LedgerHeaderWrapper const& header, @@ -119,7 +121,8 @@ class BucketSnapshotState : public AbstractLedgerStateSnapshot std::shared_ptr snapshot); ~BucketSnapshotState() override; - std::shared_ptr getLastClosedLedger() const override; + std::shared_ptr + getLastClosedLedger() const override; LedgerHeaderWrapper getLedgerHeader() const override; LedgerEntryWrapper getAccount(AccountID const& account) const override; LedgerEntryWrapper getAccount(LedgerHeaderWrapper const& header, diff --git a/src/test/TestUtils.cpp b/src/test/TestUtils.cpp index e8f5039fbf..bd805a440b 100644 --- a/src/test/TestUtils.cpp +++ b/src/test/TestUtils.cpp @@ -230,9 +230,9 @@ upgradeSorobanNetworkConfig(std::function modifyFn, LoadGenMode::SOROBAN_CREATE_UPGRADE, 1, 1, 1); createUpgradeLoadGenConfig.offset = offset; // Get current network config. - auto cfg = nodes[0]->getLedgerManager().getSorobanNetworkConfig(); - modifyFn(*cfg); - createUpgradeLoadGenConfig.copySorobanNetworkConfigToUpgradeConfig(*cfg); + auto cfg = *(nodes[0]->getLedgerManager().getSorobanNetworkConfig()); + modifyFn(cfg); + createUpgradeLoadGenConfig.copySorobanNetworkConfigToUpgradeConfig(cfg); auto upgradeSetKey = lg.getConfigUpgradeSetKey( createUpgradeLoadGenConfig.getSorobanUpgradeConfig()); lg.generateLoad(createUpgradeLoadGenConfig); @@ -259,7 +259,7 @@ upgradeSorobanNetworkConfig(std::function modifyFn, simulation->crankUntil( [&]() { auto netCfg = app.getLedgerManager().getSorobanNetworkConfig(); - return netCfg == cfg; + return *netCfg == cfg; }, 2 * Herder::EXP_LEDGER_TIMESPAN_SECONDS, false); } diff --git a/src/test/test.cpp b/src/test/test.cpp index d7ea4668e5..e66176b3e3 100644 --- a/src/test/test.cpp +++ b/src/test/test.cpp @@ -845,6 +845,7 @@ saveTestTxMeta(stdfs::path const& dir) } out.exceptions(std::ios::failbit | std::ios::badbit); out << fileRoot; + CLOG_INFO(Tx, "Wrote {} TxMetas to {}", filePair.second.size(), path); } }