From 6faafccdc273f34be071ff602be2eedbc7a70db4 Mon Sep 17 00:00:00 2001 From: MonsieurNicolas Date: Thu, 9 Dec 2021 09:31:23 -0800 Subject: [PATCH 1/2] add base fee information in logs when closing ledgers --- src/ledger/LedgerManagerImpl.cpp | 13 +++++++------ src/ledger/LedgerManagerImpl.h | 3 ++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp index 240cc210c0..ec23bd60f0 100644 --- a/src/ledger/LedgerManagerImpl.cpp +++ b/src/ledger/LedgerManagerImpl.cpp @@ -658,12 +658,12 @@ LedgerManagerImpl::closeLedger(LedgerCloseData const& ledgerData) // first, prefetch source accounts for txset, then charge fees prefetchTxSourceIds(txs); - processFeesSeqNums(txs, ltx, txSet->getBaseFee(header.current()), - ledgerCloseMeta); + auto curBaseFee = txSet->getBaseFee(header.current()); + processFeesSeqNums(txs, ltx, curBaseFee, ledgerCloseMeta); TransactionResultSet txResultSet; txResultSet.results.reserve(txs.size()); - applyTransactions(txs, ltx, txResultSet, ledgerCloseMeta); + applyTransactions(txs, ltx, txResultSet, ledgerCloseMeta, curBaseFee); ltx.loadHeader().current().txSetResultHash = xdrSha256(txResultSet); @@ -1084,7 +1084,7 @@ void LedgerManagerImpl::applyTransactions( std::vector& txs, AbstractLedgerTxn& ltx, TransactionResultSet& txResultSet, - std::unique_ptr const& ledgerCloseMeta) + std::unique_ptr const& ledgerCloseMeta, int64 curBaseFee) { ZoneNamedN(txsZone, "applyTransactions", true); int index = 0; @@ -1103,8 +1103,9 @@ LedgerManagerImpl::applyTransactions( }); mOperationCount.Update(static_cast(numOps)); TracyPlot("ledger.operation.count", static_cast(numOps)); - CLOG_INFO(Tx, "applying ledger {} (txs:{}, ops:{})", - ltx.loadHeader().current().ledgerSeq, numTxs, numOps); + CLOG_INFO(Tx, "applying ledger {} (txs:{}, ops:{}, base_fee:{})", + ltx.loadHeader().current().ledgerSeq, numTxs, numOps, + curBaseFee); } prefetchTransactionData(txs); diff --git a/src/ledger/LedgerManagerImpl.h b/src/ledger/LedgerManagerImpl.h index 812bd46157..bcf9994ff6 100644 --- a/src/ledger/LedgerManagerImpl.h +++ b/src/ledger/LedgerManagerImpl.h @@ -71,7 +71,8 @@ class LedgerManagerImpl : public LedgerManager void applyTransactions(std::vector& txs, AbstractLedgerTxn& ltx, TransactionResultSet& txResultSet, - std::unique_ptr const& ledgerCloseMeta); + std::unique_ptr const& ledgerCloseMeta, + int64 curBaseFee); void ledgerClosed(AbstractLedgerTxn& ltx); From 577bef4c56d8dd26c6554b3c45f624ff1d11e4aa Mon Sep 17 00:00:00 2001 From: MonsieurNicolas Date: Thu, 9 Dec 2021 09:32:28 -0800 Subject: [PATCH 2/2] txQueue: ban transactions when we drop them late in the pipeline --- src/herder/TransactionQueue.cpp | 40 ++++++++++++++++++++------------- src/herder/TransactionQueue.h | 8 ++++++- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/herder/TransactionQueue.cpp b/src/herder/TransactionQueue.cpp index 6ec98093f4..5ac15116fa 100644 --- a/src/herder/TransactionQueue.cpp +++ b/src/herder/TransactionQueue.cpp @@ -844,12 +844,12 @@ TransactionQueue::getMaxOpsToFloodThisPeriod() const return static_cast(opsToFlood); } -bool +TransactionQueue::BroadcastStatus TransactionQueue::broadcastTx(AccountState& state, TimestampedTx& tx) { if (tx.mBroadcasted) { - return false; + return BroadcastStatus::BROADCAST_STATUS_ALREADY; } bool allowTx{true}; @@ -933,10 +933,11 @@ TransactionQueue::broadcastTx(AccountState& state, TimestampedTx& tx) // false to our caller so that they will not count this tx against the // per-timeslice counters -- we want to allow the caller to try useful // work from other sources. - return false; + return BroadcastStatus::BROADCAST_STATUS_SKIPPED; } - return mApp.getOverlayManager().broadcastMessage( - tx.mTx->toStellarMessage()); + return mApp.getOverlayManager().broadcastMessage(tx.mTx->toStellarMessage()) + ? BroadcastStatus::BROADCAST_STATUS_SUCCESS + : BroadcastStatus::BROADCAST_STATUS_ALREADY; } struct TxQueueTracker @@ -1012,6 +1013,7 @@ TransactionQueue::broadcastSome() } } + std::vector banningTxs; while (opsToFlood != 0 && !iters.empty()) { auto curTracker = iters.top(); @@ -1027,22 +1029,30 @@ TransactionQueue::broadcastSome() // accumulate more flooding credit break; } + auto bStatus = broadcastTx(*curTracker.mAccountState, *cur); + if (bStatus == BroadcastStatus::BROADCAST_STATUS_SUCCESS) + { + auto ops = tx->getNumOperations(); + opsToFlood -= ops; + totalOpsToFlood -= ops; + } + // when skipping, we ban the transaction and skip the remainder of the + // queue + if (bStatus == BroadcastStatus::BROADCAST_STATUS_SKIPPED) + { + banningTxs.emplace_back(tx); + } else { - if (broadcastTx(*curTracker.mAccountState, *cur)) + cur++; + // if we're not done with this account, add the tracker back + if (curTracker.skipToFirstNotBroadcasted()) { - auto ops = tx->getNumOperations(); - opsToFlood -= ops; - totalOpsToFlood -= ops; + iters.push(curTracker); } - cur++; - } - // if we're not done with this account, add the tracker back - if (curTracker.skipToFirstNotBroadcasted()) - { - iters.push(curTracker); } } + ban(banningTxs); // carry over remainder, up to MAX_OPS_PER_TX ops // reason is that if we add 1 next round, we can flood a "worst case fee // bump" tx diff --git a/src/herder/TransactionQueue.h b/src/herder/TransactionQueue.h index 9e70544fd1..c836ce683b 100644 --- a/src/herder/TransactionQueue.h +++ b/src/herder/TransactionQueue.h @@ -198,7 +198,13 @@ class TransactionQueue bool broadcastSome(); void broadcast(bool fromCallback); // broadcasts a single transaction - bool broadcastTx(AccountState& state, TimestampedTx& tx); + enum class BroadcastStatus + { + BROADCAST_STATUS_ALREADY, + BROADCAST_STATUS_SUCCESS, + BROADCAST_STATUS_SKIPPED + }; + BroadcastStatus broadcastTx(AccountState& state, TimestampedTx& tx); AddResult canAdd(TransactionFrameBasePtr tx, AccountStates::iterator& stateIter,