diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp index 81cc5fd74f..e91a162da8 100644 --- a/src/herder/HerderImpl.cpp +++ b/src/herder/HerderImpl.cpp @@ -84,10 +84,7 @@ HerderImpl::SCPMetrics::SCPMetrics(Application& app) } HerderImpl::HerderImpl(Application& app) - : mTransactionQueue(app, TRANSACTION_QUEUE_TIMEOUT_LEDGERS, - TRANSACTION_QUEUE_BAN_LEDGERS, - TRANSACTION_QUEUE_SIZE_MULTIPLIER) - , mPendingEnvelopes(app, *this) + : mPendingEnvelopes(app, *this) , mHerderSCPDriver(app, *this, mUpgrades, mPendingEnvelopes) , mLastSlotSaved(0) , mTrackingTimer(app) @@ -280,7 +277,10 @@ HerderImpl::shutdown() "Shutdown interrupting quorum transitive closure analysis."); mLastQuorumMapIntersectionState.mInterruptFlag = true; } - mTransactionQueue.shutdown(); + if (mTransactionQueue) + { + mTransactionQueue->shutdown(); + } if (mSorobanTransactionQueue) { mSorobanTransactionQueue->shutdown(); @@ -594,7 +594,7 @@ HerderImpl::recvTransaction(TransactionFrameBasePtr tx, bool submittedFromSelf) mSorobanTransactionQueue->sourceAccountPending(tx->getSourceID()) && !tx->isSoroban(); bool hasClassic = - mTransactionQueue.sourceAccountPending(tx->getSourceID()) && + mTransactionQueue->sourceAccountPending(tx->getSourceID()) && tx->isSoroban(); if (hasSoroban || hasClassic) { @@ -608,11 +608,31 @@ HerderImpl::recvTransaction(TransactionFrameBasePtr tx, bool submittedFromSelf) } else if (!tx->isSoroban()) { - result = mTransactionQueue.tryAdd(tx, submittedFromSelf); + if (mApp.getConfig().BACKGROUND_TX_QUEUE && !submittedFromSelf) + { + mApp.postOnOverlayThread( + [this, tx]() { mTransactionQueue->tryAdd(tx, false); }, + "try add tx"); + result.code = TransactionQueue::AddResultCode::ADD_STATUS_UNKNOWN; + } + else + { + result = mTransactionQueue->tryAdd(tx, submittedFromSelf); + } } else if (mSorobanTransactionQueue) { - result = mSorobanTransactionQueue->tryAdd(tx, submittedFromSelf); + if (mApp.getConfig().BACKGROUND_TX_QUEUE && !submittedFromSelf) + { + mApp.postOnOverlayThread( + [this, tx]() { mSorobanTransactionQueue->tryAdd(tx, false); }, + "try add tx"); + result.code = TransactionQueue::AddResultCode::ADD_STATUS_UNKNOWN; + } + else + { + result = mSorobanTransactionQueue->tryAdd(tx, submittedFromSelf); + } } else { @@ -914,7 +934,7 @@ HerderImpl::externalizeValue(TxSetXDRFrameConstPtr txSet, uint32_t ledgerSeq, bool HerderImpl::sourceAccountPending(AccountID const& accountID) const { - bool accPending = mTransactionQueue.sourceAccountPending(accountID); + bool accPending = mTransactionQueue->sourceAccountPending(accountID); if (mSorobanTransactionQueue) { accPending = accPending || @@ -1083,7 +1103,7 @@ HerderImpl::getPendingEnvelopes() ClassicTransactionQueue& HerderImpl::getTransactionQueue() { - return mTransactionQueue; + return *mTransactionQueue; } SorobanTransactionQueue& HerderImpl::getSorobanTransactionQueue() @@ -1351,7 +1371,7 @@ HerderImpl::triggerNextLedger(uint32_t ledgerSeqToTrigger, // during last few ledger closes auto const& lcl = mLedgerManager.getLastClosedLedgerHeader(); PerPhaseTransactionList txPhases; - txPhases.emplace_back(mTransactionQueue.getTransactions(lcl.header)); + txPhases.emplace_back(mTransactionQueue->getTransactions(lcl.header)); if (protocolVersionStartsFrom(lcl.header.ledgerVersion, SOROBAN_PROTOCOL_VERSION)) @@ -1430,7 +1450,7 @@ HerderImpl::triggerNextLedger(uint32_t ledgerSeqToTrigger, invalidTxPhases[static_cast(TxSetPhase::SOROBAN)]); } - mTransactionQueue.ban( + mTransactionQueue->ban( invalidTxPhases[static_cast(TxSetPhase::CLASSIC)]); auto txSetHash = proposedSet->getContentsHash(); @@ -2129,9 +2149,11 @@ HerderImpl::maybeSetupSorobanQueue(uint32_t protocolVersion) { if (!mSorobanTransactionQueue) { + releaseAssert(mTxQueueBucketSnapshot); mSorobanTransactionQueue = std::make_unique( - mApp, TRANSACTION_QUEUE_TIMEOUT_LEDGERS, + mApp, mTxQueueBucketSnapshot, + TRANSACTION_QUEUE_TIMEOUT_LEDGERS, TRANSACTION_QUEUE_BAN_LEDGERS, SOROBAN_TRANSACTION_QUEUE_SIZE_MULTIPLIER); } @@ -2146,6 +2168,15 @@ HerderImpl::maybeSetupSorobanQueue(uint32_t protocolVersion) void HerderImpl::start() { + releaseAssert(!mTxQueueBucketSnapshot); + mTxQueueBucketSnapshot = mApp.getBucketManager() + .getBucketSnapshotManager() + .copySearchableLiveBucketListSnapshot(); + releaseAssert(!mTransactionQueue); + mTransactionQueue = std::make_unique( + mApp, mTxQueueBucketSnapshot, TRANSACTION_QUEUE_TIMEOUT_LEDGERS, + TRANSACTION_QUEUE_BAN_LEDGERS, TRANSACTION_QUEUE_SIZE_MULTIPLIER); + mMaxTxSize = mApp.getHerder().getMaxClassicTxSize(); { uint32_t version = mApp.getLedgerManager() @@ -2289,23 +2320,23 @@ HerderImpl::updateTransactionQueue(TxSetXDRFrameConstPtr externalizedTxSet) auto lhhe = mLedgerManager.getLastClosedLedgerHeader(); - auto updateQueue = [&](auto& queue, auto const& applied) { - queue.removeApplied(applied); - queue.shift(); - - auto txs = queue.getTransactions(lhhe.header); - - auto invalidTxs = TxSetUtils::getInvalidTxList( + auto filterInvalidTxs = [&](TxFrameList const& txs) { + return TxSetUtils::getInvalidTxList( txs, mApp, 0, - getUpperBoundCloseTimeOffset(mApp, lhhe.header.scpValue.closeTime)); - queue.ban(invalidTxs); - - queue.rebroadcast(); + getUpperBoundCloseTimeOffset(mApp.getAppConnector(), + lhhe.header.scpValue.closeTime)); }; + // Update bucket list snapshot, if needed. Note that this modifies the + // pointer itself on update, so we need to pass the potentially new pointer + // to the tx queues. + mApp.getBucketManager() + .getBucketSnapshotManager() + .maybeCopySearchableBucketListSnapshot(mTxQueueBucketSnapshot); if (txsPerPhase.size() > static_cast(TxSetPhase::CLASSIC)) { - updateQueue(mTransactionQueue, - txsPerPhase[static_cast(TxSetPhase::CLASSIC)]); + mTransactionQueue->update( + txsPerPhase[static_cast(TxSetPhase::CLASSIC)], lhhe.header, + mTxQueueBucketSnapshot, filterInvalidTxs); } // Even if we're in protocol 20, still check for number of phases, in case @@ -2314,8 +2345,9 @@ HerderImpl::updateTransactionQueue(TxSetXDRFrameConstPtr externalizedTxSet) if (mSorobanTransactionQueue != nullptr && txsPerPhase.size() > static_cast(TxSetPhase::SOROBAN)) { - updateQueue(*mSorobanTransactionQueue, - txsPerPhase[static_cast(TxSetPhase::SOROBAN)]); + mSorobanTransactionQueue->update( + txsPerPhase[static_cast(TxSetPhase::SOROBAN)], lhhe.header, + mTxQueueBucketSnapshot, filterInvalidTxs); } } @@ -2423,7 +2455,7 @@ HerderImpl::isNewerNominationOrBallotSt(SCPStatement const& oldSt, size_t HerderImpl::getMaxQueueSizeOps() const { - return mTransactionQueue.getMaxQueueSizeOps(); + return mTransactionQueue->getMaxQueueSizeOps(); } size_t @@ -2437,7 +2469,7 @@ HerderImpl::getMaxQueueSizeSorobanOps() const bool HerderImpl::isBannedTx(Hash const& hash) const { - auto banned = mTransactionQueue.isBanned(hash); + auto banned = mTransactionQueue->isBanned(hash); if (mSorobanTransactionQueue) { banned = banned || mSorobanTransactionQueue->isBanned(hash); @@ -2448,7 +2480,7 @@ HerderImpl::isBannedTx(Hash const& hash) const TransactionFrameBaseConstPtr HerderImpl::getTx(Hash const& hash) const { - auto classic = mTransactionQueue.getTx(hash); + auto classic = mTransactionQueue->getTx(hash); if (!classic && mSorobanTransactionQueue) { return mSorobanTransactionQueue->getTx(hash); diff --git a/src/herder/HerderImpl.h b/src/herder/HerderImpl.h index be1d3d8e12..855c926125 100644 --- a/src/herder/HerderImpl.h +++ b/src/herder/HerderImpl.h @@ -245,7 +245,7 @@ class HerderImpl : public Herder void purgeOldPersistedTxSets(); void writeDebugTxSet(LedgerCloseData const& lcd); - ClassicTransactionQueue mTransactionQueue; + std::unique_ptr mTransactionQueue; std::unique_ptr mSorobanTransactionQueue; void updateTransactionQueue(TxSetXDRFrameConstPtr txSet); @@ -298,6 +298,9 @@ class HerderImpl : public Herder Application& mApp; LedgerManager& mLedgerManager; + // Bucket list snapshot to use for transaction queues + SearchableSnapshotConstPtr mTxQueueBucketSnapshot; + struct SCPMetrics { medida::Meter& mLostSync; diff --git a/src/herder/TransactionQueue.cpp b/src/herder/TransactionQueue.cpp index ba5c44acce..9f4decf018 100644 --- a/src/herder/TransactionQueue.cpp +++ b/src/herder/TransactionQueue.cpp @@ -73,17 +73,20 @@ TransactionQueue::AddResult::AddResult(AddResultCode addCode, txResult->setResultCode(txErrorCode); } -TransactionQueue::TransactionQueue(Application& app, uint32 pendingDepth, - uint32 banDepth, uint32 poolLedgerMultiplier, - bool isSoroban) - : mApp(app) - , mPendingDepth(pendingDepth) +TransactionQueue::TransactionQueue(Application& app, + SearchableSnapshotConstPtr bucketSnapshot, + uint32 pendingDepth, uint32 banDepth, + uint32 poolLedgerMultiplier, bool isSoroban) + : mPendingDepth(pendingDepth) , mBannedTransactions(banDepth) , mBroadcastTimer(app) + , mValidationSnapshot( + std::make_shared(app)) + , mBucketSnapshot(bucketSnapshot) + , mTxQueueLimiter(poolLedgerMultiplier, isSoroban, mValidationSnapshot, + bucketSnapshot) + , mAppConn(app.getAppConnector()) { - mTxQueueLimiter = - std::make_unique(poolLedgerMultiplier, app, isSoroban); - auto const& filteredTypes = app.getConfig().EXCLUDE_TRANSACTIONS_CONTAINING_OPERATION_TYPE; mFilteredTypes.insert(filteredTypes.begin(), filteredTypes.end()); @@ -91,11 +94,11 @@ TransactionQueue::TransactionQueue(Application& app, uint32 pendingDepth, rand_uniform(0, std::numeric_limits::max()); } -ClassicTransactionQueue::ClassicTransactionQueue(Application& app, - uint32 pendingDepth, - uint32 banDepth, - uint32 poolLedgerMultiplier) - : TransactionQueue(app, pendingDepth, banDepth, poolLedgerMultiplier, false) +ClassicTransactionQueue::ClassicTransactionQueue( + Application& app, SearchableSnapshotConstPtr bucketSnapshot, + uint32 pendingDepth, uint32 banDepth, uint32 poolLedgerMultiplier) + : TransactionQueue(app, bucketSnapshot, pendingDepth, banDepth, + poolLedgerMultiplier, false) // Arb tx damping is only relevant to classic txs , mArbTxSeenCounter( app.getMetrics().NewCounter({"herder", "arb-tx", "seen"})) @@ -123,7 +126,7 @@ ClassicTransactionQueue::allowTxBroadcast(TimestampedTx const& tx) bool allowTx{true}; int32_t const signedAllowance = - mApp.getConfig().FLOOD_ARB_TX_BASE_ALLOWANCE; + mValidationSnapshot->getConfig().FLOOD_ARB_TX_BASE_ALLOWANCE; if (signedAllowance >= 0) { uint32_t const allowance = static_cast(signedAllowance); @@ -165,7 +168,8 @@ ClassicTransactionQueue::allowTxBroadcast(TimestampedTx const& tx) if (!allowTx) { std::geometric_distribution dist( - mApp.getConfig().FLOOD_ARB_TX_DAMPING_FACTOR); + mValidationSnapshot->getConfig() + .FLOOD_ARB_TX_DAMPING_FACTOR); uint32_t k = maxBroadcast - allowance; allowTx = dist(gRandomEngine) >= k; } @@ -266,6 +270,7 @@ isDuplicateTx(TransactionFrameBasePtr oldTx, TransactionFrameBasePtr newTx) bool TransactionQueue::sourceAccountPending(AccountID const& accountID) const { + std::lock_guard guard(mTxQueueMutex); return mAccountStates.find(accountID) != mAccountStates.end(); } @@ -348,6 +353,9 @@ TransactionQueue::canAdd( stateIter = mAccountStates.find(tx->getSourceID()); TransactionFrameBasePtr currentTx; + LedgerSnapshot const ls(mBucketSnapshot); + LedgerHeader const& lh = ls.getLedgerHeader().current(); + uint32_t ledgerVersion = lh.ledgerVersion; if (stateIter != mAccountStates.end()) { auto const& transaction = stateIter->second.mTransaction; @@ -380,13 +388,7 @@ TransactionQueue::canAdd( { auto txResult = tx->createSuccessResult(); if (!tx->checkSorobanResourceAndSetError( - mApp.getAppConnector(), - mApp.getLedgerManager() - .getSorobanNetworkConfigReadOnly(), - mApp.getLedgerManager() - .getLastClosedLedgerHeader() - .header.ledgerVersion, - txResult)) + *mValidationSnapshot, ledgerVersion, txResult)) { return AddResult(AddResultCode::ADD_STATUS_ERROR, txResult); } @@ -426,14 +428,12 @@ TransactionQueue::canAdd( } } - LedgerSnapshot ls(mApp); - uint32_t ledgerVersion = ls.getLedgerHeader().current().ledgerVersion; // Subtle: transactions are rejected based on the source account limit // prior to this point. This is safe because we can't evict transactions // from the same source account, so a newer transaction won't replace an // old one. auto canAddRes = - mTxQueueLimiter->canAddTx(tx, currentTx, txsToEvict, ledgerVersion); + mTxQueueLimiter.canAddTx(tx, currentTx, txsToEvict, ledgerVersion); if (!canAddRes.first) { ban({tx}); @@ -448,20 +448,21 @@ TransactionQueue::canAdd( TransactionQueue::AddResultCode::ADD_STATUS_TRY_AGAIN_LATER); } - auto closeTime = mApp.getLedgerManager() - .getLastClosedLedgerHeader() - .header.scpValue.closeTime; + auto closeTime = lh.scpValue.closeTime; if (protocolVersionStartsFrom(ledgerVersion, ProtocolVersion::V_19)) { // This is done so minSeqLedgerGap is validated against the next // ledgerSeq, which is what will be used at apply time - ls.getLedgerHeader().currentToModify().ledgerSeq = - mApp.getLedgerManager().getLastClosedLedgerNum() + 1; + ++ls.getLedgerHeader().currentToModify().ledgerSeq; + // TODO: ^^ I think this is the right thing to do. Was previously the + // commented out line below. + // ls.getLedgerHeader().currentToModify().ledgerSeq = + // mApp.getLedgerManager().getLastClosedLedgerNum() + 1; } auto txResult = - tx->checkValid(mApp.getAppConnector(), ls, 0, 0, - getUpperBoundCloseTimeOffset(mApp, closeTime)); + tx->checkValid(*mValidationSnapshot, ls, 0, 0, + getUpperBoundCloseTimeOffset(mAppConn, closeTime)); if (!txResult->isSuccess()) { return AddResult(TransactionQueue::AddResultCode::ADD_STATUS_ERROR, @@ -493,7 +494,7 @@ TransactionQueue::canAdd( releaseAssertOrThrow(sorobanTxData); sorobanTxData->pushValidationTimeDiagnosticError( - mApp.getConfig(), SCE_CONTEXT, SCEC_INVALID_INPUT, + mValidationSnapshot->getConfig(), SCE_CONTEXT, SCEC_INVALID_INPUT, "non-source auth Soroban tx uses memo or muxed source account"); return AddResult(TransactionQueue::AddResultCode::ADD_STATUS_ERROR, @@ -522,7 +523,7 @@ void TransactionQueue::prepareDropTransaction(AccountState& as) { releaseAssert(as.mTransaction); - mTxQueueLimiter->removeTransaction(as.mTransaction->mTx); + mTxQueueLimiter.removeTransaction(as.mTransaction->mTx); mKnownTxHashes.erase(as.mTransaction->mTx->getFullHash()); CLOG_DEBUG(Tx, "Dropping {} transaction", hexAbbrev(as.mTransaction->mTx->getFullHash())); @@ -644,6 +645,7 @@ TransactionQueue::AddResult TransactionQueue::tryAdd(TransactionFrameBasePtr tx, bool submittedFromSelf) { ZoneScoped; + std::lock_guard guard(mTxQueueMutex); auto c1 = tx->getEnvelope().type() == ENVELOPE_TYPE_TX_FEE_BUMP && @@ -682,12 +684,12 @@ TransactionQueue::tryAdd(TransactionFrameBasePtr tx, bool submittedFromSelf) // Drop current transaction associated with this account, replace // with `tx` prepareDropTransaction(stateIter->second); - *oldTx = {tx, false, mApp.getClock().now(), submittedFromSelf}; + *oldTx = {tx, false, mAppConn.now(), submittedFromSelf}; } else { // New transaction for this account, insert it and update age - stateIter->second.mTransaction = {tx, false, mApp.getClock().now(), + stateIter->second.mTransaction = {tx, false, mAppConn.now(), submittedFromSelf}; mQueueMetrics->mSizeByAge[stateIter->second.mAge]->inc(); } @@ -698,13 +700,14 @@ TransactionQueue::tryAdd(TransactionFrameBasePtr tx, bool submittedFromSelf) // make space so that we can add this transaction // this will succeed as `canAdd` ensures that this is the case - mTxQueueLimiter->evictTransactions( + mTxQueueLimiter.evictTransactions( txsToEvict, *tx, [&](TransactionFrameBasePtr const& txToEvict) { ban({txToEvict}); }); - mTxQueueLimiter->addTransaction(tx); + mTxQueueLimiter.addTransaction(tx); mKnownTxHashes[tx->getFullHash()] = tx; - broadcast(false); + mAppConn.postOnMainThread([this]() { broadcast(false); }, + "tx queue broadcast"); return res; } @@ -742,7 +745,7 @@ TransactionQueue::removeApplied(Transactions const& appliedTxs) { ZoneScoped; - auto now = mApp.getClock().now(); + auto now = mAppConn.now(); for (auto const& appliedTx : appliedTxs) { // If the source account is not in mAccountStates, then it has no @@ -803,6 +806,7 @@ void TransactionQueue::ban(Transactions const& banTxs) { ZoneScoped; + std::lock_guard guard(mTxQueueMutex); auto& bannedFront = mBannedTransactions.front(); // Group the transactions by source account and ban all the transactions @@ -848,6 +852,7 @@ TransactionQueue::AccountState TransactionQueue::getAccountTransactionQueueInfo( AccountID const& accountID) const { + std::lock_guard guard(mTxQueueMutex); auto i = mAccountStates.find(accountID); if (i == std::end(mAccountStates)) { @@ -859,6 +864,7 @@ TransactionQueue::getAccountTransactionQueueInfo( size_t TransactionQueue::countBanned(int index) const { + std::lock_guard guard(mTxQueueMutex); return mBannedTransactions[index].size(); } #endif @@ -924,7 +930,7 @@ TransactionQueue::shift() { mQueueMetrics->mSizeByAge[i]->set_count(sizes[i]); } - mTxQueueLimiter->resetEvictionState(); + mTxQueueLimiter.resetEvictionState(); // pick a new randomizing seed for tie breaking mBroadcastSeed = rand_uniform(0, std::numeric_limits::max()); @@ -933,6 +939,7 @@ TransactionQueue::shift() bool TransactionQueue::isBanned(Hash const& hash) const { + std::lock_guard guard(mTxQueueMutex); return std::any_of( std::begin(mBannedTransactions), std::end(mBannedTransactions), [&](UnorderedSet const& transactions) { @@ -944,6 +951,7 @@ TxFrameList TransactionQueue::getTransactions(LedgerHeader const& lcl) const { ZoneScoped; + std::lock_guard guard(mTxQueueMutex); TxFrameList txs; uint32_t const nextLedgerSeq = lcl.ledgerSeq + 1; @@ -964,6 +972,7 @@ TransactionFrameBaseConstPtr TransactionQueue::getTx(Hash const& hash) const { ZoneScoped; + std::lock_guard guard(mTxQueueMutex); auto it = mKnownTxHashes.find(hash); if (it != mKnownTxHashes.end()) { @@ -978,10 +987,11 @@ TransactionQueue::getTx(Hash const& hash) const std::pair> ClassicTransactionQueue::getMaxResourcesToFloodThisPeriod() const { - auto& cfg = mApp.getConfig(); + auto& cfg = mValidationSnapshot->getConfig(); double opRatePerLedger = cfg.FLOOD_OP_RATE_PER_LEDGER; - auto maxOps = mApp.getLedgerManager().getLastMaxTxSetSizeOps(); + auto maxOps = + LedgerManager::getMaxTxSetSizeOps(mBucketSnapshot->getLedgerHeader()); double opsToFloodLedgerDbl = opRatePerLedger * maxOps; releaseAssertOrThrow(opsToFloodLedgerDbl >= 0.0); releaseAssertOrThrow(isRepresentableAsInt64(opsToFloodLedgerDbl)); @@ -1022,6 +1032,9 @@ ClassicTransactionQueue::getMaxResourcesToFloodThisPeriod() const TransactionQueue::BroadcastStatus TransactionQueue::broadcastTx(TimestampedTx& tx) { + // Must be main thread because we are accessing the overlay manager + releaseAssert(threadIsMain()); + if (tx.mBroadcasted) { return BroadcastStatus::BROADCAST_STATUS_ALREADY; @@ -1050,18 +1063,18 @@ TransactionQueue::broadcastTx(TimestampedTx& tx) // useful work from other sources. return BroadcastStatus::BROADCAST_STATUS_SKIPPED; } - return mApp.getOverlayManager().broadcastMessage( + return mAppConn.getOverlayManager().broadcastMessage( tx.mTx->toStellarMessage(), std::make_optional(tx.mTx->getFullHash())) ? BroadcastStatus::BROADCAST_STATUS_SUCCESS : BroadcastStatus::BROADCAST_STATUS_ALREADY; } -SorobanTransactionQueue::SorobanTransactionQueue(Application& app, - uint32 pendingDepth, - uint32 banDepth, - uint32 poolLedgerMultiplier) - : TransactionQueue(app, pendingDepth, banDepth, poolLedgerMultiplier, true) +SorobanTransactionQueue::SorobanTransactionQueue( + Application& app, SearchableSnapshotConstPtr bucketSnapshot, + uint32 pendingDepth, uint32 banDepth, uint32 poolLedgerMultiplier) + : TransactionQueue(app, bucketSnapshot, pendingDepth, banDepth, + poolLedgerMultiplier, true) { std::vector sizeByAge; @@ -1084,10 +1097,11 @@ SorobanTransactionQueue::SorobanTransactionQueue(Application& app, std::pair> SorobanTransactionQueue::getMaxResourcesToFloodThisPeriod() const { - auto const& cfg = mApp.getConfig(); + auto const& cfg = mValidationSnapshot->getConfig(); double ratePerLedger = cfg.FLOOD_SOROBAN_RATE_PER_LEDGER; - auto sorRes = mApp.getLedgerManager().maxLedgerResources(true); + auto sorRes = LedgerManager::maxSorobanLedgerResources( + mValidationSnapshot->getSorobanNetworkConfig()); auto totalFloodPerLedger = multiplyByDouble(sorRes, ratePerLedger); @@ -1102,6 +1116,9 @@ SorobanTransactionQueue::getMaxResourcesToFloodThisPeriod() const bool SorobanTransactionQueue::broadcastSome() { + // Must be main thread for call to `broadcastTx` + releaseAssert(threadIsMain()); + // broadcast transactions in surge pricing order: // loop over transactions by picking from the account queue with the // highest base fee not broadcasted so far. @@ -1154,8 +1171,8 @@ SorobanTransactionQueue::broadcastSome() std::make_shared(resToFlood), mBroadcastSeed); queue.visitTopTxs(txsToBroadcast, visitor, mBroadcastOpCarryover); - Resource maxPerTx = - mApp.getLedgerManager().maxSorobanTransactionResources(); + Resource maxPerTx = LedgerManager::maxSorobanTransactionResources( + mValidationSnapshot->getSorobanNetworkConfig()); for (auto& resLeft : mBroadcastOpCarryover) { // Limit carry-over to 1 maximum resource transaction @@ -1167,24 +1184,20 @@ SorobanTransactionQueue::broadcastSome() size_t SorobanTransactionQueue::getMaxQueueSizeOps() const { - if (protocolVersionStartsFrom(mApp.getLedgerManager() - .getLastClosedLedgerHeader() - .header.ledgerVersion, - SOROBAN_PROTOCOL_VERSION)) - { - auto res = mTxQueueLimiter->maxScaledLedgerResources(true); - releaseAssert(res.size() == NUM_SOROBAN_TX_RESOURCES); - return res.getVal(Resource::Type::OPERATIONS); - } - else - { - return 0; - } + std::lock_guard guard(mTxQueueMutex); + // TODO: I removed a conditional checking that the protocol version is + // post-soroban here. I think that check is now unnecessary, right? + auto res = mTxQueueLimiter.maxScaledLedgerResources(true); + releaseAssert(res.size() == NUM_SOROBAN_TX_RESOURCES); + return res.getVal(Resource::Type::OPERATIONS); } bool ClassicTransactionQueue::broadcastSome() { + // Must be main thread for call to `broadcastTx` + releaseAssert(threadIsMain()); + // broadcast transactions in surge pricing order: // loop over transactions by picking from the account queue with the // highest base fee not broadcasted so far. @@ -1260,6 +1273,13 @@ ClassicTransactionQueue::broadcastSome() void TransactionQueue::broadcast(bool fromCallback) { + // Must be called from the main thread due to the use of `mBroadcastTimer` + releaseAssert(threadIsMain()); + + // NOTE: Although this is not a public function, it can be called from + // `mBroadcastTimer` and so it needs to be synchronized. + std::lock_guard guard(mTxQueueMutex); + if (mShutdown || (!fromCallback && mWaiting)) { return; @@ -1282,6 +1302,9 @@ TransactionQueue::broadcast(bool fromCallback) mWaiting = true; mBroadcastTimer.expires_from_now( std::chrono::milliseconds(getFloodPeriod())); + // TODO: This use of mBroadcastTimer is OK because this function can + // only be called from the main thread. If I push the cut point out to + // allow for background broadcasting then I need to replace this timer. mBroadcastTimer.async_wait([&]() { broadcast(true); }, &VirtualTimer::onFailureNoop); } @@ -1290,6 +1313,9 @@ TransactionQueue::broadcast(bool fromCallback) void TransactionQueue::rebroadcast() { + // For `broadcast` call + releaseAssert(threadIsMain()); + // force to rebroadcast everything for (auto& m : mAccountStates) { @@ -1305,10 +1331,37 @@ TransactionQueue::rebroadcast() void TransactionQueue::shutdown() { + releaseAssert(threadIsMain()); + std::lock_guard guard(mTxQueueMutex); mShutdown = true; mBroadcastTimer.cancel(); } +void +TransactionQueue::update( + Transactions const& applied, LedgerHeader const& lcl, + SearchableSnapshotConstPtr const newBucketSnapshot, + std::function const& filterInvalidTxs) +{ + ZoneScoped; + releaseAssert(threadIsMain()); + std::lock_guard guard(mTxQueueMutex); + + mValidationSnapshot = + std::make_shared(mAppConn); + mBucketSnapshot = newBucketSnapshot; + mTxQueueLimiter.updateSnapshots(mValidationSnapshot, mBucketSnapshot); + + removeApplied(applied); + shift(); + + auto txs = getTransactions(lcl); + auto invalidTxs = filterInvalidTxs(txs); + ban(invalidTxs); + + rebroadcast(); +} + static bool containsFilteredOperation(std::vector const& ops, UnorderedSet const& filteredTypes) @@ -1350,12 +1403,14 @@ TransactionQueue::isFiltered(TransactionFrameBasePtr tx) const size_t TransactionQueue::getQueueSizeOps() const { - return mTxQueueLimiter->size(); + std::lock_guard guard(mTxQueueMutex); + return mTxQueueLimiter.size(); } std::optional TransactionQueue::getInQueueSeqNum(AccountID const& account) const { + std::lock_guard guard(mTxQueueMutex); auto stateIter = mAccountStates.find(account); if (stateIter == mAccountStates.end()) { @@ -1372,7 +1427,8 @@ TransactionQueue::getInQueueSeqNum(AccountID const& account) const size_t ClassicTransactionQueue::getMaxQueueSizeOps() const { - auto res = mTxQueueLimiter->maxScaledLedgerResources(false); + std::lock_guard guard(mTxQueueMutex); + auto res = mTxQueueLimiter.maxScaledLedgerResources(false); releaseAssert(res.size() == NUM_CLASSIC_TX_RESOURCES); return res.getVal(Resource::Type::OPERATIONS); } diff --git a/src/herder/TransactionQueue.h b/src/herder/TransactionQueue.h index 81b2409853..f47df0c307 100644 --- a/src/herder/TransactionQueue.h +++ b/src/herder/TransactionQueue.h @@ -69,7 +69,8 @@ class TransactionQueue ADD_STATUS_ERROR, ADD_STATUS_TRY_AGAIN_LATER, ADD_STATUS_FILTERED, - ADD_STATUS_COUNT + ADD_STATUS_COUNT, + ADD_STATUS_UNKNOWN // TODO: rename? }; struct AddResult @@ -117,30 +118,31 @@ class TransactionQueue std::optional mTransaction; }; - explicit TransactionQueue(Application& app, uint32 pendingDepth, - uint32 banDepth, uint32 poolLedgerMultiplier, - bool isSoroban); + explicit TransactionQueue(Application& app, + SearchableSnapshotConstPtr bucketSnapshot, + uint32 pendingDepth, uint32 banDepth, + uint32 poolLedgerMultiplier, bool isSoroban); virtual ~TransactionQueue(); static std::vector findAllAssetPairsInvolvedInPaymentLoops(TransactionFrameBasePtr tx); AddResult tryAdd(TransactionFrameBasePtr tx, bool submittedFromSelf); - void removeApplied(Transactions const& txs); // Ban transactions that are no longer valid or have insufficient fee; // transaction per account limit applies here, so `txs` should have no // duplicate source accounts void ban(Transactions const& txs); - /** - * Increase age of each AccountState that has at least one transaction in - * mTransactions. Also increments the age for each banned transaction, and - * unbans transactions for which age equals banDepth. - */ - void shift(); - void rebroadcast(); void shutdown(); + // TODO: Better docs + // TODO: More descriptive name + // Update internal queue structures after a ledger closes + void update( + Transactions const& applied, LedgerHeader const& lcl, + SearchableSnapshotConstPtr newBucketSnapshot, + std::function const& filterInvalidTxs); + bool isBanned(Hash const& hash) const; TransactionFrameBaseConstPtr getTx(Hash const& hash) const; TxFrameList getTransactions(LedgerHeader const& lcl) const; @@ -170,7 +172,6 @@ class TransactionQueue */ using BannedTransactions = std::deque>; - Application& mApp; uint32 const mPendingDepth; AccountStates mAccountStates; @@ -201,6 +202,9 @@ class TransactionQueue bool mShutdown{false}; bool mWaiting{false}; + // TODO: VirtualTimer is not thread-safe. Right now it's only used in + // functions that are called from the main thread. However, if I move + // broadcasting to the background I will need to be careful with this. VirtualTimer mBroadcastTimer; virtual std::pair> @@ -230,15 +234,37 @@ class TransactionQueue bool isFiltered(TransactionFrameBasePtr tx) const; - std::unique_ptr mTxQueueLimiter; + // Snapshots to use for transaction validation + ImmutableValidationSnapshotPtr mValidationSnapshot; + SearchableSnapshotConstPtr mBucketSnapshot; + + TxQueueLimiter mTxQueueLimiter; UnorderedMap mArbitrageFloodDamping; UnorderedMap mKnownTxHashes; size_t mBroadcastSeed; + mutable std::recursive_mutex mTxQueueMutex; + + private: + AppConnector& mAppConn; + + void removeApplied(Transactions const& txs); + + /** + * Increase age of each AccountState that has at least one transaction in + * mTransactions. Also increments the age for each banned transaction, and + * unbans transactions for which age equals banDepth. + */ + void shift(); + + void rebroadcast(); + #ifdef BUILD_TESTS public: + friend class TransactionQueueTest; + size_t getQueueSizeOps() const; std::optional getInQueueSeqNum(AccountID const& account) const; std::function mTxBroadcastedEvent; @@ -248,12 +274,15 @@ class TransactionQueue class SorobanTransactionQueue : public TransactionQueue { public: - SorobanTransactionQueue(Application& app, uint32 pendingDepth, - uint32 banDepth, uint32 poolLedgerMultiplier); + SorobanTransactionQueue(Application& app, + SearchableSnapshotConstPtr bucketSnapshot, + uint32 pendingDepth, uint32 banDepth, + uint32 poolLedgerMultiplier); int getFloodPeriod() const override { - return mApp.getConfig().FLOOD_SOROBAN_TX_PERIOD_MS; + std::lock_guard guard(mTxQueueMutex); + return mValidationSnapshot->getConfig().FLOOD_SOROBAN_TX_PERIOD_MS; } size_t getMaxQueueSizeOps() const override; @@ -261,6 +290,7 @@ class SorobanTransactionQueue : public TransactionQueue void clearBroadcastCarryover() { + std::lock_guard guard(mTxQueueMutex); mBroadcastOpCarryover.clear(); mBroadcastOpCarryover.resize(1, Resource::makeEmptySoroban()); } @@ -282,13 +312,16 @@ class SorobanTransactionQueue : public TransactionQueue class ClassicTransactionQueue : public TransactionQueue { public: - ClassicTransactionQueue(Application& app, uint32 pendingDepth, - uint32 banDepth, uint32 poolLedgerMultiplier); + ClassicTransactionQueue(Application& app, + SearchableSnapshotConstPtr bucketSnapshot, + uint32 pendingDepth, uint32 banDepth, + uint32 poolLedgerMultiplier); int getFloodPeriod() const override { - return mApp.getConfig().FLOOD_TX_PERIOD_MS; + std::lock_guard guard(mTxQueueMutex); + return mValidationSnapshot->getConfig().FLOOD_TX_PERIOD_MS; } size_t getMaxQueueSizeOps() const override; diff --git a/src/herder/TxQueueLimiter.cpp b/src/herder/TxQueueLimiter.cpp index 265ce8fb72..fab7d5deef 100644 --- a/src/herder/TxQueueLimiter.cpp +++ b/src/herder/TxQueueLimiter.cpp @@ -29,14 +29,15 @@ computeBetterFee(std::pair const& evictedBid, } -TxQueueLimiter::TxQueueLimiter(uint32 multiplier, Application& app, - bool isSoroban) +TxQueueLimiter::TxQueueLimiter(uint32 multiplier, bool isSoroban, + ImmutableValidationSnapshotPtr vs, + SearchableSnapshotConstPtr bls) : mPoolLedgerMultiplier(multiplier) - , mLedgerManager(app.getLedgerManager()) - , mApp(app) , mIsSoroban(isSoroban) + , mValidationSnapshot(vs) + , mBucketSnapshot(bls) { - auto maxDexOps = app.getConfig().MAX_DEX_TX_OPERATIONS_IN_TX_SET; + auto maxDexOps = vs->getConfig().MAX_DEX_TX_OPERATIONS_IN_TX_SET; if (maxDexOps && !mIsSoroban) { mMaxDexOperations = @@ -60,8 +61,12 @@ TxQueueLimiter::size() const Resource TxQueueLimiter::maxScaledLedgerResources(bool isSoroban) const { - return multiplyByDouble(mLedgerManager.maxLedgerResources(isSoroban), - mPoolLedgerMultiplier); + Resource const r = isSoroban + ? LedgerManager::maxSorobanLedgerResources( + mValidationSnapshot->getSorobanNetworkConfig()) + : LedgerManager::maxClassicLedgerResources( + mBucketSnapshot->getLedgerHeader()); + return multiplyByDouble(r, mPoolLedgerMultiplier); } void @@ -84,9 +89,7 @@ TxQueueLimiter::canAddTx( std::vector>& txsToEvict) { return canAddTx(newTx, oldTx, txsToEvict, - mApp.getLedgerManager() - .getLastClosedLedgerHeader() - .header.ledgerVersion); + mBucketSnapshot->getLedgerHeader().ledgerVersion); } #endif @@ -236,6 +239,13 @@ TxQueueLimiter::reset(uint32_t ledgerVersion) resetEvictionState(); } +void +TxQueueLimiter::updateSnapshots(ImmutableValidationSnapshotPtr vs, SearchableSnapshotConstPtr bls) +{ + mValidationSnapshot = vs; + mBucketSnapshot = bls; +} + void TxQueueLimiter::resetEvictionState() { diff --git a/src/herder/TxQueueLimiter.h b/src/herder/TxQueueLimiter.h index e913af6436..8aa9372e44 100644 --- a/src/herder/TxQueueLimiter.h +++ b/src/herder/TxQueueLimiter.h @@ -17,7 +17,6 @@ class TxQueueLimiter { // number of ledgers we can pool in memory uint32 const mPoolLedgerMultiplier; - LedgerManager& mLedgerManager; // all known transactions std::unique_ptr mTxs; @@ -33,11 +32,15 @@ class TxQueueLimiter // limits. std::shared_ptr mSurgePricingLaneConfig; - Application& mApp; bool const mIsSoroban; + // State snapshots used to compute limits + ImmutableValidationSnapshotPtr mValidationSnapshot; + SearchableSnapshotConstPtr mBucketSnapshot; + public: - TxQueueLimiter(uint32 multiplier, Application& app, bool isSoroban); + TxQueueLimiter(uint32 multiplier, bool isSoroban, ImmutableValidationSnapshotPtr vs, + SearchableSnapshotConstPtr bls); ~TxQueueLimiter(); void addTransaction(TransactionFrameBasePtr const& tx); @@ -80,5 +83,9 @@ class TxQueueLimiter // Resets the internal transaction container and the eviction state. void reset(uint32_t ledgerVersion); + + // Update snapshots. Should be called after ledger close + void updateSnapshots(ImmutableValidationSnapshotPtr vs, + SearchableSnapshotConstPtr bls); }; } diff --git a/src/herder/TxSetFrame.cpp b/src/herder/TxSetFrame.cpp index cf4afc218d..b0c05aa2c1 100644 --- a/src/herder/TxSetFrame.cpp +++ b/src/herder/TxSetFrame.cpp @@ -430,6 +430,7 @@ phaseTxsAreValid(TxSetPhaseFrame const& phase, Application& app, uint64_t upperBoundCloseTimeOffset) { ZoneScoped; + releaseAssert(threadIsMain()); // This is done so minSeqLedgerGap is validated against the next // ledgerSeq, which is what will be used at apply time @@ -438,10 +439,12 @@ phaseTxsAreValid(TxSetPhaseFrame const& phase, Application& app, LedgerSnapshot ls(app); ls.getLedgerHeader().currentToModify().ledgerSeq = app.getLedgerManager().getLastClosedLedgerNum() + 1; + // TODO: Double check that the readonly soroban network config is what we + // want here. + AppValidationWrapper const avw(app.getAppConnector(), false); for (auto const& tx : phase) { - auto txResult = tx->checkValid(app.getAppConnector(), ls, 0, - lowerBoundCloseTimeOffset, + auto txResult = tx->checkValid(avw, ls, 0, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); if (!txResult->isSuccess()) { diff --git a/src/herder/TxSetUtils.cpp b/src/herder/TxSetUtils.cpp index f81456b584..1a51b5e1c7 100644 --- a/src/herder/TxSetUtils.cpp +++ b/src/herder/TxSetUtils.cpp @@ -176,10 +176,10 @@ TxSetUtils::getInvalidTxList(TxFrameList const& txs, Application& app, TxFrameList invalidTxs; + AppValidationWrapper avw(app.getAppConnector(), false); for (auto const& tx : txs) { - auto txResult = tx->checkValid(app.getAppConnector(), ls, 0, - lowerBoundCloseTimeOffset, + auto txResult = tx->checkValid(avw, ls, 0, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); if (!txResult->isSuccess()) { diff --git a/src/herder/test/TransactionQueueTests.cpp b/src/herder/test/TransactionQueueTests.cpp index 64caea683e..1540834352 100644 --- a/src/herder/test/TransactionQueueTests.cpp +++ b/src/herder/test/TransactionQueueTests.cpp @@ -70,7 +70,10 @@ invalidTransaction(Application& app, TestAccount& account, int sequenceDelta) app, account, account.getLastSequenceNumber() + sequenceDelta, {payment(account.getPublicKey(), -1)}); } +} +namespace stellar +{ class TransactionQueueTest { public: @@ -258,7 +261,10 @@ TEST_CASE("TransactionQueue complex scenarios", "[herder][transactionqueue]") cfg.TESTING_UPGRADE_MAX_TX_SET_SIZE = 4; cfg.FLOOD_TX_PERIOD_MS = 100; auto app = createTestApplication(clock, cfg); - auto queue = ClassicTransactionQueue{*app, 4, 2, 2}; + auto bls = app->getBucketManager() + .getBucketSnapshotManager() + .copySearchableLiveBucketListSnapshot(); + auto queue = ClassicTransactionQueue{*app, bls, 4, 2, 2}; auto const minBalance2 = app->getLedgerManager().getLastMinBalance(2); auto root = TestAccount::createRoot(*app); @@ -527,7 +533,10 @@ testTransactionQueueBasicScenarios() cfg.TESTING_UPGRADE_MAX_TX_SET_SIZE = 4; cfg.FLOOD_TX_PERIOD_MS = 100; auto app = createTestApplication(clock, cfg); - auto queue = ClassicTransactionQueue{*app, 4, 2, 2}; + auto bls = app->getBucketManager() + .getBucketSnapshotManager() + .copySearchableLiveBucketListSnapshot(); + auto queue = ClassicTransactionQueue{*app, bls, 4, 2, 2}; auto const minBalance2 = app->getLedgerManager().getLastMinBalance(2); auto root = TestAccount::createRoot(*app); @@ -770,7 +779,10 @@ TEST_CASE("TransactionQueue hitting the rate limit", cfg.TESTING_UPGRADE_MAX_TX_SET_SIZE = 4; cfg.FLOOD_TX_PERIOD_MS = 100; auto app = createTestApplication(clock, cfg); - auto queue = ClassicTransactionQueue{*app, 4, 2, 2}; + auto bls = app->getBucketManager() + .getBucketSnapshotManager() + .copySearchableLiveBucketListSnapshot(); + auto queue = ClassicTransactionQueue{*app, bls, 4, 2, 2}; auto const minBalance2 = app->getLedgerManager().getLastMinBalance(2); auto root = TestAccount::createRoot(*app); @@ -851,7 +863,10 @@ TEST_CASE("TransactionQueue with PreconditionsV2", "[herder][transactionqueue]") cfg.TESTING_UPGRADE_MAX_TX_SET_SIZE = 4; cfg.FLOOD_TX_PERIOD_MS = 100; auto app = createTestApplication(clock, cfg); - auto queue = ClassicTransactionQueue{*app, 4, 2, 2}; + auto bls = app->getBucketManager() + .getBucketSnapshotManager() + .copySearchableLiveBucketListSnapshot(); + auto queue = ClassicTransactionQueue{*app, bls, 4, 2, 2}; auto const minBalance2 = app->getLedgerManager().getLastMinBalance(2); auto root = TestAccount::createRoot(*app); @@ -1701,7 +1716,12 @@ TEST_CASE("TransactionQueue limits", "[herder][transactionqueue]") auto account6 = root.create("a6", minBalance2); auto account7 = root.create("a7", minBalance2); - TxQueueLimiter limiter(1, *app, false); + auto vs = std::make_shared( + app->getAppConnector()); + auto bls = app->getBucketManager() + .getBucketSnapshotManager() + .copySearchableLiveBucketListSnapshot(); + TxQueueLimiter limiter(1, false, vs, bls); struct SetupElement { @@ -1882,7 +1902,12 @@ TEST_CASE("TransactionQueue limiter with DEX separation", auto account6 = root.create("a6", minBalance2); // 3 * 3 = 9 operations limit, 3 * 1 = 3 DEX operations limit. - TxQueueLimiter limiter(3, *app, false); + auto vs = std::make_shared( + app->getAppConnector()); + auto bls = app->getBucketManager() + .getBucketSnapshotManager() + .copySearchableLiveBucketListSnapshot(); + TxQueueLimiter limiter(3, false, vs, bls); std::vector txs; @@ -2130,7 +2155,10 @@ TEST_CASE("transaction queue starting sequence boundary", acc1.bumpSequence(startingSeq - 1); REQUIRE(acc1.loadSequenceNumber() == startingSeq - 1); - ClassicTransactionQueue tq(*app, 4, 10, 4); + auto bls = app->getBucketManager() + .getBucketSnapshotManager() + .copySearchableLiveBucketListSnapshot(); + ClassicTransactionQueue tq(*app, bls, 4, 10, 4); REQUIRE(tq.tryAdd(transaction(*app, acc1, 1, 1, 100), false).code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); @@ -2511,12 +2539,18 @@ TEST_CASE("transaction queue with fee-bump", "[herder][transactionqueue]") SECTION("classic") { - auto queue = ClassicTransactionQueue{*app, 4, 2, 2}; + auto bls = app->getBucketManager() + .getBucketSnapshotManager() + .copySearchableLiveBucketListSnapshot(); + auto queue = ClassicTransactionQueue{*app, bls, 4, 2, 2}; testFeeBump(queue, /* isSoroban */ false); } SECTION("soroban") { - auto queue = SorobanTransactionQueue{*app, 4, 2, 2}; + auto bls = app->getBucketManager() + .getBucketSnapshotManager() + .copySearchableLiveBucketListSnapshot(); + auto queue = SorobanTransactionQueue{*app, bls, 4, 2, 2}; testFeeBump(queue, /* isSoroban */ true); } } @@ -2688,12 +2722,18 @@ TEST_CASE("replace by fee", "[herder][transactionqueue]") SECTION("classic") { - auto queue = ClassicTransactionQueue{*app, 4, 2, 2}; + auto bls = app->getBucketManager() + .getBucketSnapshotManager() + .copySearchableLiveBucketListSnapshot(); + auto queue = ClassicTransactionQueue{*app, bls, 4, 2, 2}; testReplaceByFee(queue, /* isSoroban */ false); } SECTION("soroban") { - auto queue = SorobanTransactionQueue{*app, 4, 2, 2}; + auto bls = app->getBucketManager() + .getBucketSnapshotManager() + .copySearchableLiveBucketListSnapshot(); + auto queue = SorobanTransactionQueue{*app, bls, 4, 2, 2}; testReplaceByFee(queue, /* isSoroban */ true); } } diff --git a/src/ledger/LedgerManager.h b/src/ledger/LedgerManager.h index f85ef02161..7ad297c08d 100644 --- a/src/ledger/LedgerManager.h +++ b/src/ledger/LedgerManager.h @@ -86,6 +86,18 @@ class LedgerManager // Genesis ledger static LedgerHeader genesisLedger(); + // TODO: Docs + static Resource maxClassicLedgerResources(LedgerHeader const& conf); + + // TODO: Docs + static Resource maxSorobanLedgerResources(SorobanNetworkConfig const& conf); + + // TODO: Docs + static Resource maxSorobanTransactionResources(SorobanNetworkConfig const& conf); + + // TODO: Docs + static uint32_t getMaxTxSetSizeOps(LedgerHeader const& header); + // Called by Herder to inform LedgerManager that a SCP has agreed on a new // close event. This is the most common cause of LedgerManager advancing // from one ledger to the next: the network reached consensus on diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp index ca0b0bf949..efdeeddf97 100644 --- a/src/ledger/LedgerManagerImpl.cpp +++ b/src/ledger/LedgerManagerImpl.cpp @@ -122,6 +122,50 @@ LedgerManager::ledgerAbbrev(LedgerHeaderHistoryEntry const& he) return ledgerAbbrev(he.header, he.hash); } +Resource +LedgerManager::maxClassicLedgerResources(LedgerHeader const& header) +{ + return Resource(LedgerManager::getMaxTxSetSizeOps(header)); +} + +Resource +LedgerManager::maxSorobanLedgerResources(SorobanNetworkConfig const& conf) +{ + ZoneScoped std::vector limits = { + conf.ledgerMaxTxCount(), + conf.ledgerMaxInstructions(), + conf.ledgerMaxTransactionSizesBytes(), + conf.ledgerMaxReadBytes(), + conf.ledgerMaxWriteBytes(), + conf.ledgerMaxReadLedgerEntries(), + conf.ledgerMaxWriteLedgerEntries()}; + return Resource(limits); +} + +Resource +LedgerManager::maxSorobanTransactionResources(SorobanNetworkConfig const& conf) +{ + ZoneScoped int64_t const opCount = 1; + std::vector limits = {opCount, + conf.txMaxInstructions(), + conf.txMaxSizeBytes(), + conf.txMaxReadBytes(), + conf.txMaxWriteBytes(), + conf.txMaxReadLedgerEntries(), + conf.txMaxWriteLedgerEntries()}; + return Resource(limits); +} + +uint32_t +LedgerManager::getMaxTxSetSizeOps(LedgerHeader const& header) +{ + auto n = header.maxTxSetSize; + return protocolVersionStartsFrom(header.ledgerVersion, + ProtocolVersion::V_11) + ? n + : (n * MAX_OPS_PER_TX); +} + LedgerManagerImpl::LedgerManagerImpl(Application& app) : mApp(app) , mSorobanMetrics(app.getMetrics()) @@ -439,34 +483,24 @@ uint32_t LedgerManagerImpl::getLastMaxTxSetSizeOps() const { releaseAssert(threadIsMain()); - auto n = mLastClosedLedger.header.maxTxSetSize; - return protocolVersionStartsFrom(mLastClosedLedger.header.ledgerVersion, - ProtocolVersion::V_11) - ? n - : (n * MAX_OPS_PER_TX); + return LedgerManager::getMaxTxSetSizeOps(mLastClosedLedger.header); } Resource LedgerManagerImpl::maxLedgerResources(bool isSoroban) { ZoneScoped; + releaseAssert(threadIsMain()); if (isSoroban) { - auto conf = getSorobanNetworkConfigReadOnly(); - std::vector limits = {conf.ledgerMaxTxCount(), - conf.ledgerMaxInstructions(), - conf.ledgerMaxTransactionSizesBytes(), - conf.ledgerMaxReadBytes(), - conf.ledgerMaxWriteBytes(), - conf.ledgerMaxReadLedgerEntries(), - conf.ledgerMaxWriteLedgerEntries()}; - return Resource(limits); + return LedgerManager::maxSorobanLedgerResources( + getSorobanNetworkConfigReadOnly()); } else { - uint32_t maxOpsLedger = getLastMaxTxSetSizeOps(); - return Resource(maxOpsLedger); + return LedgerManager::maxClassicLedgerResources( + mLastClosedLedger.header); } } @@ -475,17 +509,8 @@ LedgerManagerImpl::maxSorobanTransactionResources() { ZoneScoped; - auto const& conf = - mApp.getLedgerManager().getSorobanNetworkConfigReadOnly(); - int64_t const opCount = 1; - std::vector limits = {opCount, - conf.txMaxInstructions(), - conf.txMaxSizeBytes(), - conf.txMaxReadBytes(), - conf.txMaxWriteBytes(), - conf.txMaxReadLedgerEntries(), - conf.txMaxWriteLedgerEntries()}; - return Resource(limits); + return LedgerManager::maxSorobanTransactionResources( + mApp.getLedgerManager().getSorobanNetworkConfigReadOnly()); } int64_t diff --git a/src/ledger/LedgerStateSnapshot.cpp b/src/ledger/LedgerStateSnapshot.cpp index b8c373742e..c48c6967b7 100644 --- a/src/ledger/LedgerStateSnapshot.cpp +++ b/src/ledger/LedgerStateSnapshot.cpp @@ -239,6 +239,11 @@ LedgerSnapshot::LedgerSnapshot(Application& app) app.getLedgerManager().getCurrentLedgerStateSnaphot()); } +LedgerSnapshot::LedgerSnapshot(SearchableSnapshotConstPtr snapshot) + : mGetter(std::make_unique(snapshot)) +{ +} + LedgerHeaderWrapper LedgerSnapshot::getLedgerHeader() const { diff --git a/src/ledger/LedgerStateSnapshot.h b/src/ledger/LedgerStateSnapshot.h index cc9fe382ec..0103ccef7c 100644 --- a/src/ledger/LedgerStateSnapshot.h +++ b/src/ledger/LedgerStateSnapshot.h @@ -143,6 +143,7 @@ class LedgerSnapshot : public NonMovableOrCopyable public: LedgerSnapshot(AbstractLedgerTxn& ltx); LedgerSnapshot(Application& app); + explicit LedgerSnapshot(SearchableSnapshotConstPtr snapshot); LedgerHeaderWrapper getLedgerHeader() const; LedgerEntryWrapper getAccount(AccountID const& account) const; LedgerEntryWrapper diff --git a/src/main/AppConnector.cpp b/src/main/AppConnector.cpp index d282e7eddd..1e5e4385e1 100644 --- a/src/main/AppConnector.cpp +++ b/src/main/AppConnector.cpp @@ -14,33 +14,33 @@ namespace stellar { AppConnector::AppConnector(Application& app) - : mApp(app), mConfig(app.getConfig()) + : mApp(app), mConfig(std::make_shared(app.getConfig())) { } Herder& -AppConnector::getHerder() +AppConnector::getHerder() const { releaseAssert(threadIsMain()); return mApp.getHerder(); } LedgerManager& -AppConnector::getLedgerManager() +AppConnector::getLedgerManager() const { releaseAssert(threadIsMain()); return mApp.getLedgerManager(); } OverlayManager& -AppConnector::getOverlayManager() +AppConnector::getOverlayManager() const { releaseAssert(threadIsMain()); return mApp.getOverlayManager(); } BanManager& -AppConnector::getBanManager() +AppConnector::getBanManager() const { releaseAssert(threadIsMain()); return mApp.getBanManager(); @@ -53,6 +53,17 @@ AppConnector::getSorobanNetworkConfigReadOnly() const return mApp.getLedgerManager().getSorobanNetworkConfigReadOnly(); } +std::optional +AppConnector::maybeGetSorobanNetworkConfigReadOnly() const +{ + releaseAssert(threadIsMain()); + if (mApp.getLedgerManager().hasSorobanNetworkConfig()) + { + return mApp.getLedgerManager().getSorobanNetworkConfigReadOnly(); + } + return std::nullopt; +} + medida::MetricsRegistry& AppConnector::getMetrics() const { @@ -100,6 +111,12 @@ AppConnector::postOnOverlayThread(std::function&& f, Config const& AppConnector::getConfig() const +{ + return *mConfig; +} + +std::shared_ptr +AppConnector::getConfigPtr() const { return mConfig; } @@ -116,6 +133,17 @@ AppConnector::now() const return mApp.getClock().now(); } +VirtualClock::system_time_point +AppConnector::system_now() const +{ + // TODO: Is this thread safe? It looks like it is when in REAL_TIME mode, + // but I'm not so sure about VIRTUAL_TIME mode as that mode has a + // `mVirtualNow` that looks like it can change during access? The same is + // true for `AppConnector::now`, which is marked "thread safe" in the header + // file. Maybe both of these need some hardening though? + return mApp.getClock().system_now(); +} + bool AppConnector::shouldYield() const { diff --git a/src/main/AppConnector.h b/src/main/AppConnector.h index ec35925133..3cef08b670 100644 --- a/src/main/AppConnector.h +++ b/src/main/AppConnector.h @@ -23,18 +23,26 @@ class AppConnector Application& mApp; // Copy config for threads to use, and avoid warnings from thread sanitizer // about accessing mApp - Config const mConfig; + std::shared_ptr const mConfig; public: AppConnector(Application& app); // Methods that can only be called from main thread - Herder& getHerder(); - LedgerManager& getLedgerManager(); - OverlayManager& getOverlayManager(); - BanManager& getBanManager(); + Herder& getHerder() const; + LedgerManager& getLedgerManager() const; + OverlayManager& getOverlayManager() const; + BanManager& getBanManager() const; bool shouldYield() const; SorobanNetworkConfig const& getSorobanNetworkConfigReadOnly() const; + // TODO: Docs. Mention that the difference between this and `getSorobanNetowrkConfig` is that: + // 1. This makes a copy, which is safe to use in other threads (TODO: Really + // double check this. I don't see any references or pointers in + // SorobanNetworkConfig, but there is a `mutable` field, which needs to be + // investigated as it throws `const` functions into question). + // 2. This returns nullopt when the network config is not set, while + // `getSorobanNetworkConfig` will throw an assertion error in that case. + std::optional maybeGetSorobanNetworkConfigReadOnly() const; medida::MetricsRegistry& getMetrics() const; SorobanMetrics& getSorobanMetrics() const; void checkOnOperationApply(Operation const& operation, @@ -49,7 +57,9 @@ class AppConnector void postOnOverlayThread(std::function&& f, std::string const& message); VirtualClock::time_point now() const; + VirtualClock::system_time_point system_now() const; Config const& getConfig() const; + std::shared_ptr getConfigPtr() const; bool overlayShuttingDown() const; OverlayMetrics& getOverlayMetrics(); // This method is always exclusively called from one thread diff --git a/src/main/Application.h b/src/main/Application.h index ae23517a57..7578f057a3 100644 --- a/src/main/Application.h +++ b/src/main/Application.h @@ -178,7 +178,7 @@ class Application // Return a reference to the Application-local copy of the Config object // that the Application was constructed with. - virtual Config const& getConfig() = 0; + virtual Config const& getConfig() const = 0; // Gets the current execution-state of the Application // (derived from the state of other modules @@ -206,7 +206,7 @@ class Application // Get references to each of the "subsystem" objects. virtual TmpDirManager& getTmpDirManager() = 0; virtual LedgerManager& getLedgerManager() = 0; - virtual BucketManager& getBucketManager() = 0; + virtual BucketManager& getBucketManager() const = 0; virtual CatchupManager& getCatchupManager() = 0; virtual HistoryArchiveManager& getHistoryArchiveManager() = 0; virtual HistoryManager& getHistoryManager() = 0; @@ -306,7 +306,7 @@ class Application // instances virtual Hash const& getNetworkID() const = 0; - virtual AbstractLedgerTxnParent& getLedgerTxnRoot() = 0; + virtual AbstractLedgerTxnParent& getLedgerTxnRoot() const = 0; virtual void validateAndLogConfig() = 0; @@ -327,7 +327,7 @@ class Application return ret; } - virtual AppConnector& getAppConnector() = 0; + virtual AppConnector& getAppConnector() const = 0; protected: Application() diff --git a/src/main/ApplicationImpl.cpp b/src/main/ApplicationImpl.cpp index 482ceef177..c98f169522 100644 --- a/src/main/ApplicationImpl.cpp +++ b/src/main/ApplicationImpl.cpp @@ -1091,7 +1091,7 @@ ApplicationImpl::applyCfgCommands() } Config const& -ApplicationImpl::getConfig() +ApplicationImpl::getConfig() const { return mConfig; } @@ -1230,7 +1230,7 @@ ApplicationImpl::getLedgerManager() } BucketManager& -ApplicationImpl::getBucketManager() +ApplicationImpl::getBucketManager() const { return *mBucketManager; } @@ -1442,7 +1442,7 @@ ApplicationImpl::createDatabase() } AbstractLedgerTxnParent& -ApplicationImpl::getLedgerTxnRoot() +ApplicationImpl::getLedgerTxnRoot() const { releaseAssert(threadIsMain()); @@ -1457,7 +1457,7 @@ ApplicationImpl::getLedgerTxnRoot() } AppConnector& -ApplicationImpl::getAppConnector() +ApplicationImpl::getAppConnector() const { return *mAppConnector; } diff --git a/src/main/ApplicationImpl.h b/src/main/ApplicationImpl.h index 1fc7ea989c..707b54d6b5 100644 --- a/src/main/ApplicationImpl.h +++ b/src/main/ApplicationImpl.h @@ -49,7 +49,7 @@ class ApplicationImpl : public Application virtual uint64_t timeNow() override; - virtual Config const& getConfig() override; + virtual Config const& getConfig() const override; virtual State getState() const override; virtual std::string getStateHuman() const override; @@ -61,7 +61,7 @@ class ApplicationImpl : public Application virtual void clearMetrics(std::string const& domain) override; virtual TmpDirManager& getTmpDirManager() override; virtual LedgerManager& getLedgerManager() override; - virtual BucketManager& getBucketManager() override; + virtual BucketManager& getBucketManager() const override; virtual CatchupManager& getCatchupManager() override; virtual HistoryArchiveManager& getHistoryArchiveManager() override; virtual HistoryManager& getHistoryManager() override; @@ -77,7 +77,7 @@ class ApplicationImpl : public Application virtual WorkScheduler& getWorkScheduler() override; virtual BanManager& getBanManager() override; virtual StatusManager& getStatusManager() override; - virtual AppConnector& getAppConnector() override; + virtual AppConnector& getAppConnector() const override; virtual asio::io_context& getWorkerIOContext() override; virtual asio::io_context& getEvictionIOContext() override; @@ -133,7 +133,7 @@ class ApplicationImpl : public Application virtual Hash const& getNetworkID() const override; - virtual AbstractLedgerTxnParent& getLedgerTxnRoot() override; + virtual AbstractLedgerTxnParent& getLedgerTxnRoot() const override; private: VirtualClock& mVirtualClock; diff --git a/src/main/Config.cpp b/src/main/Config.cpp index 0d13451681..b3172106c9 100644 --- a/src/main/Config.cpp +++ b/src/main/Config.cpp @@ -157,6 +157,7 @@ Config::Config() : NODE_SEED(SecretKey::random()) CATCHUP_COMPLETE = false; CATCHUP_RECENT = 0; BACKGROUND_OVERLAY_PROCESSING = true; + BACKGROUND_TX_QUEUE = false; BUCKETLIST_DB_INDEX_PAGE_SIZE_EXPONENT = 14; // 2^14 == 16 kb BUCKETLIST_DB_INDEX_CUTOFF = 20; // 20 mb BUCKETLIST_DB_PERSIST_INDEX = true; @@ -1065,6 +1066,8 @@ Config::processConfig(std::shared_ptr t) }}, {"BACKGROUND_OVERLAY_PROCESSING", [&]() { BACKGROUND_OVERLAY_PROCESSING = readBool(item); }}, + {"EXPERIMENTAL_BACKGROUND_TX_QUEUE", + [&]() { BACKGROUND_TX_QUEUE = readBool(item); }}, // https://github.com/stellar/stellar-core/issues/4581 {"BACKGROUND_EVICTION_SCAN", [&]() { diff --git a/src/main/Config.h b/src/main/Config.h index 6e4e1e7418..0e365372d2 100644 --- a/src/main/Config.h +++ b/src/main/Config.h @@ -466,6 +466,11 @@ class Config : public std::enable_shared_from_this // Enable parallel processing of overlay operations (experimental) bool BACKGROUND_OVERLAY_PROCESSING; + // TODO: Docs, both here and in the example cfg + // TODO: Require BACKGROUND_OVERLAY_PROCESSING to be set to true if this is + // enabled? + bool BACKGROUND_TX_QUEUE; + // When set to true, BucketListDB indexes are persisted on-disk so that the // BucketList does not need to be reindexed on startup. Defaults to true. // This should only be set to false for testing purposes diff --git a/src/main/test/CommandHandlerTests.cpp b/src/main/test/CommandHandlerTests.cpp index 3e4f5df9d9..3decfbb758 100644 --- a/src/main/test/CommandHandlerTests.cpp +++ b/src/main/test/CommandHandlerTests.cpp @@ -502,7 +502,8 @@ TEST_CASE("manualclose", "[commandhandler]") setMinTime(txFrame, 0); TimePoint const maxTime = lastCloseTime() + defaultManualCloseTimeInterval + - getUpperBoundCloseTimeOffset(*app, lastCloseTime()); + getUpperBoundCloseTimeOffset(app->getAppConnector(), + lastCloseTime()); setMaxTime(txFrame, maxTime); txFrame->getMutableEnvelope().v1().signatures.clear(); txFrame->addSignature(root); diff --git a/src/simulation/ApplyLoad.cpp b/src/simulation/ApplyLoad.cpp index bbf9eb6e08..0aa875d9bf 100644 --- a/src/simulation/ApplyLoad.cpp +++ b/src/simulation/ApplyLoad.cpp @@ -396,6 +396,7 @@ ApplyLoad::benchmark() gRandomEngine); bool limitHit = false; + AppValidationWrapper avw(mApp.getAppConnector(), false); for (auto accountIndex : shuffledAccounts) { auto it = accounts.find(accountIndex); @@ -407,8 +408,7 @@ ApplyLoad::benchmark() { LedgerTxn ltx(mApp.getLedgerTxnRoot()); - auto res = tx.second->checkValid(mApp.getAppConnector(), ltx, 0, 0, - UINT64_MAX); + auto res = tx.second->checkValid(avw, ltx, 0, 0, UINT64_MAX); releaseAssert((res && res->isSuccess())); } diff --git a/src/test/FuzzerImpl.cpp b/src/test/FuzzerImpl.cpp index afa750f206..55b5151bc1 100644 --- a/src/test/FuzzerImpl.cpp +++ b/src/test/FuzzerImpl.cpp @@ -920,15 +920,15 @@ class FuzzTransactionFrame : public TransactionFrame // attempt application of transaction without processing the fee or // committing the LedgerTxn + AppValidationWrapper avw(app.getAppConnector(), false); SignatureChecker signatureChecker{ ltx.loadHeader().current().ledgerVersion, getContentsHash(), mEnvelope.v1().signatures}; LedgerSnapshot ltxStmt(ltx); // if any ill-formed Operations, do not attempt transaction application auto isInvalidOperation = [&](auto const& op, auto& opResult) { - return !op->checkValid(app.getAppConnector(), signatureChecker, - ltxStmt, false, opResult, - mTxResult->getSorobanData()); + return !op->checkValid(avw, signatureChecker, ltxStmt, false, + opResult, mTxResult->getSorobanData()); }; auto const& ops = getOperations(); diff --git a/src/transactions/FeeBumpTransactionFrame.cpp b/src/transactions/FeeBumpTransactionFrame.cpp index f8ad199508..ce704ff0b7 100644 --- a/src/transactions/FeeBumpTransactionFrame.cpp +++ b/src/transactions/FeeBumpTransactionFrame.cpp @@ -153,14 +153,13 @@ FeeBumpTransactionFrame::checkSignature(SignatureChecker& signatureChecker, } MutableTxResultPtr -FeeBumpTransactionFrame::checkValid(AppConnector& app, LedgerSnapshot const& ls, +FeeBumpTransactionFrame::checkValid(ValidationConnector const& vc, + LedgerSnapshot const& ls, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const { - if (!isTransactionXDRValidForProtocol( - ls.getLedgerHeader().current().ledgerVersion, app.getConfig(), - mEnvelope) || + if (!isTransactionXDRValidForCurrentProtocol(vc, mEnvelope) || !XDRProvidesValidFee()) { auto txResult = createSuccessResult(); @@ -188,7 +187,7 @@ FeeBumpTransactionFrame::checkValid(AppConnector& app, LedgerSnapshot const& ls, } auto innerTxResult = mInnerTx->checkValidWithOptionallyChargedFee( - app, ls, current, false, lowerBoundCloseTimeOffset, + vc, ls, current, false, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); auto finalTxResult = createSuccessResultWithNewInnerTx( std::move(txResult), std::move(innerTxResult), mInnerTx); @@ -198,10 +197,10 @@ FeeBumpTransactionFrame::checkValid(AppConnector& app, LedgerSnapshot const& ls, bool FeeBumpTransactionFrame::checkSorobanResourceAndSetError( - AppConnector& app, SorobanNetworkConfig const& cfg, uint32_t ledgerVersion, + ValidationConnector const& vc, uint32_t ledgerVersion, MutableTxResultPtr txResult) const { - return mInnerTx->checkSorobanResourceAndSetError(app, cfg, ledgerVersion, + return mInnerTx->checkSorobanResourceAndSetError(vc, ledgerVersion, txResult); } diff --git a/src/transactions/FeeBumpTransactionFrame.h b/src/transactions/FeeBumpTransactionFrame.h index 8d005612c5..1afdd1d6c7 100644 --- a/src/transactions/FeeBumpTransactionFrame.h +++ b/src/transactions/FeeBumpTransactionFrame.h @@ -77,12 +77,13 @@ class FeeBumpTransactionFrame : public TransactionFrameBase MutableTxResultPtr txResult) const override; MutableTxResultPtr - checkValid(AppConnector& app, LedgerSnapshot const& ls, + checkValid(ValidationConnector const& app, LedgerSnapshot const& ls, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const override; - bool checkSorobanResourceAndSetError( - AppConnector& app, SorobanNetworkConfig const& cfg, - uint32_t ledgerVersion, MutableTxResultPtr txResult) const override; + bool + checkSorobanResourceAndSetError(ValidationConnector const& app, + uint32_t ledgerVersion, + MutableTxResultPtr txResult) const override; MutableTxResultPtr createSuccessResult() const override; diff --git a/src/transactions/OperationFrame.cpp b/src/transactions/OperationFrame.cpp index ff14610986..4c1156da27 100644 --- a/src/transactions/OperationFrame.cpp +++ b/src/transactions/OperationFrame.cpp @@ -143,9 +143,10 @@ OperationFrame::apply(AppConnector& app, SignatureChecker& signatureChecker, ZoneScoped; CLOG_TRACE(Tx, "{}", xdrToCerealString(mOperation, "Operation")); + AppValidationWrapper avw(app, true); LedgerSnapshot ltxState(ltx); bool applyRes = - checkValid(app, signatureChecker, ltxState, true, res, sorobanData); + checkValid(avw, signatureChecker, ltxState, true, res, sorobanData); if (applyRes) { applyRes = doApply(app, ltx, sorobanBasePrngSeed, res, sorobanData); @@ -217,7 +218,7 @@ OperationFrame::getSourceID() const // make sure sig is correct // verifies that the operation is well formed (operation specific) bool -OperationFrame::checkValid(AppConnector& app, +OperationFrame::checkValid(ValidationConnector const& vc, SignatureChecker& signatureChecker, LedgerSnapshot const& ls, bool forApply, OperationResult& res, @@ -225,8 +226,7 @@ OperationFrame::checkValid(AppConnector& app, { ZoneScoped; bool validationResult = false; - auto validate = [this, &res, forApply, &signatureChecker, &app, - &sorobanData, + auto validate = [this, &res, forApply, &signatureChecker, &vc, &sorobanData, &validationResult](LedgerSnapshot const& ls) { if (!isOpSupported(ls.getLedgerHeader().current())) { @@ -262,11 +262,10 @@ OperationFrame::checkValid(AppConnector& app, isSoroban()) { releaseAssertOrThrow(sorobanData); - auto const& sorobanConfig = - app.getLedgerManager().getSorobanNetworkConfigForApply(); + auto const& sorobanConfig = vc.getSorobanNetworkConfig(); validationResult = - doCheckValidForSoroban(sorobanConfig, app.getConfig(), + doCheckValidForSoroban(sorobanConfig, vc.getConfig(), ledgerVersion, res, *sorobanData); } else diff --git a/src/transactions/OperationFrame.h b/src/transactions/OperationFrame.h index 5cc8aa6641..c02d3f3c53 100644 --- a/src/transactions/OperationFrame.h +++ b/src/transactions/OperationFrame.h @@ -73,7 +73,8 @@ class OperationFrame AccountID getSourceID() const; - bool checkValid(AppConnector& app, SignatureChecker& signatureChecker, + bool checkValid(ValidationConnector const& vc, + SignatureChecker& signatureChecker, LedgerSnapshot const& ls, bool forApply, OperationResult& res, std::shared_ptr sorobanData) const; diff --git a/src/transactions/TransactionFrame.cpp b/src/transactions/TransactionFrame.cpp index 223817b92c..0752a449fb 100644 --- a/src/transactions/TransactionFrame.cpp +++ b/src/transactions/TransactionFrame.cpp @@ -875,8 +875,7 @@ TransactionFrame::isTooEarlyForAccount(LedgerHeaderWrapper const& header, bool TransactionFrame::commonValidPreSeqNum( - AppConnector& app, std::optional const& cfg, - LedgerSnapshot const& ls, bool chargeFee, + ValidationConnector const& vc, LedgerSnapshot const& ls, bool chargeFee, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, std::optional sorobanResourceFee, MutableTxResultPtr txResult) const @@ -946,9 +945,7 @@ TransactionFrame::commonValidPreSeqNum( return false; } - releaseAssert(cfg); - if (!checkSorobanResourceAndSetError(app, cfg.value(), ledgerVersion, - txResult)) + if (!checkSorobanResourceAndSetError(vc, ledgerVersion, txResult)) { return false; } @@ -958,7 +955,7 @@ TransactionFrame::commonValidPreSeqNum( if (sorobanData.resourceFee > getFullFee()) { sorobanTxData.pushValidationTimeDiagnosticError( - app.getConfig(), SCE_STORAGE, SCEC_EXCEEDED_LIMIT, + vc.getConfig(), SCE_STORAGE, SCEC_EXCEEDED_LIMIT, "transaction `sorobanData.resourceFee` is higher than the " "full transaction fee", {makeU64SCVal(sorobanData.resourceFee), @@ -972,7 +969,7 @@ TransactionFrame::commonValidPreSeqNum( INT64_MAX - sorobanResourceFee->non_refundable_fee) { sorobanTxData.pushValidationTimeDiagnosticError( - app.getConfig(), SCE_STORAGE, SCEC_INVALID_INPUT, + vc.getConfig(), SCE_STORAGE, SCEC_INVALID_INPUT, "transaction resource fees cannot be added", {makeU64SCVal(sorobanResourceFee->refundable_fee), makeU64SCVal(sorobanResourceFee->non_refundable_fee)}); @@ -985,7 +982,7 @@ TransactionFrame::commonValidPreSeqNum( if (sorobanData.resourceFee < resourceFees) { sorobanTxData.pushValidationTimeDiagnosticError( - app.getConfig(), SCE_STORAGE, SCEC_EXCEEDED_LIMIT, + vc.getConfig(), SCE_STORAGE, SCEC_EXCEEDED_LIMIT, "transaction `sorobanData.resourceFee` is lower than the " "actual Soroban resource fee", {makeU64SCVal(sorobanData.resourceFee), @@ -1004,7 +1001,7 @@ TransactionFrame::commonValidPreSeqNum( if (!set.emplace(lk).second) { sorobanTxData.pushValidationTimeDiagnosticError( - app.getConfig(), SCE_STORAGE, SCEC_INVALID_INPUT, + vc.getConfig(), SCE_STORAGE, SCEC_INVALID_INPUT, "Found duplicate key in the Soroban footprint; every " "key across read-only and read-write footprints has to " "be unique.", @@ -1183,8 +1180,7 @@ TransactionFrame::isBadSeq(LedgerHeaderWrapper const& header, } TransactionFrame::ValidationType -TransactionFrame::commonValid(AppConnector& app, - std::optional const& cfg, +TransactionFrame::commonValid(ValidationConnector const& vc, SignatureChecker& signatureChecker, LedgerSnapshot const& ls, SequenceNumber current, bool applying, bool chargeFee, @@ -1198,9 +1194,9 @@ TransactionFrame::commonValid(AppConnector& app, ValidationType res = ValidationType::kInvalid; auto validate = [this, &signatureChecker, applying, - lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset, &app, - chargeFee, sorobanResourceFee, txResult, ¤t, &res, - cfg](LedgerSnapshot const& ls) { + lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset, &vc, + chargeFee, sorobanResourceFee, txResult, ¤t, + &res](LedgerSnapshot const& ls) { if (applying && (lowerBoundCloseTimeOffset != 0 || upperBoundCloseTimeOffset != 0)) { @@ -1208,9 +1204,9 @@ TransactionFrame::commonValid(AppConnector& app, "Applying transaction with non-current closeTime"); } - if (!commonValidPreSeqNum( - app, cfg, ls, chargeFee, lowerBoundCloseTimeOffset, - upperBoundCloseTimeOffset, sorobanResourceFee, txResult)) + if (!commonValidPreSeqNum(vc, ls, chargeFee, lowerBoundCloseTimeOffset, + upperBoundCloseTimeOffset, sorobanResourceFee, + txResult)) { return; } @@ -1415,8 +1411,8 @@ TransactionFrame::removeAccountSigner(AbstractLedgerTxn& ltxOuter, MutableTxResultPtr TransactionFrame::checkValidWithOptionallyChargedFee( - AppConnector& app, LedgerSnapshot const& ls, SequenceNumber current, - bool chargeFee, uint64_t lowerBoundCloseTimeOffset, + ValidationConnector const& vc, LedgerSnapshot const& ls, + SequenceNumber current, bool chargeFee, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const { ZoneScoped; @@ -1443,20 +1439,17 @@ TransactionFrame::checkValidWithOptionallyChargedFee( getSignatures(mEnvelope)}; std::optional sorobanResourceFee; - std::optional sorobanConfig; if (protocolVersionStartsFrom(ls.getLedgerHeader().current().ledgerVersion, SOROBAN_PROTOCOL_VERSION) && isSoroban()) { - sorobanConfig = - app.getLedgerManager().getSorobanNetworkConfigReadOnly(); sorobanResourceFee = computePreApplySorobanResourceFee( - ls.getLedgerHeader().current().ledgerVersion, sorobanConfig.value(), - app.getConfig()); + ls.getLedgerHeader().current().ledgerVersion, + vc.getSorobanNetworkConfig(), vc.getConfig()); } - bool res = commonValid(app, sorobanConfig, signatureChecker, ls, current, - false, chargeFee, lowerBoundCloseTimeOffset, - upperBoundCloseTimeOffset, sorobanResourceFee, + bool res = commonValid(vc, signatureChecker, ls, current, false, chargeFee, + lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset, + sorobanResourceFee, txResult) == ValidationType::kMaybeValid; if (res) { @@ -1465,7 +1458,7 @@ TransactionFrame::checkValidWithOptionallyChargedFee( auto const& op = mOperations[i]; auto& opResult = txResult->getOpResultAt(i); - if (!op->checkValid(app, signatureChecker, ls, false, opResult, + if (!op->checkValid(vc, signatureChecker, ls, false, opResult, txResult->getSorobanData())) { // it's OK to just fast fail here and not try to call @@ -1486,8 +1479,8 @@ TransactionFrame::checkValidWithOptionallyChargedFee( } MutableTxResultPtr -TransactionFrame::checkValid(AppConnector& app, LedgerSnapshot const& ls, - SequenceNumber current, +TransactionFrame::checkValid(ValidationConnector const& vc, + LedgerSnapshot const& ls, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const { @@ -1495,26 +1488,24 @@ TransactionFrame::checkValid(AppConnector& app, LedgerSnapshot const& ls, // `checkValidWithOptionallyChargedFee` in order to not validate the // envelope XDR twice for the fee bump transactions (they use // `checkValidWithOptionallyChargedFee` for the inner tx). - if (!isTransactionXDRValidForProtocol( - ls.getLedgerHeader().current().ledgerVersion, app.getConfig(), - mEnvelope)) + if (!isTransactionXDRValidForCurrentProtocol(vc, mEnvelope)) { auto txResult = createSuccessResult(); txResult->setResultCode(txMALFORMED); return txResult; } - return checkValidWithOptionallyChargedFee(app, ls, current, true, + return checkValidWithOptionallyChargedFee(vc, ls, current, true, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); } bool TransactionFrame::checkSorobanResourceAndSetError( - AppConnector& app, SorobanNetworkConfig const& cfg, uint32_t ledgerVersion, + ValidationConnector const& vc, uint32_t ledgerVersion, MutableTxResultPtr txResult) const { - if (!validateSorobanResources(cfg, app.getConfig(), ledgerVersion, - *txResult->getSorobanData())) + if (!validateSorobanResources(vc.getSorobanNetworkConfig(), vc.getConfig(), + ledgerVersion, *txResult->getSorobanData())) { txResult->setInnermostResultCode(txSOROBAN_INVALID); return false; @@ -1838,8 +1829,6 @@ TransactionFrame::apply(AppConnector& app, AbstractLedgerTxn& ltx, SOROBAN_PROTOCOL_VERSION) && isSoroban()) { - sorobanConfig = - app.getLedgerManager().getSorobanNetworkConfigForApply(); sorobanResourceFee = computePreApplySorobanResourceFee( ledgerVersion, *sorobanConfig, app.getConfig()); @@ -1852,9 +1841,9 @@ TransactionFrame::apply(AppConnector& app, AbstractLedgerTxn& ltx, } LedgerTxn ltxTx(ltx); LedgerSnapshot ltxStmt(ltxTx); - auto cv = - commonValid(app, sorobanConfig, *signatureChecker, ltxStmt, 0, true, - chargeFee, 0, 0, sorobanResourceFee, txResult); + AppValidationWrapper avw(app, true); + auto cv = commonValid(avw, *signatureChecker, ltxStmt, 0, true, + chargeFee, 0, 0, sorobanResourceFee, txResult); if (cv >= ValidationType::kInvalidUpdateSeqNum) { processSeqNum(ltxTx); diff --git a/src/transactions/TransactionFrame.h b/src/transactions/TransactionFrame.h index 96811b6312..cbfd8a0e0e 100644 --- a/src/transactions/TransactionFrame.h +++ b/src/transactions/TransactionFrame.h @@ -94,9 +94,8 @@ class TransactionFrame : public TransactionFrameBase LedgerEntryWrapper const& sourceAccount, uint64_t lowerBoundCloseTimeOffset) const; - bool commonValidPreSeqNum(AppConnector& app, - std::optional const& cfg, - LedgerSnapshot const& ls, bool chargeFee, + bool commonValidPreSeqNum(ValidationConnector const& vc, LedgerSnapshot const& ls, + bool chargeFee, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, std::optional sorobanResourceFee, @@ -105,8 +104,7 @@ class TransactionFrame : public TransactionFrameBase virtual bool isBadSeq(LedgerHeaderWrapper const& header, int64_t seqNum) const; - ValidationType commonValid(AppConnector& app, - std::optional const& cfg, + ValidationType commonValid(ValidationConnector const& vc, SignatureChecker& signatureChecker, LedgerSnapshot const& ls, SequenceNumber current, bool applying, bool chargeFee, @@ -210,16 +208,18 @@ class TransactionFrame : public TransactionFrameBase bool checkExtraSigners(SignatureChecker& signatureChecker) const; MutableTxResultPtr checkValidWithOptionallyChargedFee( - AppConnector& app, LedgerSnapshot const& ls, SequenceNumber current, - bool chargeFee, uint64_t lowerBoundCloseTimeOffset, + ValidationConnector const& vc, LedgerSnapshot const& ls, + SequenceNumber current, bool chargeFee, + uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const; MutableTxResultPtr - checkValid(AppConnector& app, LedgerSnapshot const& ls, + checkValid(ValidationConnector const& vc, LedgerSnapshot const& ls, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const override; - bool checkSorobanResourceAndSetError( - AppConnector& app, SorobanNetworkConfig const& cfg, - uint32_t ledgerVersion, MutableTxResultPtr txResult) const override; + bool + checkSorobanResourceAndSetError(ValidationConnector const& vc, + uint32_t ledgerVersion, + MutableTxResultPtr txResult) const override; MutableTxResultPtr createSuccessResult() const override; diff --git a/src/transactions/TransactionFrameBase.cpp b/src/transactions/TransactionFrameBase.cpp index a1cc4dcc74..8bfdb95c57 100644 --- a/src/transactions/TransactionFrameBase.cpp +++ b/src/transactions/TransactionFrameBase.cpp @@ -3,12 +3,72 @@ // of this distribution or at http://www.apache.org/licenses/LICENSE-2.0 #include "transactions/TransactionFrameBase.h" +#include "ledger/LedgerManager.h" +#include "main/AppConnector.h" #include "transactions/FeeBumpTransactionFrame.h" #include "transactions/TransactionFrame.h" namespace stellar { +AppValidationWrapper::AppValidationWrapper(AppConnector const& app, + bool forApply) + : mApp(app), mForApply(forApply) +{ +} + +Config const& +AppValidationWrapper::getConfig() const +{ + return mApp.getConfig(); +} + +SorobanNetworkConfig const& +AppValidationWrapper::getSorobanNetworkConfig() const +{ + return mForApply ? mApp.getLedgerManager().getSorobanNetworkConfigForApply() + : mApp.getSorobanNetworkConfigReadOnly(); +} + +uint32_t +AppValidationWrapper::getCurrentProtocolVersion() const +{ + return mApp.getLedgerManager() + .getLastClosedLedgerHeader() + .header.ledgerVersion; +} + +ImmutableValidationSnapshot::ImmutableValidationSnapshot( + AppConnector const& app) + : mConfig(app.getConfigPtr()) + , mSorobanNetworkConfig(app.maybeGetSorobanNetworkConfigReadOnly()) + , mCurrentProtocolVersion(app.getLedgerManager() + .getLastClosedLedgerHeader() + .header.ledgerVersion) +{ + releaseAssert(threadIsMain()); +} + +Config const& +ImmutableValidationSnapshot::getConfig() const +{ + return *mConfig; +} + +SorobanNetworkConfig const& +ImmutableValidationSnapshot::getSorobanNetworkConfig() const +{ + // TODO: This can throw. Check and throw a more usefull exception instead. + // Also document this. + return mSorobanNetworkConfig.value(); +} + +uint32_t +ImmutableValidationSnapshot::getCurrentProtocolVersion() const +{ + return mCurrentProtocolVersion; +} + TransactionFrameBasePtr TransactionFrameBase::makeTransactionFromWire(Hash const& networkID, TransactionEnvelope const& env) diff --git a/src/transactions/TransactionFrameBase.h b/src/transactions/TransactionFrameBase.h index 94763cdeae..369b2b4d86 100644 --- a/src/transactions/TransactionFrameBase.h +++ b/src/transactions/TransactionFrameBase.h @@ -35,6 +35,57 @@ using TransactionFrameBasePtr = std::shared_ptr; using TransactionFrameBaseConstPtr = std::shared_ptr; +// TODO: Explain why this exists. Allows common validation flows between "apply" +// and "validate" to work whether given an immutable snapshot (for use in +// "validate") or a wrapper around the AppConnector (for use in "apply"). +// AppValidationWrapper is also usable outside of "apply" by setting "forApply" +// to false. This is useful for some of our testing infrastructure. +class ValidationConnector +{ + public: + virtual ~ValidationConnector() = default; + virtual Config const& getConfig() const = 0; + virtual SorobanNetworkConfig const& getSorobanNetworkConfig() const = 0; + virtual uint32_t getCurrentProtocolVersion() const = 0; +}; + +// TODO: Docs +class AppValidationWrapper : public ValidationConnector +{ + public: + explicit AppValidationWrapper(AppConnector const& app, bool forApply); + ~AppValidationWrapper() override = default; + + Config const& getConfig() const override; + SorobanNetworkConfig const& getSorobanNetworkConfig() const override; + uint32_t getCurrentProtocolVersion() const override; + + private: + AppConnector const& mApp; + bool const mForApply; +}; + +// TODO: Docs +class ImmutableValidationSnapshot : public ValidationConnector +{ + public: + explicit ImmutableValidationSnapshot(AppConnector const& app); + ~ImmutableValidationSnapshot() override = default; + + Config const& getConfig() const override; + SorobanNetworkConfig const& getSorobanNetworkConfig() const override; + uint32_t getCurrentProtocolVersion() const override; + + private: + std::shared_ptr const mConfig; + std::optional const mSorobanNetworkConfig; + uint32_t const mCurrentProtocolVersion; +}; + +// TODO: Rename to indicate constness / Immutableness? +using ImmutableValidationSnapshotPtr = + std::shared_ptr; + class TransactionFrameBase { public: @@ -46,12 +97,13 @@ class TransactionFrameBase TransactionMetaFrame& meta, MutableTxResultPtr txResult, Hash const& sorobanBasePrngSeed = Hash{}) const = 0; virtual MutableTxResultPtr - checkValid(AppConnector& app, LedgerSnapshot const& ls, + checkValid(ValidationConnector const& vc, LedgerSnapshot const& ls, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const = 0; - virtual bool checkSorobanResourceAndSetError( - AppConnector& app, SorobanNetworkConfig const& cfg, - uint32_t ledgerVersion, MutableTxResultPtr txResult) const = 0; + virtual bool + checkSorobanResourceAndSetError(ValidationConnector const& vc, + uint32_t ledgerVersion, + MutableTxResultPtr txResult) const = 0; virtual MutableTxResultPtr createSuccessResult() const = 0; diff --git a/src/transactions/TransactionUtils.cpp b/src/transactions/TransactionUtils.cpp index 15a177906b..bd15fa32f1 100644 --- a/src/transactions/TransactionUtils.cpp +++ b/src/transactions/TransactionUtils.cpp @@ -1257,9 +1257,9 @@ trustLineFlagIsValid(uint32_t flag, LedgerTxnHeader const& header) } uint64_t -getUpperBoundCloseTimeOffset(Application& app, uint64_t lastCloseTime) +getUpperBoundCloseTimeOffset(AppConnector& app, uint64_t lastCloseTime) { - uint64_t currentTime = VirtualClock::to_time_t(app.getClock().system_now()); + uint64_t currentTime = VirtualClock::to_time_t(app.system_now()); // account for the time between closeTime and now uint64_t closeTimeDrift = @@ -1980,10 +1980,11 @@ hasMuxedAccount(TransactionEnvelope const& e) } bool -isTransactionXDRValidForProtocol(uint32_t currProtocol, Config const& cfg, - TransactionEnvelope const& envelope) +isTransactionXDRValidForCurrentProtocol(ValidationConnector const& vc, + TransactionEnvelope const& envelope) { - uint32_t maxProtocol = cfg.CURRENT_LEDGER_PROTOCOL_VERSION; + uint32_t maxProtocol = vc.getConfig().CURRENT_LEDGER_PROTOCOL_VERSION; + uint32_t currProtocol = vc.getCurrentProtocolVersion(); // If we could parse the XDR when ledger is using the maximum supported // protocol version, then XDR has to be valid. // This check also is pointless before protocol 21 as Soroban environment diff --git a/src/transactions/TransactionUtils.h b/src/transactions/TransactionUtils.h index 5c051fe1e0..146b3411de 100644 --- a/src/transactions/TransactionUtils.h +++ b/src/transactions/TransactionUtils.h @@ -30,6 +30,7 @@ class SorobanNetworkConfig; class TransactionFrame; class TransactionFrameBase; class SorobanTxData; +class ValidationConnector; struct ClaimAtom; struct LedgerHeader; struct LedgerKey; @@ -261,10 +262,11 @@ bool accountFlagMaskCheckIsValid(uint32_t flag, uint32_t ledgerVersion); bool hasMuxedAccount(TransactionEnvelope const& e); -bool isTransactionXDRValidForProtocol(uint32_t currProtocol, Config const& cfg, - TransactionEnvelope const& envelope); +bool +isTransactionXDRValidForCurrentProtocol(ValidationConnector const& vc, + TransactionEnvelope const& envelope); -uint64_t getUpperBoundCloseTimeOffset(Application& app, uint64_t lastCloseTime); +uint64_t getUpperBoundCloseTimeOffset(AppConnector& app, uint64_t lastCloseTime); bool hasAccountEntryExtV2(AccountEntry const& ae); bool hasAccountEntryExtV3(AccountEntry const& ae); diff --git a/src/transactions/test/InvokeHostFunctionTests.cpp b/src/transactions/test/InvokeHostFunctionTests.cpp index 27a91a1c2f..e68859d3c7 100644 --- a/src/transactions/test/InvokeHostFunctionTests.cpp +++ b/src/transactions/test/InvokeHostFunctionTests.cpp @@ -421,8 +421,8 @@ TEST_CASE("basic contract invocation", "[tx][soroban]") addContractKeys); auto tx = invocation.createTx(&rootAccount); - auto result = - tx->checkValid(test.getApp().getAppConnector(), rootLtx, 0, 0, 0); + AppValidationWrapper avw(test.getApp().getAppConnector(), false); + auto result = tx->checkValid(avw, rootLtx, 0, 0, 0); REQUIRE(tx->getFullFee() == spec.getInclusionFee() + spec.getResourceFee()); @@ -742,6 +742,7 @@ TEST_CASE("Soroban footprint validation", "[tx][soroban]") { SorobanTest test; auto const& cfg = test.getNetworkCfg(); + AppValidationWrapper const avw(test.getApp().getAppConnector(), false); auto& addContract = test.deployWasmContract(rust_bridge::get_test_wasm_add_i32()); @@ -760,8 +761,7 @@ TEST_CASE("Soroban footprint validation", "[tx][soroban]") MutableTxResultPtr result; { LedgerTxn ltx(test.getApp().getLedgerTxnRoot()); - result = - tx->checkValid(test.getApp().getAppConnector(), ltx, 0, 0, 0); + result = tx->checkValid(avw, ltx, 0, 0, 0); } REQUIRE(result->isSuccess() == shouldBeValid); @@ -777,8 +777,7 @@ TEST_CASE("Soroban footprint validation", "[tx][soroban]") MutableTxResultPtr result; { LedgerTxn ltx(test.getApp().getLedgerTxnRoot()); - result = - tx->checkValid(test.getApp().getAppConnector(), ltx, 0, 0, 0); + result = tx->checkValid(avw, ltx, 0, 0, 0); } REQUIRE(result->isSuccess() == shouldBeValid); if (!shouldBeValid) @@ -806,8 +805,7 @@ TEST_CASE("Soroban footprint validation", "[tx][soroban]") MutableTxResultPtr result; { LedgerTxn ltx(test.getApp().getLedgerTxnRoot()); - result = - tx->checkValid(test.getApp().getAppConnector(), ltx, 0, 0, 0); + result = tx->checkValid(avw, ltx, 0, 0, 0); } REQUIRE(result->isSuccess() == shouldBeValid); if (!shouldBeValid) @@ -1456,8 +1454,9 @@ TEST_CASE("transaction validation diagnostics", "[tx][soroban]") .createTx(); MutableTxResultPtr result; { + AppValidationWrapper const avw(test.getApp().getAppConnector(), false); LedgerTxn ltx(test.getApp().getLedgerTxnRoot()); - result = tx->checkValid(test.getApp().getAppConnector(), ltx, 0, 0, 0); + result = tx->checkValid(avw, ltx, 0, 0, 0); } REQUIRE(!test.isTxValid(tx)); diff --git a/src/transactions/test/SorobanTxTestUtils.cpp b/src/transactions/test/SorobanTxTestUtils.cpp index f16a7b9b4a..3cd225f261 100644 --- a/src/transactions/test/SorobanTxTestUtils.cpp +++ b/src/transactions/test/SorobanTxTestUtils.cpp @@ -820,8 +820,9 @@ SorobanTest::invokeArchivalOp(TransactionFrameBaseConstPtr tx, { MutableTxResultPtr result; { + AppValidationWrapper const avw(getApp().getAppConnector(), false); LedgerTxn ltx(getApp().getLedgerTxnRoot()); - result = tx->checkValid(getApp().getAppConnector(), ltx, 0, 0, 0); + result = tx->checkValid(avw, ltx, 0, 0, 0); } REQUIRE(result->isSuccess()); int64_t initBalance = getRoot().getBalance(); @@ -1100,8 +1101,9 @@ SorobanTest::createRestoreTx(SorobanResources const& resources, uint32_t fee, bool SorobanTest::isTxValid(TransactionFrameBaseConstPtr tx) { + AppValidationWrapper const avw(getApp().getAppConnector(), false); LedgerTxn ltx(getApp().getLedgerTxnRoot()); - auto ret = tx->checkValid(getApp().getAppConnector(), ltx, 0, 0, 0); + auto ret = tx->checkValid(avw, ltx, 0, 0, 0); return ret->isSuccess(); } @@ -1110,9 +1112,9 @@ SorobanTest::invokeTx(TransactionFrameBaseConstPtr tx, TransactionMetaFrame* txMeta) { { + AppValidationWrapper const avw(getApp().getAppConnector(), false); LedgerTxn ltx(getApp().getLedgerTxnRoot()); - REQUIRE(tx->checkValid(getApp().getAppConnector(), ltx, 0, 0, 0) - ->isSuccess()); + REQUIRE(tx->checkValid(avw, ltx, 0, 0, 0)->isSuccess()); } auto resultSet = closeLedger(*mApp, {tx}); diff --git a/src/transactions/test/TransactionTestFrame.cpp b/src/transactions/test/TransactionTestFrame.cpp index 7373b7eec4..300410f22f 100644 --- a/src/transactions/test/TransactionTestFrame.cpp +++ b/src/transactions/test/TransactionTestFrame.cpp @@ -95,20 +95,22 @@ TransactionTestFrame::checkValid(AppConnector& app, AbstractLedgerTxn& ltxOuter, uint64_t upperBoundCloseTimeOffset) const { LedgerTxn ltx(ltxOuter); + AppValidationWrapper const avw(app, false); auto ls = LedgerSnapshot(ltx); mTransactionTxResult = mTransactionFrame->checkValid( - app, ls, current, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); + avw, ls, current, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); return mTransactionTxResult; } MutableTxResultPtr -TransactionTestFrame::checkValid(AppConnector& app, LedgerSnapshot const& ls, +TransactionTestFrame::checkValid(ValidationConnector const& vc, + LedgerSnapshot const& ls, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const { mTransactionTxResult = mTransactionFrame->checkValid( - app, ls, current, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); + vc, ls, current, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); return mTransactionTxResult; } @@ -142,11 +144,11 @@ TransactionTestFrame::checkValidForTesting(AppConnector& app, bool TransactionTestFrame::checkSorobanResourceAndSetError( - AppConnector& app, SorobanNetworkConfig const& cfg, uint32_t ledgerVersion, + ValidationConnector const& vc, uint32_t ledgerVersion, MutableTxResultPtr txResult) const { auto ret = mTransactionFrame->checkSorobanResourceAndSetError( - app, cfg, ledgerVersion, txResult); + vc, ledgerVersion, txResult); mTransactionTxResult = txResult; return ret; } diff --git a/src/transactions/test/TransactionTestFrame.h b/src/transactions/test/TransactionTestFrame.h index 4f6f577057..7ee640eae9 100644 --- a/src/transactions/test/TransactionTestFrame.h +++ b/src/transactions/test/TransactionTestFrame.h @@ -67,12 +67,13 @@ class TransactionTestFrame : public TransactionFrameBase uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const; MutableTxResultPtr - checkValid(AppConnector& app, LedgerSnapshot const& ls, + checkValid(ValidationConnector const& vc, LedgerSnapshot const& ls, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const override; - bool checkSorobanResourceAndSetError( - AppConnector& app, SorobanNetworkConfig const& cfg, - uint32_t ledgerVersion, MutableTxResultPtr txResult) const override; + bool + checkSorobanResourceAndSetError(ValidationConnector const& vc, + uint32_t ledgerVersion, + MutableTxResultPtr txResult) const override; MutableTxResultPtr createSuccessResult() const override; diff --git a/src/transactions/test/TxEnvelopeTests.cpp b/src/transactions/test/TxEnvelopeTests.cpp index 5964e6cdcd..20c7e0515c 100644 --- a/src/transactions/test/TxEnvelopeTests.cpp +++ b/src/transactions/test/TxEnvelopeTests.cpp @@ -2074,8 +2074,8 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") VirtualClock::from_time_t(closeTime + 5)); } - auto offset = - getUpperBoundCloseTimeOffset(*app, closeTime); + auto offset = getUpperBoundCloseTimeOffset( + app->getAppConnector(), closeTime); auto upperBoundCloseTime = closeTime + offset; SECTION("success")