diff --git a/src/herder/Herder.h b/src/herder/Herder.h index 4e9c2ee1b6..a6c07ce276 100644 --- a/src/herder/Herder.h +++ b/src/herder/Herder.h @@ -112,7 +112,7 @@ class Herder // restores Herder's state from disk virtual void start() = 0; - virtual void lastClosedLedgerIncreased() = 0; + virtual void lastClosedLedgerIncreased(bool latest) = 0; // Setup Herder's state to fully participate in consensus virtual void setTrackingSCPState(uint64_t index, StellarValue const& value, diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp index 6222b57564..1d7f740252 100644 --- a/src/herder/HerderImpl.cpp +++ b/src/herder/HerderImpl.cpp @@ -81,9 +81,6 @@ HerderImpl::HerderImpl(Application& app) : mTransactionQueue(app, TRANSACTION_QUEUE_TIMEOUT_LEDGERS, TRANSACTION_QUEUE_BAN_LEDGERS, TRANSACTION_QUEUE_SIZE_MULTIPLIER) - , mSorobanTransactionQueue(app, TRANSACTION_QUEUE_TIMEOUT_LEDGERS, - TRANSACTION_QUEUE_BAN_LEDGERS, - SOROBAN_TRANSACTION_QUEUE_SIZE_MULTIPLIER) , mPendingEnvelopes(app, *this) , mHerderSCPDriver(app, *this, mUpgrades, mPendingEnvelopes) , mLastSlotSaved(0) @@ -268,7 +265,10 @@ HerderImpl::shutdown() mLastQuorumMapIntersectionState.mInterruptFlag = true; } mTransactionQueue.shutdown(); - mSorobanTransactionQueue.shutdown(); + if (mSorobanTransactionQueue) + { + mSorobanTransactionQueue->shutdown(); + } mTxSetGarbageCollectTimer.cancel(); } @@ -525,41 +525,38 @@ HerderImpl::recvTransaction(TransactionFrameBasePtr tx, bool submittedFromSelf) ZoneScoped; TransactionQueue::AddResult result; - auto const& lcl = mLedgerManager.getLastClosedLedgerHeader(); - if (protocolVersionStartsFrom(lcl.header.ledgerVersion, - SOROBAN_PROTOCOL_VERSION)) + // Allow txs of the same kind to reach the tx queue in case it can be + // replaced by fee + bool hasSoroban = + mSorobanTransactionQueue && + mSorobanTransactionQueue->sourceAccountPending(tx->getSourceID()) && + !tx->isSoroban(); + bool hasClassic = + mTransactionQueue.sourceAccountPending(tx->getSourceID()) && + tx->isSoroban(); + if (hasSoroban || hasClassic) { - // Allow txs of the same kind to reach the tx queue in case it can be - // replaced by fee - bool hasSoroban = - mSorobanTransactionQueue.sourceAccountPending(tx->getSourceID()) && - !tx->isSoroban(); - bool hasClassic = - mTransactionQueue.sourceAccountPending(tx->getSourceID()) && - tx->isSoroban(); - if (hasSoroban || hasClassic) - { - CLOG_DEBUG( - Herder, - "recv transaction {} for {} rejected due to 1 tx per source " - "account per ledger limit", - hexAbbrev(tx->getFullHash()), - KeyUtils::toShortString(tx->getSourceID())); - result = TransactionQueue::AddResult::ADD_STATUS_TRY_AGAIN_LATER; - } - else if (tx->isSoroban()) - { - result = mSorobanTransactionQueue.tryAdd(tx, submittedFromSelf); - } - else - { - result = mTransactionQueue.tryAdd(tx, submittedFromSelf); - } + CLOG_DEBUG(Herder, + "recv transaction {} for {} rejected due to 1 tx per source " + "account per ledger limit", + hexAbbrev(tx->getFullHash()), + KeyUtils::toShortString(tx->getSourceID())); + result = TransactionQueue::AddResult::ADD_STATUS_TRY_AGAIN_LATER; } - else + else if (!tx->isSoroban()) { result = mTransactionQueue.tryAdd(tx, submittedFromSelf); } + else if (mSorobanTransactionQueue) + { + result = mSorobanTransactionQueue->tryAdd(tx, submittedFromSelf); + } + else + { + // Received Soroban transaction before protocol 20; since this + // transaction isn't supported yet, return ERROR + result = TransactionQueue::AddResult::ADD_STATUS_ERROR; + } if (result == TransactionQueue::AddResult::ADD_STATUS_PENDING) { @@ -839,10 +836,13 @@ HerderImpl::externalizeValue(TxSetFrameConstPtr txSet, uint32_t ledgerSeq, bool HerderImpl::sourceAccountPending(AccountID const& accountID) const { - return mApp.getHerder().getTransactionQueue().sourceAccountPending( - accountID) || - mApp.getHerder().getSorobanTransactionQueue().sourceAccountPending( - accountID); + bool accPending = mTransactionQueue.sourceAccountPending(accountID); + if (mSorobanTransactionQueue) + { + accPending = accPending || + mSorobanTransactionQueue->sourceAccountPending(accountID); + } + return accPending; } #endif @@ -1010,7 +1010,8 @@ HerderImpl::getTransactionQueue() SorobanTransactionQueue& HerderImpl::getSorobanTransactionQueue() { - return mSorobanTransactionQueue; + releaseAssert(mSorobanTransactionQueue); + return *mSorobanTransactionQueue; } #endif @@ -1057,14 +1058,20 @@ HerderImpl::safelyProcessSCPQueue(bool synchronous) } void -HerderImpl::lastClosedLedgerIncreased() +HerderImpl::lastClosedLedgerIncreased(bool latest) { - releaseAssert(isTracking()); - releaseAssert(trackingConsensusLedgerIndex() == - mLedgerManager.getLastClosedLedgerNum()); - releaseAssert(mLedgerManager.isSynced()); + maybeSetupSorobanQueue( + mLedgerManager.getLastClosedLedgerHeader().header.ledgerVersion); - setupTriggerNextLedger(); + if (latest) + { + releaseAssert(isTracking()); + releaseAssert(trackingConsensusLedgerIndex() == + mLedgerManager.getLastClosedLedgerNum()); + releaseAssert(mLedgerManager.isSynced()); + + setupTriggerNextLedger(); + } } void @@ -1212,9 +1219,10 @@ HerderImpl::getMinLedgerSeqToAskPeers() const SequenceNumber HerderImpl::getMaxSeqInPendingTxs(AccountID const& acc) { - if (mSorobanTransactionQueue.sourceAccountPending(acc)) + if (mSorobanTransactionQueue && + mSorobanTransactionQueue->sourceAccountPending(acc)) { - return mSorobanTransactionQueue.getAccountTransactionQueueInfo(acc) + return mSorobanTransactionQueue->getAccountTransactionQueueInfo(acc) .mMaxSeq; } return mTransactionQueue.getAccountTransactionQueueInfo(acc).mMaxSeq; @@ -1279,8 +1287,9 @@ HerderImpl::triggerNextLedger(uint32_t ledgerSeqToTrigger, if (protocolVersionStartsFrom(lcl.header.ledgerVersion, SOROBAN_PROTOCOL_VERSION)) { + releaseAssert(mSorobanTransactionQueue); txPhases.emplace_back( - mSorobanTransactionQueue.getTransactions(lcl.header)); + mSorobanTransactionQueue->getTransactions(lcl.header)); } // We pick as next close time the current time unless it's before the last @@ -1324,7 +1333,8 @@ HerderImpl::triggerNextLedger(uint32_t ledgerSeqToTrigger, if (protocolVersionStartsFrom(lcl.header.ledgerVersion, SOROBAN_PROTOCOL_VERSION)) { - mSorobanTransactionQueue.ban( + releaseAssert(mSorobanTransactionQueue); + mSorobanTransactionQueue->ban( invalidTxPhases[static_cast(TxSetFrame::Phase::SOROBAN)]); } @@ -2022,6 +2032,27 @@ HerderImpl::maybeHandleUpgrade() } } +void +HerderImpl::maybeSetupSorobanQueue(uint32_t protocolVersion) +{ + if (protocolVersionStartsFrom(protocolVersion, SOROBAN_PROTOCOL_VERSION)) + { + if (!mSorobanTransactionQueue) + { + mSorobanTransactionQueue = + std::make_unique( + mApp, TRANSACTION_QUEUE_TIMEOUT_LEDGERS, + TRANSACTION_QUEUE_BAN_LEDGERS, + SOROBAN_TRANSACTION_QUEUE_SIZE_MULTIPLIER); + } + } + else if (mSorobanTransactionQueue) + { + throw std::runtime_error( + "Invalid state: Soroban queue initialized before v20"); + } +} + void HerderImpl::start() { @@ -2031,13 +2062,15 @@ HerderImpl::start() /* shouldUpdateLastModified */ true, TransactionMode::READ_ONLY_WITHOUT_SQL_TXN); - if (protocolVersionStartsFrom(ltx.loadHeader().current().ledgerVersion, - SOROBAN_PROTOCOL_VERSION)) + uint32_t version = ltx.loadHeader().current().ledgerVersion; + if (protocolVersionStartsFrom(version, SOROBAN_PROTOCOL_VERSION)) { auto const& conf = mApp.getLedgerManager().getSorobanNetworkConfig(ltx); mMaxTxSize = std::max(mMaxTxSize, conf.txMaxSizeBytes()); } + + maybeSetupSorobanQueue(version); } auto const& cfg = mApp.getConfig(); @@ -2084,7 +2117,10 @@ HerderImpl::start() // make sure that the transaction queue is setup against // the lcl that we have right now mTransactionQueue.maybeVersionUpgraded(); - mSorobanTransactionQueue.maybeVersionUpgraded(); + if (mSorobanTransactionQueue) + { + mSorobanTransactionQueue->maybeVersionUpgraded(); + } startTxSetGCTimer(); } @@ -2182,9 +2218,13 @@ HerderImpl::updateTransactionQueue(TxSetFrameConstPtr txSet) updateQueue(mTransactionQueue, txSet->getTxsForPhase(TxSetFrame::Phase::CLASSIC)); - if (txSet->numPhases() > static_cast(TxSetFrame::Phase::SOROBAN)) + // Even if we're in protocol 20, still check for number of phases, in case + // we're dealing with the upgrade ledger that contains old-style transaction + // set + if (txSet->numPhases() > static_cast(TxSetFrame::Phase::SOROBAN) && + mSorobanTransactionQueue) { - updateQueue(mSorobanTransactionQueue, + updateQueue(*mSorobanTransactionQueue, txSet->getTxsForPhase(TxSetFrame::Phase::SOROBAN)); } } @@ -2299,23 +2339,29 @@ HerderImpl::getMaxQueueSizeOps() const size_t HerderImpl::getMaxQueueSizeSorobanOps() const { - return mSorobanTransactionQueue.getMaxQueueSizeOps(); + return mSorobanTransactionQueue + ? mSorobanTransactionQueue->getMaxQueueSizeOps() + : 0; } bool HerderImpl::isBannedTx(Hash const& hash) const { - return mTransactionQueue.isBanned(hash) || - mSorobanTransactionQueue.isBanned(hash); + auto banned = mTransactionQueue.isBanned(hash); + if (mSorobanTransactionQueue) + { + banned = banned || mSorobanTransactionQueue->isBanned(hash); + } + return banned; } TransactionFrameBaseConstPtr HerderImpl::getTx(Hash const& hash) const { auto classic = mTransactionQueue.getTx(hash); - if (!classic) + if (!classic && mSorobanTransactionQueue) { - return mSorobanTransactionQueue.getTx(hash); + return mSorobanTransactionQueue->getTx(hash); } return classic; } diff --git a/src/herder/HerderImpl.h b/src/herder/HerderImpl.h index 6a9aea5f94..561a4cf0cd 100644 --- a/src/herder/HerderImpl.h +++ b/src/herder/HerderImpl.h @@ -73,7 +73,7 @@ class HerderImpl : public Herder void start() override; - void lastClosedLedgerIncreased() override; + void lastClosedLedgerIncreased(bool latest) override; SCP& getSCP(); HerderSCPDriver& @@ -230,9 +230,10 @@ class HerderImpl : public Herder void writeDebugTxSet(LedgerCloseData const& lcd); ClassicTransactionQueue mTransactionQueue; - SorobanTransactionQueue mSorobanTransactionQueue; + std::unique_ptr mSorobanTransactionQueue; void updateTransactionQueue(TxSetFrameConstPtr txSet); + void maybeSetupSorobanQueue(uint32_t protocolVersion); PendingEnvelopes mPendingEnvelopes; Upgrades mUpgrades; diff --git a/src/herder/PendingEnvelopes.cpp b/src/herder/PendingEnvelopes.cpp index 65efd75c54..e974becc5c 100644 --- a/src/herder/PendingEnvelopes.cpp +++ b/src/herder/PendingEnvelopes.cpp @@ -60,6 +60,9 @@ PendingEnvelopes::peerDoesntHave(MessageType type, Hash const& itemID, { switch (type) { + // Subtle: it is important to treat both TX_SET and GENERALIZED_TX_SET the + // same way here, since the sending node may have the type wrong depending + // on the protocol version case TX_SET: case GENERALIZED_TX_SET: mTxSetFetcher.doesntHave(itemID, peer); diff --git a/src/herder/TransactionQueue.cpp b/src/herder/TransactionQueue.cpp index 7c5b378371..a0501445f4 100644 --- a/src/herder/TransactionQueue.cpp +++ b/src/herder/TransactionQueue.cpp @@ -1231,14 +1231,6 @@ SorobanTransactionQueue::getMaxResourcesToFloodThisPeriod() const LedgerTxn ltx(mApp.getLedgerTxnRoot(), /* shouldUpdateLastModified */ true, TransactionMode::READ_ONLY_WITHOUT_SQL_TXN); - - // If we're not on the right protocol yet, there's nothing to broadcast - if (protocolVersionIsBefore(ltx.loadHeader().current().ledgerVersion, - SOROBAN_PROTOCOL_VERSION)) - { - return std::make_pair(Resource::makeEmpty(true), std::nullopt); - } - auto sorRes = mApp.getLedgerManager().maxLedgerResources(true, ltx); auto totalFloodPerLedger = multiplyByDouble(sorRes, ratePerLedger); @@ -1307,16 +1299,12 @@ SorobanTransactionQueue::broadcastSome() LedgerTxn ltx(mApp.getLedgerTxnRoot(), true, TransactionMode::READ_ONLY_WITHOUT_SQL_TXN); - if (protocolVersionStartsFrom(ltx.loadHeader().current().ledgerVersion, - SOROBAN_PROTOCOL_VERSION)) + Resource maxPerTx = mApp.getLedgerManager().maxTransactionResources( + /* isSoroban */ true, ltx); + for (auto& resLeft : mBroadcastOpCarryover) { - Resource maxPerTx = mApp.getLedgerManager().maxTransactionResources( - /* isSoroban */ true, ltx); - for (auto& resLeft : mBroadcastOpCarryover) - { - // Limit carry-over to 1 maximum resource transaction - resLeft = limitTo(resLeft, maxPerTx); - } + // Limit carry-over to 1 maximum resource transaction + resLeft = limitTo(resLeft, maxPerTx); } return !totalResToFlood.isZero(); } diff --git a/src/herder/TxSetFrame.cpp b/src/herder/TxSetFrame.cpp index 8697f7ab4f..6ef3ddfd3d 100644 --- a/src/herder/TxSetFrame.cpp +++ b/src/herder/TxSetFrame.cpp @@ -831,7 +831,7 @@ TxSetFrame::computeTxFeesForNonGeneralizedSet(LedgerHeader const& lclHeader, for (auto const& tx : phase) { - mTxBaseFeeClassic[tx] = baseFee; + mTxBaseInclusionFeeClassic[tx] = baseFee; } mFeesComputed[0] = true; } @@ -899,11 +899,11 @@ TxSetFrame::getTxBaseFee(TransactionFrameBaseConstPtr const& tx, releaseAssert(!isGeneralizedTxSet()); computeTxFeesForNonGeneralizedSet(lclHeader); } - auto it = mTxBaseFeeClassic.find(tx); - if (it == mTxBaseFeeClassic.end()) + auto it = mTxBaseInclusionFeeClassic.find(tx); + if (it == mTxBaseInclusionFeeClassic.end()) { - it = mTxBaseFeeSoroban.find(tx); - if (it == mTxBaseFeeSoroban.end()) + it = mTxBaseInclusionFeeSoroban.find(tx); + if (it == mTxBaseInclusionFeeSoroban.end()) { throw std::runtime_error("Transaction not found in tx set"); } @@ -1027,7 +1027,7 @@ TxSetFrame::summary() const { return fmt::format(FMT_STRING("txs:{}, ops:{}, base_fee:{}"), sizeTxTotal(), sizeOpTotal(), - *mTxBaseFeeClassic.begin()->second); + *mTxBaseInclusionFeeClassic.begin()->second); } } @@ -1283,10 +1283,10 @@ TxSetFrame::getInclusionFeeMap(Phase phase) const { if (phase == Phase::SOROBAN) { - return mTxBaseFeeSoroban; + return mTxBaseInclusionFeeSoroban; } releaseAssert(phase == Phase::CLASSIC); - return mTxBaseFeeClassic; + return mTxBaseInclusionFeeClassic; } std::string diff --git a/src/herder/TxSetFrame.h b/src/herder/TxSetFrame.h index f09d7a6d8f..55fcb22432 100644 --- a/src/herder/TxSetFrame.h +++ b/src/herder/TxSetFrame.h @@ -216,10 +216,10 @@ class TxSetFrame : public NonMovableOrCopyable mutable std::vector mFeesComputed; mutable std::unordered_map> - mTxBaseFeeClassic; + mTxBaseInclusionFeeClassic; mutable std::unordered_map> - mTxBaseFeeSoroban; + mTxBaseInclusionFeeSoroban; std::optional getTxSetSorobanResource() const; // Get _inclusion_ fee map for a given phase. The map contains lowest base diff --git a/src/herder/test/HerderTests.cpp b/src/herder/test/HerderTests.cpp index be125048d8..afe4d4a593 100644 --- a/src/herder/test/HerderTests.cpp +++ b/src/herder/test/HerderTests.cpp @@ -3530,6 +3530,103 @@ TEST_CASE("soroban txs each parameter surge priced") } } +TEST_CASE("accept soroban txs after network upgrade") +{ + auto networkID = sha256(getTestConfig().NETWORK_PASSPHRASE); + + auto simulation = + Topologies::core(4, 1, Simulation::OVER_LOOPBACK, networkID, [](int i) { + auto cfg = getTestConfig(i, Config::TESTDB_ON_DISK_SQLITE); + cfg.TESTING_UPGRADE_MAX_TX_SET_SIZE = 100; + cfg.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = + static_cast(SOROBAN_PROTOCOL_VERSION) - 1; + return cfg; + }); + + simulation->startAllNodes(); + auto nodes = simulation->getNodes(); + uint32_t numAccounts = 100; + auto& loadGen = nodes[0]->getLoadGenerator(); + + // Generate some accounts + auto& loadGenDone = + nodes[0]->getMetrics().NewMeter({"loadgen", "run", "complete"}, "run"); + auto currLoadGenCount = loadGenDone.count(); + loadGen.generateLoad( + GeneratedLoadConfig::createAccountsLoad(numAccounts, 1)); + simulation->crankUntil( + [&]() { return loadGenDone.count() > currLoadGenCount; }, + 10 * Herder::EXP_LEDGER_TIMESPAN_SECONDS, false); + + // Ensure more transactions get in the ledger post upgrade + ConfigUpgradeSetFrameConstPtr res; + Upgrades::UpgradeParameters scheduledUpgrades; + scheduledUpgrades.mUpgradeTime = + VirtualClock::from_time_t(nodes[0] + ->getLedgerManager() + .getLastClosedLedgerHeader() + .header.scpValue.closeTime + + 15); + scheduledUpgrades.mProtocolVersion = + static_cast(SOROBAN_PROTOCOL_VERSION); + for (auto const& app : nodes) + { + app->getHerder().setUpgrades(scheduledUpgrades); + } + + auto& secondLoadGen = nodes[1]->getLoadGenerator(); + auto& secondLoadGenDone = + nodes[1]->getMetrics().NewMeter({"loadgen", "run", "complete"}, "run"); + currLoadGenCount = loadGenDone.count(); + auto secondLoadGenCount = secondLoadGenDone.count(); + + // Generate classic txs from another node (with offset to prevent + // overlapping accounts) + secondLoadGen.generateLoad(GeneratedLoadConfig::txLoad(LoadGenMode::PAY, 50, + /* nTxs */ 100, 2, + /* offset */ 50)); + + // Crank a bit and verify that upgrade went through + simulation->crankForAtLeast(4 * Herder::EXP_LEDGER_TIMESPAN_SECONDS, false); + REQUIRE(nodes[0] + ->getLedgerManager() + .getLastClosedLedgerHeader() + .header.ledgerVersion == + static_cast(SOROBAN_PROTOCOL_VERSION)); + + // Now generate Soroban txs + auto sorobanConfig = + GeneratedLoadConfig::txLoad(LoadGenMode::SOROBAN, 50, + /* nTxs */ 15, 1, /* offset */ 0); + sorobanConfig.skipLowFeeTxs = true; + loadGen.generateLoad(sorobanConfig); + + simulation->crankUntil( + [&]() { + return loadGenDone.count() > currLoadGenCount && + secondLoadGenDone.count() > secondLoadGenCount; + }, + 200 * Herder::EXP_LEDGER_TIMESPAN_SECONDS, false); + auto& loadGenFailed = + nodes[0]->getMetrics().NewMeter({"loadgen", "run", "failed"}, "run"); + REQUIRE(loadGenFailed.count() == 0); + auto& secondLoadGenFailed = + nodes[1]->getMetrics().NewMeter({"loadgen", "run", "failed"}, "run"); + REQUIRE(secondLoadGenFailed.count() == 0); + + // Ensure some Soroban txs got into the ledger + auto totalSoroban = + nodes[0] + ->getMetrics() + .NewMeter({"soroban", "host-fn-op", "success"}, "call") + .count() + + nodes[0] + ->getMetrics() + .NewMeter({"soroban", "host-fn-op", "failure"}, "call") + .count(); + REQUIRE(totalSoroban > 0); +} + TEST_CASE("soroban txs accepted by the network", "[herder][soroban][transactionqueue]") { diff --git a/src/herder/test/TransactionQueueTests.cpp b/src/herder/test/TransactionQueueTests.cpp index 9d181ccd1c..92833f668c 100644 --- a/src/herder/test/TransactionQueueTests.cpp +++ b/src/herder/test/TransactionQueueTests.cpp @@ -1089,6 +1089,30 @@ class SorobanLimitingLaneConfigForTesting : public SurgePricingLaneConfig std::vector mLaneOpsLimits; }; +TEST_CASE("Soroban TransactionQueue pre-protocol-20") +{ + VirtualClock clock; + auto cfg = getTestConfig(); + cfg.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = + static_cast(SOROBAN_PROTOCOL_VERSION) - 1; + + auto app = createTestApplication(clock, cfg); + auto root = TestAccount::createRoot(*app); + + SorobanResources resources; + resources.instructions = 2'000'000; + resources.readBytes = 2000; + resources.writeBytes = 1000; + + auto tx = createUploadWasmTx(*app, root, 10'000'000, + DEFAULT_TEST_RESOURCE_FEE, resources); + + // Soroban tx is not supported + REQUIRE(app->getHerder().recvTransaction(tx, false) == + TransactionQueue::AddResult::ADD_STATUS_ERROR); + REQUIRE(app->getHerder().getTx(tx->getFullHash()) == nullptr); +} + TEST_CASE("Soroban TransactionQueue limits", "[herder][transactionqueue][soroban]") { diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp index 38359946fb..5cd8347d30 100644 --- a/src/ledger/LedgerManagerImpl.cpp +++ b/src/ledger/LedgerManagerImpl.cpp @@ -582,19 +582,21 @@ LedgerManagerImpl::valueExternalized(LedgerCloseData const& ledgerData) // We set the state to synced // if we have closed the latest ledger we have heard of. + bool appliedLatest = false; if (cm.getLargestLedgerSeqHeard() == getLastClosedLedgerNum()) { setState(LM_SYNCED_STATE); - // New ledger(s) got closed, notify Herder - if (getLastClosedLedgerNum() > lcl) - { - CLOG_DEBUG(Ledger, - "LedgerManager::valueExternalized LCL advanced {} -> {}", - lcl, getLastClosedLedgerNum()); - mApp.getHerder().lastClosedLedgerIncreased(); - } + appliedLatest = true; } + // New ledger(s) got closed, notify Herder + if (getLastClosedLedgerNum() > lcl) + { + CLOG_DEBUG(Ledger, + "LedgerManager::valueExternalized LCL advanced {} -> {}", + lcl, getLastClosedLedgerNum()); + mApp.getHerder().lastClosedLedgerIncreased(appliedLatest); + } FrameMark; } diff --git a/src/overlay/Peer.cpp b/src/overlay/Peer.cpp index 15e2c2795e..d6ee1387f5 100644 --- a/src/overlay/Peer.cpp +++ b/src/overlay/Peer.cpp @@ -1056,14 +1056,6 @@ Peer::recvGetTxSet(StellarMessage const& msg) StellarMessage newMsg; if (txSet->isGeneralizedTxSet()) { - if (mRemoteOverlayVersion < - Peer::FIRST_VERSION_SUPPORTING_GENERALIZED_TX_SET) - { - // The peer wouldn't be able to accept the generalized tx - // set, but it wouldn't be correct to say we don't have it. - // So we just let the request to timeout. - return; - } newMsg.type(GENERALIZED_TX_SET); txSet->toXDR(newMsg.generalizedTxSet()); } @@ -1089,18 +1081,6 @@ Peer::recvGetTxSet(StellarMessage const& msg) SOROBAN_PROTOCOL_VERSION) ? TX_SET : GENERALIZED_TX_SET; - // If peer is not aware of generalized tx sets and we don't have the - // requested hash, then it probably requests an old-style tx set we - // don't have. Another option is that the peer is in incorrect - // state, but it's also ok to say we don't have the requested - // old-style tx set. - if (messageType == GENERALIZED_TX_SET && - mRemoteOverlayVersion < - Peer::FIRST_VERSION_SUPPORTING_GENERALIZED_TX_SET) - { - sendDontHave(TX_SET, msg.txSetHash()); - return; - } sendDontHave(messageType, msg.txSetHash()); } } @@ -1744,7 +1724,7 @@ Peer::queueTxHashToAdvertise(Hash const& txHash) // Flush adverts at the earliest of the following two conditions: // 1. The number of hashes reaches the threshold. // 2. The oldest tx hash hash been in the queue for FLOOD_TX_PERIOD_MS. - if (mTxHashesToAdvertise.size() == + if (mTxHashesToAdvertise.size() >= mApp.getOverlayManager().getMaxAdvertSize()) { flushAdvert(); diff --git a/src/overlay/Peer.h b/src/overlay/Peer.h index a9ced38ad0..ae4aa06c04 100644 --- a/src/overlay/Peer.h +++ b/src/overlay/Peer.h @@ -57,7 +57,6 @@ class Peer : public std::enable_shared_from_this, { public: - static constexpr uint32_t FIRST_VERSION_SUPPORTING_GENERALIZED_TX_SET = 23; static constexpr std::chrono::seconds PEER_SEND_MODE_IDLE_TIMEOUT = std::chrono::seconds(60); static constexpr std::chrono::nanoseconds PEER_METRICS_DURATION_UNIT = diff --git a/src/overlay/test/OverlayTests.cpp b/src/overlay/test/OverlayTests.cpp index 35fe519a86..6d285d09a3 100644 --- a/src/overlay/test/OverlayTests.cpp +++ b/src/overlay/test/OverlayTests.cpp @@ -2458,70 +2458,6 @@ TEST_CASE("disconnected topology recovery") } } -TEST_CASE("generalized tx sets are not sent to non-upgraded peers", - "[txset][overlay]") -{ - if (protocolVersionIsBefore(Config::CURRENT_LEDGER_PROTOCOL_VERSION, - SOROBAN_PROTOCOL_VERSION)) - { - return; - } - auto runTest = [](bool hasNonUpgraded) { - int const nonUpgradedNodeIndex = 1; - auto networkID = sha256(getTestConfig().NETWORK_PASSPHRASE); - auto simulation = Topologies::core( - 4, 0.75, Simulation::OVER_LOOPBACK, networkID, [&](int i) { - auto cfg = getTestConfig(i, Config::TESTDB_ON_DISK_SQLITE); - cfg.MAX_SLOTS_TO_REMEMBER = 10; - cfg.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = - static_cast(SOROBAN_PROTOCOL_VERSION); - if (hasNonUpgraded && i == nonUpgradedNodeIndex) - { - cfg.OVERLAY_PROTOCOL_VERSION = - Peer::FIRST_VERSION_SUPPORTING_GENERALIZED_TX_SET - 1; - } - return cfg; - }); - - simulation->startAllNodes(); - auto nodeIDs = simulation->getNodeIDs(); - auto node = simulation->getNode(nodeIDs[0]); - - auto root = TestAccount::createRoot(*node); - - int64_t const minBalance = - node->getLedgerManager().getLastMinBalance(0); - REQUIRE(node->getHerder().recvTransaction( - root.tx({txtest::createAccount( - txtest::getAccount("acc").getPublicKey(), minBalance)}), - false) == TransactionQueue::AddResult::ADD_STATUS_PENDING); - simulation->crankForAtLeast(Herder::EXP_LEDGER_TIMESPAN_SECONDS, false); - - for (auto const& nodeID : simulation->getNodeIDs()) - { - auto simNode = simulation->getNode(nodeID); - if (hasNonUpgraded && nodeID == nodeIDs[nonUpgradedNodeIndex]) - { - REQUIRE(simNode->getLedgerManager().getLastClosedLedgerNum() == - 1); - } - else - { - REQUIRE(simNode->getLedgerManager().getLastClosedLedgerNum() == - 2); - } - } - }; - SECTION("all nodes upgraded") - { - runTest(false); - } - SECTION("non upgraded node does not externalize") - { - runTest(true); - } -} - TEST_CASE("overlay pull mode", "[overlay][pullmode]") { VirtualClock clock;