diff --git a/docs/stellar-core_example.cfg b/docs/stellar-core_example.cfg index 4e2669f50e..61d2851247 100644 --- a/docs/stellar-core_example.cfg +++ b/docs/stellar-core_example.cfg @@ -267,6 +267,11 @@ BACKGROUND_OVERLAY_PROCESSING = true # performance on multicore machines. Note that this is not compatible with SQLite. EXPERIMENTAL_PARALLEL_LEDGER_APPLY = false +# EXPERIMENTAL_BACKGROUND_TX_VALIDATION (bool) default false +# Validates transactions received over the network in the background, which can +# lead to better performance on multicore machines. +EXPERIMENTAL_BACKGROUND_TX_VALIDATION = false + # PREFERRED_PEERS (list of strings) default is empty # These are IP:port strings that this server will add to its DB of peers. # This server will try to always stay connected to the other peers on this list. diff --git a/src/bucket/test/BucketTestUtils.cpp b/src/bucket/test/BucketTestUtils.cpp index 057065655f..805ac4b804 100644 --- a/src/bucket/test/BucketTestUtils.cpp +++ b/src/bucket/test/BucketTestUtils.cpp @@ -299,7 +299,7 @@ LedgerManagerForBucketTests::sealLedgerTxnAndTransferEntriesToBucketList( } LedgerManagerForBucketTests& -BucketTestApplication::getLedgerManager() +BucketTestApplication::getLedgerManager() const { auto& lm = ApplicationImpl::getLedgerManager(); return static_cast(lm); diff --git a/src/bucket/test/BucketTestUtils.h b/src/bucket/test/BucketTestUtils.h index 99e84d877e..4602a26247 100644 --- a/src/bucket/test/BucketTestUtils.h +++ b/src/bucket/test/BucketTestUtils.h @@ -98,7 +98,7 @@ class BucketTestApplication : public TestApplication { } - virtual LedgerManagerForBucketTests& getLedgerManager() override; + virtual LedgerManagerForBucketTests& getLedgerManager() const override; private: virtual std::unique_ptr diff --git a/src/herder/Herder.h b/src/herder/Herder.h index d42430ad77..f361f271ac 100644 --- a/src/herder/Herder.h +++ b/src/herder/Herder.h @@ -130,10 +130,11 @@ class Herder // generator, and therefore can skip certain expensive validity checks virtual TransactionQueue::AddResult recvTransaction(TransactionFrameBasePtr tx, bool submittedFromSelf, - bool isLoadgenTx = false) = 0; + bool isPreValidated, bool isLoadgenTx = false) = 0; #else virtual TransactionQueue::AddResult - recvTransaction(TransactionFrameBasePtr tx, bool submittedFromSelf) = 0; + recvTransaction(TransactionFrameBasePtr tx, bool submittedFromSelf, + bool isPreValidated) = 0; #endif virtual void peerDoesntHave(stellar::MessageType type, uint256 const& itemID, Peer::pointer peer) = 0; diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp index f9e5a5b040..0e0a2ec9d8 100644 --- a/src/herder/HerderImpl.cpp +++ b/src/herder/HerderImpl.cpp @@ -590,7 +590,8 @@ HerderImpl::emitEnvelope(SCPEnvelope const& envelope) } TransactionQueue::AddResult -HerderImpl::recvTransaction(TransactionFrameBasePtr tx, bool submittedFromSelf +HerderImpl::recvTransaction(TransactionFrameBasePtr tx, bool submittedFromSelf, + bool isPreValidated #ifdef BUILD_TESTS , bool isLoadgenTx @@ -622,7 +623,7 @@ HerderImpl::recvTransaction(TransactionFrameBasePtr tx, bool submittedFromSelf } else if (!tx->isSoroban()) { - result = mTransactionQueue.tryAdd(tx, submittedFromSelf + result = mTransactionQueue.tryAdd(tx, submittedFromSelf, isPreValidated #ifdef BUILD_TESTS , isLoadgenTx @@ -631,7 +632,8 @@ HerderImpl::recvTransaction(TransactionFrameBasePtr tx, bool submittedFromSelf } else if (mSorobanTransactionQueue) { - result = mSorobanTransactionQueue->tryAdd(tx, submittedFromSelf + result = mSorobanTransactionQueue->tryAdd(tx, submittedFromSelf, + isPreValidated #ifdef BUILD_TESTS , isLoadgenTx @@ -1174,6 +1176,9 @@ HerderImpl::lastClosedLedgerIncreased(bool latest, TxSetXDRFrameConstPtr txSet) // applied transactions. updateTransactionQueue(txSet); + // Update snapshots used for transaction validation + mApp.getOverlayManager().updateSnapshots(); + // If we're in sync and there are no buffered ledgers to apply, trigger next // ledger if (latest) @@ -2356,7 +2361,8 @@ HerderImpl::updateTransactionQueue(TxSetXDRFrameConstPtr externalizedTxSet) auto invalidTxs = TxSetUtils::getInvalidTxList( txs, mApp, 0, - getUpperBoundCloseTimeOffset(mApp, lhhe.header.scpValue.closeTime)); + getUpperBoundCloseTimeOffset(mApp.getAppConnector(), + lhhe.header.scpValue.closeTime)); queue.ban(invalidTxs); queue.rebroadcast(); diff --git a/src/herder/HerderImpl.h b/src/herder/HerderImpl.h index 54a68400d0..cfe0bf70de 100644 --- a/src/herder/HerderImpl.h +++ b/src/herder/HerderImpl.h @@ -100,11 +100,11 @@ class HerderImpl : public Herder #ifdef BUILD_TESTS TransactionQueue::AddResult recvTransaction(TransactionFrameBasePtr tx, bool submittedFromSelf, - bool isLoadgenTx = false) override; + bool isPreValidated, bool isLoadgenTx = false) override; #else - TransactionQueue::AddResult - recvTransaction(TransactionFrameBasePtr tx, - bool submittedFromSelf) override; + TransactionQueue::AddResult recvTransaction(TransactionFrameBasePtr tx, + bool submittedFromSelf, + bool isPreValidated) override; #endif EnvelopeStatus recvSCPEnvelope(SCPEnvelope const& envelope) override; diff --git a/src/herder/TransactionQueue.cpp b/src/herder/TransactionQueue.cpp index ebd61fe32e..440c2ab08d 100644 --- a/src/herder/TransactionQueue.cpp +++ b/src/herder/TransactionQueue.cpp @@ -328,7 +328,8 @@ validateSorobanMemo(TransactionFrameBasePtr tx) TransactionQueue::AddResult TransactionQueue::canAdd( TransactionFrameBasePtr tx, AccountStates::iterator& stateIter, - std::vector>& txsToEvict + std::vector>& txsToEvict, + bool isPreValidated #ifdef BUILD_TESTS , bool isLoadgenTx @@ -355,6 +356,7 @@ TransactionQueue::canAdd( stateIter = mAccountStates.find(tx->getSourceID()); TransactionFrameBasePtr currentTx; + ExtendedLedgerSnapshot ls(mApp); if (stateIter != mAccountStates.end()) { auto const& transaction = stateIter->second.mTransaction; @@ -386,14 +388,7 @@ TransactionQueue::canAdd( if (tx->isSoroban()) { auto txResult = tx->createSuccessResult(); - if (!tx->checkSorobanResourceAndSetError( - mApp.getAppConnector(), - mApp.getLedgerManager() - .getLastClosedSorobanNetworkConfig(), - mApp.getLedgerManager() - .getLastClosedLedgerHeader() - .header.ledgerVersion, - txResult)) + if (!tx->checkSorobanResourceAndSetError(ls, txResult)) { return AddResult(AddResultCode::ADD_STATUS_ERROR, txResult); } @@ -433,7 +428,6 @@ 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 @@ -469,13 +463,11 @@ TransactionQueue::canAdd( // Loadgen txs were generated by this local node, and therefore can skip // validation, and be added directly to the queue. auto txResult = tx->createSuccessResult(); -#ifdef BUILD_TESTS - if (!isLoadgenTx) -#endif + if (!isPreValidated) { - txResult = - tx->checkValid(mApp.getAppConnector(), ls, 0, 0, - getUpperBoundCloseTimeOffset(mApp, closeTime)); + txResult = tx->checkValid( + ls, 0, 0, + getUpperBoundCloseTimeOffset(mApp.getAppConnector(), closeTime)); } if (!txResult->isSuccess()) { @@ -663,7 +655,8 @@ TransactionQueue::findAllAssetPairsInvolvedInPaymentLoops( } TransactionQueue::AddResult -TransactionQueue::tryAdd(TransactionFrameBasePtr tx, bool submittedFromSelf +TransactionQueue::tryAdd(TransactionFrameBasePtr tx, bool submittedFromSelf, + bool isPreValidated #ifdef BUILD_TESTS , bool isLoadgenTx @@ -689,7 +682,7 @@ TransactionQueue::tryAdd(TransactionFrameBasePtr tx, bool submittedFromSelf AccountStates::iterator stateIter; std::vector> txsToEvict; - auto const res = canAdd(tx, stateIter, txsToEvict + auto const res = canAdd(tx, stateIter, txsToEvict, isPreValidated #ifdef BUILD_TESTS , isLoadgenTx diff --git a/src/herder/TransactionQueue.h b/src/herder/TransactionQueue.h index 527cd915b6..33f374a2c5 100644 --- a/src/herder/TransactionQueue.h +++ b/src/herder/TransactionQueue.h @@ -125,11 +125,19 @@ class TransactionQueue static std::vector findAllAssetPairsInvolvedInPaymentLoops(TransactionFrameBasePtr tx); + /** + * Try to add `tx` to the queue. If the transaction is submitted from self + * (not over the network), then set `submittedFromSelf` to true. If the + * transaction has already been validated (via background transaction + * validation, or loadgen), then set `isPreValidated` to true. If the + * transaction is a loadgen transaction, set `isLoadgenTx` to true. + */ #ifdef BUILD_TESTS AddResult tryAdd(TransactionFrameBasePtr tx, bool submittedFromSelf, - bool isLoadgenTx = false); + bool isPreValidated, bool isLoadgenTx); #else - AddResult tryAdd(TransactionFrameBasePtr tx, bool submittedFromSelf); + AddResult tryAdd(TransactionFrameBasePtr tx, bool submittedFromSelf, + bool isPreValidated); #endif void removeApplied(Transactions const& txs); // Ban transactions that are no longer valid or have insufficient fee; @@ -239,11 +247,12 @@ class TransactionQueue TransactionQueue::AddResult canAdd(TransactionFrameBasePtr tx, AccountStates::iterator& stateIter, std::vector>& txsToEvict, - bool isLoadgenTx = false); + bool isPreValidated, bool isLoadgenTx = false); #else TransactionQueue::AddResult canAdd(TransactionFrameBasePtr tx, AccountStates::iterator& stateIter, - std::vector>& txsToEvict); + std::vector>& txsToEvict, + bool isPreValidated); #endif void releaseFeeMaybeEraseAccountState(TransactionFrameBasePtr tx); diff --git a/src/herder/TxSetFrame.cpp b/src/herder/TxSetFrame.cpp index 3fe1473cf5..455091d23f 100644 --- a/src/herder/TxSetFrame.cpp +++ b/src/herder/TxSetFrame.cpp @@ -1814,13 +1814,12 @@ TxSetPhaseFrame::txsAreValid(Application& app, // Grab read-only latest ledger state; This is only used to validate tx sets // for LCL+1 - LedgerSnapshot ls(app); + ExtendedLedgerSnapshot ls(app); ls.getLedgerHeader().currentToModify().ledgerSeq = app.getLedgerManager().getLastClosedLedgerNum() + 1; for (auto const& tx : *this) { - auto txResult = tx->checkValid(app.getAppConnector(), ls, 0, - lowerBoundCloseTimeOffset, + auto txResult = tx->checkValid(ls, 0, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); if (!txResult->isSuccess()) { diff --git a/src/herder/TxSetUtils.cpp b/src/herder/TxSetUtils.cpp index f81456b584..d0f6d621c3 100644 --- a/src/herder/TxSetUtils.cpp +++ b/src/herder/TxSetUtils.cpp @@ -168,7 +168,7 @@ TxSetUtils::getInvalidTxList(TxFrameList const& txs, Application& app, { ZoneScoped; releaseAssert(threadIsMain()); - LedgerSnapshot ls(app); + ExtendedLedgerSnapshot ls(app); // This is done so minSeqLedgerGap is validated against the next // ledgerSeq, which is what will be used at apply time ls.getLedgerHeader().currentToModify().ledgerSeq = @@ -178,8 +178,7 @@ TxSetUtils::getInvalidTxList(TxFrameList const& txs, Application& app, for (auto const& tx : txs) { - auto txResult = tx->checkValid(app.getAppConnector(), ls, 0, - lowerBoundCloseTimeOffset, + auto txResult = tx->checkValid(ls, 0, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); if (!txResult->isSuccess()) { diff --git a/src/herder/test/HerderTests.cpp b/src/herder/test/HerderTests.cpp index 9a94fdeee4..bc1d31431a 100644 --- a/src/herder/test/HerderTests.cpp +++ b/src/herder/test/HerderTests.cpp @@ -92,8 +92,9 @@ TEST_CASE_VERSIONS("standalone", "[herder][acceptance]") auto feedTx = [&](TransactionTestFramePtr tx, TransactionQueue::AddResultCode expectedRes) { - REQUIRE(app->getHerder().recvTransaction(tx, false).code == - expectedRes); + REQUIRE( + app->getHerder().recvTransaction(tx, false, false).code == + expectedRes); }; auto waitForExternalize = [&]() { @@ -2934,11 +2935,11 @@ TEST_CASE("tx queue source account limit", "[herder][transactionqueue]") auto [root, a1, b1, tx1, tx2] = makeTxs(app); // Submit txs for the same account, should be good - REQUIRE(app->getHerder().recvTransaction(tx1, true).code == + REQUIRE(app->getHerder().recvTransaction(tx1, true, false).code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); // Second tx is rejected due to limit - REQUIRE(app->getHerder().recvTransaction(tx2, true).code == + REQUIRE(app->getHerder().recvTransaction(tx2, true, false).code == TransactionQueue::AddResultCode::ADD_STATUS_TRY_AGAIN_LATER); uint32_t lcl = app->getLedgerManager().getLastClosedLedgerNum(); @@ -2965,7 +2966,7 @@ TEST_CASE("tx queue source account limit", "[herder][transactionqueue]") // Now submit the second tx (which was rejected earlier) and make sure // it ends up in the ledger - REQUIRE(app->getHerder().recvTransaction(tx2, true).code == + REQUIRE(app->getHerder().recvTransaction(tx2, true, false).code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); lcl = app->getLedgerManager().getLastClosedLedgerNum(); @@ -3234,6 +3235,18 @@ TEST_CASE("overlay parallel processing") }); } + SECTION("background transaction validation") + { + // Set threshold to 1 so all have to vote + simulation = + Topologies::core(4, 1, Simulation::OVER_TCP, networkID, [](int i) { + auto cfg = getTestConfig(i); + cfg.TESTING_UPGRADE_MAX_TX_SET_SIZE = 100; + cfg.BACKGROUND_TX_VALIDATION = true; + return cfg; + }); + } + // Background ledger close requires postgres #ifdef USE_POSTGRES SECTION("background ledger close") @@ -3645,7 +3658,7 @@ herderExternalizesValuesWithProtocol(uint32_t version, auto sorobanTx = createUploadWasmTx( *A, *root, 100, DEFAULT_TEST_RESOURCE_FEE, resources); REQUIRE( - herderA.recvTransaction(sorobanTx, true).code == + herderA.recvTransaction(sorobanTx, true, false).code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); submitted = true; } @@ -4332,9 +4345,9 @@ TEST_CASE("do not flood invalid transactions", "[herder]") // this will be invalid after tx1r gets applied auto tx2r = root->tx({payment(*root, 1)}); - herder.recvTransaction(tx1a, false); - herder.recvTransaction(tx1r, false); - herder.recvTransaction(tx2r, false); + herder.recvTransaction(tx1a, false, false); + herder.recvTransaction(tx1r, false, false); + herder.recvTransaction(tx2r, false, false); size_t numBroadcast = 0; tq.mTxBroadcastedEvent = [&](TransactionFrameBasePtr&) { ++numBroadcast; }; @@ -4447,7 +4460,7 @@ TEST_CASE("do not flood too many soroban transactions", auto tx = createUploadWasmTx(*app, source, inclusionFee, 10'000'000, resources); - REQUIRE(herder.recvTransaction(tx, false).code == + REQUIRE(herder.recvTransaction(tx, false, false).code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); return tx; }; @@ -4622,7 +4635,7 @@ TEST_CASE("do not flood too many transactions", "[herder][transactionqueue]") curFeeOffset--; } - REQUIRE(herder.recvTransaction(tx, false).code == + REQUIRE(herder.recvTransaction(tx, false, false).code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); return tx; }; @@ -4832,7 +4845,7 @@ TEST_CASE("do not flood too many transactions with DEX separation", curFeeOffset--; } - REQUIRE(herder.recvTransaction(tx, false).code == + REQUIRE(herder.recvTransaction(tx, false, false).code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); return tx; }; @@ -5260,7 +5273,7 @@ TEST_CASE("exclude transactions by operation type", "[herder]") auto acc = getAccount("acc"); auto tx = root->tx({createAccount(acc.getPublicKey(), 1)}); - REQUIRE(app->getHerder().recvTransaction(tx, false).code == + REQUIRE(app->getHerder().recvTransaction(tx, false, false).code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); } @@ -5275,7 +5288,7 @@ TEST_CASE("exclude transactions by operation type", "[herder]") auto acc = getAccount("acc"); auto tx = root->tx({createAccount(acc.getPublicKey(), 1)}); - REQUIRE(app->getHerder().recvTransaction(tx, false).code == + REQUIRE(app->getHerder().recvTransaction(tx, false, false).code == TransactionQueue::AddResultCode::ADD_STATUS_FILTERED); } @@ -5291,7 +5304,7 @@ TEST_CASE("exclude transactions by operation type", "[herder]") auto acc = getAccount("acc"); auto tx = root->tx({createAccount(acc.getPublicKey(), 1)}); - REQUIRE(app->getHerder().recvTransaction(tx, false).code == + REQUIRE(app->getHerder().recvTransaction(tx, false, false).code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); } } diff --git a/src/herder/test/TransactionQueueTests.cpp b/src/herder/test/TransactionQueueTests.cpp index 6cd22771db..47ef199478 100644 --- a/src/herder/test/TransactionQueueTests.cpp +++ b/src/herder/test/TransactionQueueTests.cpp @@ -102,7 +102,7 @@ class TransactionQueueTest add(TransactionFrameBasePtr const& tx, TransactionQueue::AddResultCode expected) { - auto res = mTransactionQueue.tryAdd(tx, false); + auto res = mTransactionQueue.tryAdd(tx, false, false, false); REQUIRE(res.code == expected); return res; } @@ -1010,7 +1010,7 @@ TEST_CASE("TransactionQueue with PreconditionsV2", "[herder][transactionqueue]") auto& herder = static_cast(app->getHerder()); auto& tq = herder.getTransactionQueue(); - REQUIRE(herder.recvTransaction(tx, false).code == + REQUIRE(herder.recvTransaction(tx, false, false).code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); REQUIRE(tq.getTransactions({}).size() == 1); @@ -1138,7 +1138,7 @@ TEST_CASE("Soroban TransactionQueue pre-protocol-20", DEFAULT_TEST_RESOURCE_FEE, resources); // Soroban tx is not supported - REQUIRE(app->getHerder().recvTransaction(tx, false).code == + REQUIRE(app->getHerder().recvTransaction(tx, false, false).code == TransactionQueue::AddResultCode::ADD_STATUS_ERROR); REQUIRE(app->getHerder().getTx(tx->getFullHash()) == nullptr); } @@ -1185,8 +1185,9 @@ TEST_CASE("Soroban tx and memos", "[soroban][transactionqueue]") app->getNetworkID(), a1, {uploadOp}, {}, resources, uploadResourceFee + 100, uploadResourceFee, "memo"); - REQUIRE(app->getHerder().recvTransaction(txWithMemo, false).code == - TransactionQueue::AddResultCode::ADD_STATUS_PENDING); + REQUIRE( + app->getHerder().recvTransaction(txWithMemo, false, false).code == + TransactionQueue::AddResultCode::ADD_STATUS_PENDING); } SECTION("non-source auth tx with memo") @@ -1202,8 +1203,9 @@ TEST_CASE("Soroban tx and memos", "[soroban][transactionqueue]") app->getNetworkID(), a1, {uploadOp}, {}, resources, uploadResourceFee + 100, uploadResourceFee, "memo"); - REQUIRE(app->getHerder().recvTransaction(txWithMemo, false).code == - TransactionQueue::AddResultCode::ADD_STATUS_ERROR); + REQUIRE( + app->getHerder().recvTransaction(txWithMemo, false, false).code == + TransactionQueue::AddResultCode::ADD_STATUS_ERROR); } SECTION("non-source auth tx with muxed tx source") @@ -1215,9 +1217,9 @@ TEST_CASE("Soroban tx and memos", "[soroban][transactionqueue]") uploadResourceFee + 100, uploadResourceFee, std::nullopt, 1 /*muxedData*/); - REQUIRE( - app->getHerder().recvTransaction(txWithMuxedTxSource, false).code == - TransactionQueue::AddResultCode::ADD_STATUS_ERROR); + REQUIRE(app->getHerder() + .recvTransaction(txWithMuxedTxSource, false, false) + .code == TransactionQueue::AddResultCode::ADD_STATUS_ERROR); } SECTION("non-source auth tx with muxed op source") @@ -1232,9 +1234,9 @@ TEST_CASE("Soroban tx and memos", "[soroban][transactionqueue]") app->getNetworkID(), a1, {uploadOp}, {}, resources, uploadResourceFee + 100, uploadResourceFee); - REQUIRE( - app->getHerder().recvTransaction(txWithMuxedOpSource, false).code == - TransactionQueue::AddResultCode::ADD_STATUS_ERROR); + REQUIRE(app->getHerder() + .recvTransaction(txWithMuxedOpSource, false, false) + .code == TransactionQueue::AddResultCode::ADD_STATUS_ERROR); } } @@ -1275,7 +1277,7 @@ TEST_CASE("Soroban TransactionQueue limits", auto tx = createUploadWasmTx(*app, *root, initialInclusionFee, resourceFee, resAdjusted); - REQUIRE(app->getHerder().recvTransaction(tx, false).code == + REQUIRE(app->getHerder().recvTransaction(tx, false, false).code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); REQUIRE(app->getHerder().getTx(tx->getFullHash()) != nullptr); @@ -1335,7 +1337,7 @@ TEST_CASE("Soroban TransactionQueue limits", REQUIRE_THROWS_AS(badTx->getInclusionFee(), std::runtime_error); } // Gracefully handle malformed transaction - auto addResult = app->getHerder().recvTransaction(badTx, false); + auto addResult = app->getHerder().recvTransaction(badTx, false, false); REQUIRE(addResult.code == TransactionQueue::AddResultCode::ADD_STATUS_ERROR); REQUIRE(addResult.txResult); @@ -1350,12 +1352,16 @@ TEST_CASE("Soroban TransactionQueue limits", auto checkLimitEnforced = [&](auto const& pendingTx, auto const& rejectedTx, TransactionQueue& txQueue) { - REQUIRE(app->getHerder().recvTransaction(pendingTx, false).code == + REQUIRE(app->getHerder() + .recvTransaction(pendingTx, false, false) + .code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); // Can't submit tx due to source account limit REQUIRE( - app->getHerder().recvTransaction(rejectedTx, false).code == + app->getHerder() + .recvTransaction(rejectedTx, false, false) + .code == TransactionQueue::AddResultCode::ADD_STATUS_TRY_AGAIN_LATER); // ban existing tx @@ -1365,7 +1371,9 @@ TEST_CASE("Soroban TransactionQueue limits", REQUIRE(app->getHerder().isBannedTx(pendingTx->getFullHash())); // Now can submit classic txs - REQUIRE(app->getHerder().recvTransaction(rejectedTx, false).code == + REQUIRE(app->getHerder() + .recvTransaction(rejectedTx, false, false) + .code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); }; SECTION("classic is rejected when soroban is pending") @@ -1421,8 +1429,9 @@ TEST_CASE("Soroban TransactionQueue limits", auto txNew = createUploadWasmTx(*app, account1, initialInclusionFee, resourceFee, resources); - REQUIRE(app->getHerder().recvTransaction(txNew, false).code == - TransactionQueue::AddResultCode::ADD_STATUS_PENDING); + REQUIRE( + app->getHerder().recvTransaction(txNew, false, false).code == + TransactionQueue::AddResultCode::ADD_STATUS_PENDING); SECTION("insufficient fee") { @@ -1445,7 +1454,8 @@ TEST_CASE("Soroban TransactionQueue limits", } // Same per-op fee, no eviction - auto addResult = app->getHerder().recvTransaction(tx2, false); + auto addResult = + app->getHerder().recvTransaction(tx2, false, false); REQUIRE(addResult.code == TransactionQueue::AddResultCode::ADD_STATUS_ERROR); REQUIRE(!app->getHerder().isBannedTx(tx->getFullHash())); @@ -1463,7 +1473,8 @@ TEST_CASE("Soroban TransactionQueue limits", createUploadWasmTx(*app, account2, initialInclusionFee, minBalance2, resources); - auto addResult = app->getHerder().recvTransaction(tx2, false); + auto addResult = + app->getHerder().recvTransaction(tx2, false, false); REQUIRE(addResult.code == TransactionQueue::AddResultCode::ADD_STATUS_ERROR); REQUIRE(!app->getHerder().isBannedTx(tx->getFullHash())); @@ -1492,7 +1503,8 @@ TEST_CASE("Soroban TransactionQueue limits", resourceFee, resources); } - auto addResult = app->getHerder().recvTransaction(tx2, false); + auto addResult = + app->getHerder().recvTransaction(tx2, false, false); REQUIRE(addResult.code == TransactionQueue::AddResultCode::ADD_STATUS_ERROR); REQUIRE(!app->getHerder().isBannedTx(tx->getFullHash())); @@ -1506,7 +1518,8 @@ TEST_CASE("Soroban TransactionQueue limits", *app, account2, initialInclusionFee * 2, resourceFee, resources, std::nullopt, /* addInvalidOps */ 1); - auto addResult = app->getHerder().recvTransaction(tx2, false); + auto addResult = + app->getHerder().recvTransaction(tx2, false, false); REQUIRE(addResult.code == TransactionQueue::AddResultCode::ADD_STATUS_ERROR); REQUIRE(!app->getHerder().isBannedTx(tx->getFullHash())); @@ -1541,10 +1554,10 @@ TEST_CASE("Soroban TransactionQueue limits", 2 * (initialInclusionFee + 1)); } - auto res = app->getHerder().recvTransaction(tx2, false); + auto res = app->getHerder().recvTransaction(tx2, false, false); REQUIRE(res.code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); - auto res2 = app->getHerder().recvTransaction(tx3, false); + auto res2 = app->getHerder().recvTransaction(tx3, false, false); REQUIRE(res2.code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); @@ -2131,8 +2144,9 @@ TEST_CASE("transaction queue starting sequence boundary", REQUIRE(acc1.loadSequenceNumber() == startingSeq - 1); ClassicTransactionQueue tq(*app, 4, 10, 4); - REQUIRE(tq.tryAdd(transaction(*app, acc1, 1, 1, 100), false).code == - TransactionQueue::AddResultCode::ADD_STATUS_PENDING); + REQUIRE( + tq.tryAdd(transaction(*app, acc1, 1, 1, 100), false, false, false) + .code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); auto checkTxSet = [&](uint32_t ledgerSeq) { auto lcl = app->getLedgerManager().getLastClosedLedgerHeader(); @@ -2720,9 +2734,9 @@ TEST_CASE("remove applied", "[herder][transactionqueue]") auto tx3 = acc2.tx({payment(*root, 1)}); auto tx4 = acc3.tx({payment(*root, 1)}); - herder.recvTransaction(tx1a, false); - herder.recvTransaction(tx2, false); - herder.recvTransaction(tx3, false); + herder.recvTransaction(tx1a, false, false); + herder.recvTransaction(tx2, false, false); + herder.recvTransaction(tx3, false, false); { auto const& lcl = lm.getLastClosedLedgerHeader(); @@ -2742,7 +2756,7 @@ TEST_CASE("remove applied", "[herder][transactionqueue]") } REQUIRE(tq.getTransactions({}).size() == 1); - REQUIRE(herder.recvTransaction(tx4, false).code == + REQUIRE(herder.recvTransaction(tx4, false, false).code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); REQUIRE(tq.getTransactions({}).size() == 2); } diff --git a/src/herder/test/TxSetTests.cpp b/src/herder/test/TxSetTests.cpp index bef8c4d51c..85f60735af 100644 --- a/src/herder/test/TxSetTests.cpp +++ b/src/herder/test/TxSetTests.cpp @@ -991,9 +991,8 @@ TEST_CASE("applicable txset validation - transactions belong to correct phase", 1)}, 2000); } - LedgerSnapshot ls(*app); - REQUIRE(tx->checkValid(app->getAppConnector(), ls, 0, 0, 0) - ->isSuccess()); + ExtendedLedgerSnapshot ls(*app); + REQUIRE(tx->checkValid(ls, 0, 0, 0)->isSuccess()); return tx; }; @@ -1130,9 +1129,8 @@ TEST_CASE("applicable txset validation - Soroban resources", "[txset][soroban]") auto tx = sorobanTransactionFrameFromOps( app->getNetworkID(), source, {op}, {}, resources, 2000, 100'000'000); - LedgerSnapshot ls(*app); - REQUIRE(tx->checkValid(app->getAppConnector(), ls, 0, 0, 0) - ->isSuccess()); + ExtendedLedgerSnapshot ls(*app); + REQUIRE(tx->checkValid(ls, 0, 0, 0)->isSuccess()); return tx; }; diff --git a/src/ledger/LedgerManager.h b/src/ledger/LedgerManager.h index 5094196de8..dc7f5da975 100644 --- a/src/ledger/LedgerManager.h +++ b/src/ledger/LedgerManager.h @@ -252,6 +252,7 @@ class LedgerManager virtual SorobanNetworkConfig const& getSorobanNetworkConfigForApply() = 0; virtual bool hasLastClosedSorobanNetworkConfig() const = 0; + virtual bool hasSorobanNetworkConfigForApply() const = 0; #ifdef BUILD_TESTS virtual SorobanNetworkConfig& getMutableSorobanNetworkConfigForApply() = 0; diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp index 45ebd30f70..043ac0ac69 100644 --- a/src/ledger/LedgerManagerImpl.cpp +++ b/src/ledger/LedgerManagerImpl.cpp @@ -555,7 +555,7 @@ LedgerManagerImpl::getLastClosedSorobanNetworkConfig() SorobanNetworkConfig const& LedgerManagerImpl::getSorobanNetworkConfigForApply() { - releaseAssert(mApplyState.mSorobanNetworkConfig); + releaseAssert(hasSorobanNetworkConfigForApply()); return *mApplyState.mSorobanNetworkConfig; } @@ -566,6 +566,12 @@ LedgerManagerImpl::hasLastClosedSorobanNetworkConfig() const return static_cast(getLCLState().sorobanConfig); } +bool +LedgerManagerImpl::hasSorobanNetworkConfigForApply() const +{ + return static_cast(mApplyState.mSorobanNetworkConfig); +} + #ifdef BUILD_TESTS SorobanNetworkConfig& LedgerManagerImpl::getMutableSorobanNetworkConfigForApply() diff --git a/src/ledger/LedgerManagerImpl.h b/src/ledger/LedgerManagerImpl.h index 61abb7152c..85c1f63add 100644 --- a/src/ledger/LedgerManagerImpl.h +++ b/src/ledger/LedgerManagerImpl.h @@ -210,6 +210,7 @@ class LedgerManagerImpl : public LedgerManager SorobanNetworkConfig const& getSorobanNetworkConfigForApply() override; bool hasLastClosedSorobanNetworkConfig() const override; + bool hasSorobanNetworkConfigForApply() const override; #ifdef BUILD_TESTS SorobanNetworkConfig& getMutableSorobanNetworkConfigForApply() override; diff --git a/src/ledger/LedgerStateSnapshot.cpp b/src/ledger/LedgerStateSnapshot.cpp index badce420fc..1a481e736d 100644 --- a/src/ledger/LedgerStateSnapshot.cpp +++ b/src/ledger/LedgerStateSnapshot.cpp @@ -156,10 +156,11 @@ LedgerTxnReadOnly::load(LedgerKey const& key) const void LedgerTxnReadOnly::executeWithMaybeInnerSnapshot( - std::function f) const + std::function f, + ExtendedLedgerSnapshot const& outer) const { LedgerTxn inner(mLedgerTxn); - LedgerSnapshot lsg(inner); + ExtendedLedgerSnapshot lsg(inner, outer); return f(lsg); } @@ -209,7 +210,8 @@ BucketSnapshotState::load(LedgerKey const& key) const void BucketSnapshotState::executeWithMaybeInnerSnapshot( - std::function f) const + std::function f, + ExtendedLedgerSnapshot const& outer) const { throw std::runtime_error( "BucketSnapshotState::executeWithMaybeInnerSnapshot is illegal: " @@ -221,7 +223,7 @@ LedgerSnapshot::LedgerSnapshot(AbstractLedgerTxn& ltx) { } -LedgerSnapshot::LedgerSnapshot(Application& app) +LedgerSnapshot::LedgerSnapshot(Application const& app) { releaseAssert(threadIsMain()); #ifdef BUILD_TESTS @@ -257,10 +259,66 @@ LedgerSnapshot::load(LedgerKey const& key) const return mGetter->load(key); } +ExtendedLedgerSnapshot::ExtendedLedgerSnapshot(Application const& app) + : LedgerSnapshot(app) + , mConfig(app.getAppConnector().getConfigPtr()) + , mSorobanNetworkConfig( + app.getAppConnector().maybeGetLastClosedSorobanNetworkConfig()) + , mCurrentProtocolVersion(app.getLedgerManager() + .getLastClosedLedgerHeader() + .header.ledgerVersion) +{ + releaseAssert(threadIsMain()); +} + +ExtendedLedgerSnapshot::ExtendedLedgerSnapshot(AbstractLedgerTxn& ltx, + AppConnector const& app, + bool forApply) + : LedgerSnapshot(ltx) + , mConfig(app.getConfigPtr()) + , mSorobanNetworkConfig(forApply + ? app.maybeGetSorobanNetworkConfigForApply() + : app.maybeGetLastClosedSorobanNetworkConfig()) + , mCurrentProtocolVersion(ltx.loadHeader().current().ledgerVersion) +{ + releaseAssert( + threadIsMain() || + (forApply && app.threadIsType(Application::ThreadType::APPLY))); +} + +ExtendedLedgerSnapshot::ExtendedLedgerSnapshot( + AbstractLedgerTxn& ltx, ExtendedLedgerSnapshot const& outer) + : LedgerSnapshot(ltx) + , mConfig(outer.mConfig) + , mSorobanNetworkConfig(outer.mSorobanNetworkConfig) + , mCurrentProtocolVersion(outer.mCurrentProtocolVersion) +{ +} + void -LedgerSnapshot::executeWithMaybeInnerSnapshot( - std::function f) const +ExtendedLedgerSnapshot::executeWithMaybeInnerSnapshot( + std::function f) const { - return mGetter->executeWithMaybeInnerSnapshot(f); + return mGetter->executeWithMaybeInnerSnapshot(f, *this); } + +Config const& +ExtendedLedgerSnapshot::getConfig() const +{ + releaseAssert(mConfig); + return *mConfig; +} + +SorobanNetworkConfig const& +ExtendedLedgerSnapshot::getSorobanNetworkConfig() const +{ + releaseAssertOrThrow(mSorobanNetworkConfig.has_value()); + return mSorobanNetworkConfig.value(); +} + +uint32_t +ExtendedLedgerSnapshot::getCurrentProtocolVersion() const +{ + return mCurrentProtocolVersion; } +} \ No newline at end of file diff --git a/src/ledger/LedgerStateSnapshot.h b/src/ledger/LedgerStateSnapshot.h index a4e0653a1c..7e76e00df8 100644 --- a/src/ledger/LedgerStateSnapshot.h +++ b/src/ledger/LedgerStateSnapshot.h @@ -7,6 +7,7 @@ #include "bucket/BucketSnapshotManager.h" #include "bucket/SearchableBucketList.h" #include "ledger/LedgerTxn.h" +#include "ledger/NetworkConfig.h" #include "util/NonCopyable.h" #include @@ -77,7 +78,8 @@ class AbstractLedgerStateSnapshot // to support the replay of old buggy protocols (<8), see // `TransactionFrame::loadSourceAccount` virtual void executeWithMaybeInnerSnapshot( - std::function f) const = 0; + std::function f, + ExtendedLedgerSnapshot const& outer) const = 0; }; // A concrete implementation of read-only SQL snapshot wrapper @@ -100,7 +102,8 @@ class LedgerTxnReadOnly : public AbstractLedgerStateSnapshot AccountID const& AccountID) const override; LedgerEntryWrapper load(LedgerKey const& key) const override; void executeWithMaybeInnerSnapshot( - std::function f) const override; + std::function f, + ExtendedLedgerSnapshot const& outer) const override; }; // A concrete implementation of read-only BucketList snapshot wrapper @@ -125,7 +128,8 @@ class BucketSnapshotState : public AbstractLedgerStateSnapshot AccountID const& AccountID) const override; LedgerEntryWrapper load(LedgerKey const& key) const override; void executeWithMaybeInnerSnapshot( - std::function f) const override; + std::function f, + ExtendedLedgerSnapshot const& outer) const override; }; // A helper class to create and query read-only snapshots @@ -137,12 +141,15 @@ class BucketSnapshotState : public AbstractLedgerStateSnapshot // ledger state. class LedgerSnapshot : public NonMovableOrCopyable { - std::unique_ptr mGetter; std::unique_ptr mLegacyLedgerTxn; + protected: + std::unique_ptr mGetter; + public: LedgerSnapshot(AbstractLedgerTxn& ltx); - LedgerSnapshot(Application& app); + LedgerSnapshot(Application const& app); + virtual ~LedgerSnapshot() = default; LedgerHeaderWrapper getLedgerHeader() const; LedgerEntryWrapper getAccount(AccountID const& account) const; LedgerEntryWrapper @@ -158,12 +165,43 @@ class LedgerSnapshot : public NonMovableOrCopyable return mGetter->getAccount(header, tx, AccountID); } LedgerEntryWrapper load(LedgerKey const& key) const; +}; + +// A subclass of LedgerSnapshot that contains additional snapshots necessary for +// transaction validation. +class ExtendedLedgerSnapshot : public LedgerSnapshot +{ + public: + // Generate a snapshot from the most recently closed ledger. The resulting + // `ExtendedLedgerSnapshot` cannot be used for transaction application. + explicit ExtendedLedgerSnapshot(Application const& app); + + // Generate a snapshot from a specific AbstractLedgerTxn. Set `forApply` to + // `true` to use this snapshot in transaction application. + ExtendedLedgerSnapshot(AbstractLedgerTxn& ltx, AppConnector const& app, + bool forApply); + + // Generate a snapshot from a specific `AbstractLedgerTxn`, copying + // additional snapshot data from `outer`. This is intended only for use with + // `executeWithMaybeInnerSnapshot`. + ExtendedLedgerSnapshot(AbstractLedgerTxn& ltx, + ExtendedLedgerSnapshot const& outer); + virtual ~ExtendedLedgerSnapshot() = default; // Execute a function with a nested snapshot, if supported. This is needed // to support the replay of old buggy protocols (<8), see // `TransactionFrame::loadSourceAccount` void executeWithMaybeInnerSnapshot( - std::function f) const; -}; + std::function f) const; + // Getters for the additional snapshots + Config const& getConfig() const; + SorobanNetworkConfig const& getSorobanNetworkConfig() const; + uint32_t getCurrentProtocolVersion() const; + + private: + std::shared_ptr const mConfig; + std::optional const mSorobanNetworkConfig; + uint32_t const mCurrentProtocolVersion; +}; } diff --git a/src/main/AppConnector.cpp b/src/main/AppConnector.cpp index b18f0506bd..bb076e9f71 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(); @@ -56,9 +56,34 @@ AppConnector::getLastClosedSorobanNetworkConfig() const SorobanNetworkConfig const& AppConnector::getSorobanNetworkConfigForApply() const { + releaseAssert(threadIsMain() || + mApp.threadIsType(Application::ThreadType::APPLY)); return mApp.getLedgerManager().getSorobanNetworkConfigForApply(); } +std::optional +AppConnector::maybeGetLastClosedSorobanNetworkConfig() const +{ + releaseAssert(threadIsMain()); + if (mApp.getLedgerManager().hasLastClosedSorobanNetworkConfig()) + { + return mApp.getLedgerManager().getLastClosedSorobanNetworkConfig(); + } + return std::nullopt; +} + +std::optional +AppConnector::maybeGetSorobanNetworkConfigForApply() const +{ + releaseAssert(threadIsMain() || + mApp.threadIsType(Application::ThreadType::APPLY)); + if (mApp.getLedgerManager().hasSorobanNetworkConfigForApply()) + { + return mApp.getLedgerManager().getSorobanNetworkConfigForApply(); + } + return std::nullopt; +} + medida::MetricsRegistry& AppConnector::getMetrics() const { @@ -101,8 +126,22 @@ AppConnector::postOnOverlayThread(std::function&& f, mApp.postOnOverlayThread(std::move(f), message); } +void +AppConnector::postOnTxValidationThread(std::function&& f, + std::string const& message) +{ + mApp.postOnTxValidationThread(std::move(f), message); +} + Config const& AppConnector::getConfig() const +{ + releaseAssert(mConfig); + return *mConfig; +} + +std::shared_ptr +AppConnector::getConfigPtr() const { return mConfig; } @@ -119,6 +158,12 @@ AppConnector::now() const return mApp.getClock().now(); } +VirtualClock::system_time_point +AppConnector::system_now() const +{ + return mApp.getClock().system_now(); +} + bool AppConnector::shouldYield() const { diff --git a/src/main/AppConnector.h b/src/main/AppConnector.h index 9d78ab9e66..9c8296db65 100644 --- a/src/main/AppConnector.h +++ b/src/main/AppConnector.h @@ -25,16 +25,16 @@ 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); + explicit 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; SorobanMetrics& getSorobanMetrics() const; void checkOnOperationApply(Operation const& operation, @@ -48,8 +48,12 @@ class AppConnector Scheduler::ActionType type = Scheduler::ActionType::NORMAL_ACTION); void postOnOverlayThread(std::function&& f, std::string const& message); + void postOnTxValidationThread(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 @@ -57,6 +61,18 @@ class AppConnector checkScheduledAndCache(std::shared_ptr msgTracker); SorobanNetworkConfig const& getLastClosedSorobanNetworkConfig() const; SorobanNetworkConfig const& getSorobanNetworkConfigForApply() const; + + // These `maybeGet*` methods have two main differences from their `get*` + // counterparts: + // 1. They return `nullopt` instead of throwing an assertion error if + // the value is not set. + // 2. They make copies instead of returning references. This allows other + // threads to maintain their own snapshots of Soroban configs. + std::optional + maybeGetLastClosedSorobanNetworkConfig() const; + std::optional + maybeGetSorobanNetworkConfigForApply() const; + bool threadIsType(Application::ThreadType type) const; medida::MetricsRegistry& getMetrics() const; diff --git a/src/main/Application.h b/src/main/Application.h index ac71fa6502..dfff105be7 100644 --- a/src/main/Application.h +++ b/src/main/Application.h @@ -172,7 +172,8 @@ class Application WORKER, EVICTION, OVERLAY, - APPLY + APPLY, + TX_VALIDATION }; virtual ~Application(){}; @@ -189,7 +190,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 @@ -216,8 +217,8 @@ class Application // Get references to each of the "subsystem" objects. virtual TmpDirManager& getTmpDirManager() = 0; - virtual LedgerManager& getLedgerManager() = 0; - virtual BucketManager& getBucketManager() = 0; + virtual LedgerManager& getLedgerManager() const = 0; + virtual BucketManager& getBucketManager() const = 0; virtual LedgerApplyManager& getLedgerApplyManager() = 0; virtual HistoryArchiveManager& getHistoryArchiveManager() = 0; virtual HistoryManager& getHistoryManager() = 0; @@ -254,6 +255,8 @@ class Application std::string jobName) = 0; virtual void postOnOverlayThread(std::function&& f, std::string jobName) = 0; + virtual void postOnTxValidationThread(std::function&& f, + std::string jobName) = 0; virtual void postOnLedgerCloseThread(std::function&& f, std::string jobName) = 0; @@ -322,7 +325,7 @@ class Application // instances virtual Hash const& getNetworkID() const = 0; - virtual AbstractLedgerTxnParent& getLedgerTxnRoot() = 0; + virtual AbstractLedgerTxnParent& getLedgerTxnRoot() const = 0; virtual void validateAndLogConfig() = 0; @@ -346,7 +349,7 @@ class Application // Returns true iff the calling thread has the same type as `type` virtual bool threadIsType(ThreadType type) const = 0; - virtual AppConnector& getAppConnector() = 0; + virtual AppConnector& getAppConnector() const = 0; protected: Application() diff --git a/src/main/ApplicationImpl.cpp b/src/main/ApplicationImpl.cpp index d99c1d9b3c..20acf06fc3 100644 --- a/src/main/ApplicationImpl.cpp +++ b/src/main/ApplicationImpl.cpp @@ -93,6 +93,13 @@ ApplicationImpl::ApplicationImpl(VirtualClock& clock, Config const& cfg) , mOverlayWork(mOverlayIOContext ? std::make_unique( *mOverlayIOContext) : nullptr) + , mTxValidationIOContext(mConfig.BACKGROUND_TX_VALIDATION + ? std::make_unique(1) + : nullptr) + , mTxValidationWork(mTxValidationIOContext + ? std::make_unique( + *mTxValidationIOContext) + : nullptr) , mLedgerCloseIOContext(mConfig.parallelLedgerClose() ? std::make_unique(1) : nullptr) @@ -115,6 +122,8 @@ ApplicationImpl::ApplicationImpl(VirtualClock& clock, Config const& cfg) mMetrics->NewTimer({"app", "post-on-background-thread", "delay"})) , mPostOnOverlayThreadDelay( mMetrics->NewTimer({"app", "post-on-overlay-thread", "delay"})) + , mPostOnTxValidationThreadDelay( + mMetrics->NewTimer({"app", "post-on-tx-validation-thread", "delay"})) , mPostOnLedgerCloseThreadDelay( mMetrics->NewTimer({"app", "post-on-ledger-close-thread", "delay"})) , mStartedOn(clock.system_now()) @@ -189,6 +198,14 @@ ApplicationImpl::ApplicationImpl(VirtualClock& clock, Config const& cfg) mThreadTypes[mOverlayThread->get_id()] = ThreadType::OVERLAY; } + if (mConfig.BACKGROUND_TX_VALIDATION) + { + // Keep priority unchanged as transaction validation is time-sensitive + mTxValidationThread = + std::thread{[this]() { mTxValidationIOContext->run(); }}; + mThreadTypes[mTxValidationThread->get_id()] = ThreadType::TX_VALIDATION; + } + if (mConfig.parallelLedgerClose()) { mLedgerCloseThread = @@ -879,6 +896,10 @@ ApplicationImpl::joinAllThreads() { mOverlayWork.reset(); } + if (mTxValidationWork) + { + mTxValidationWork.reset(); + } if (mEvictionWork) { mEvictionWork.reset(); @@ -896,6 +917,12 @@ ApplicationImpl::joinAllThreads() mOverlayThread->join(); } + if (mTxValidationThread) + { + LOG_INFO(DEFAULT_LOG, "Joining the tx validation thread"); + mTxValidationThread->join(); + } + if (mEvictionThread) { LOG_INFO(DEFAULT_LOG, "Joining eviction thread"); @@ -1148,7 +1175,7 @@ ApplicationImpl::applyCfgCommands() } Config const& -ApplicationImpl::getConfig() +ApplicationImpl::getConfig() const { return mConfig; } @@ -1289,13 +1316,13 @@ ApplicationImpl::getTmpDirManager() } LedgerManager& -ApplicationImpl::getLedgerManager() +ApplicationImpl::getLedgerManager() const { return *mLedgerManager; } BucketManager& -ApplicationImpl::getBucketManager() +ApplicationImpl::getBucketManager() const { return *mBucketManager; } @@ -1474,6 +1501,19 @@ ApplicationImpl::postOnOverlayThread(std::function&& f, }); } +void +ApplicationImpl::postOnTxValidationThread(std::function&& f, + std::string jobName) +{ + releaseAssert(mTxValidationIOContext); + LogSlowExecution isSlow{std::move(jobName), LogSlowExecution::Mode::MANUAL, + "executed after"}; + asio::post(*mTxValidationIOContext, [this, f = std::move(f), isSlow]() { + mPostOnTxValidationThreadDelay.Update(isSlow.checkElapsedTime()); + f(); + }); +} + void ApplicationImpl::postOnLedgerCloseThread(std::function&& f, std::string jobName) @@ -1527,7 +1567,7 @@ ApplicationImpl::createDatabase() } AbstractLedgerTxnParent& -ApplicationImpl::getLedgerTxnRoot() +ApplicationImpl::getLedgerTxnRoot() const { #ifdef BUILD_TESTS if (mConfig.MODE_USES_IN_MEMORY_LEDGER) @@ -1540,7 +1580,7 @@ ApplicationImpl::getLedgerTxnRoot() } AppConnector& -ApplicationImpl::getAppConnector() +ApplicationImpl::getAppConnector() const { return *mAppConnector; } diff --git a/src/main/ApplicationImpl.h b/src/main/ApplicationImpl.h index 97d91100c9..b7002d71f8 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; @@ -60,8 +60,8 @@ class ApplicationImpl : public Application virtual void syncAllMetrics() override; virtual void clearMetrics(std::string const& domain) override; virtual TmpDirManager& getTmpDirManager() override; - virtual LedgerManager& getLedgerManager() override; - virtual BucketManager& getBucketManager() override; + virtual LedgerManager& getLedgerManager() const override; + virtual BucketManager& getBucketManager() const override; virtual LedgerApplyManager& getLedgerApplyManager() 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; @@ -93,6 +93,8 @@ class ApplicationImpl : public Application virtual void postOnOverlayThread(std::function&& f, std::string jobName) override; + virtual void postOnTxValidationThread(std::function&& f, + std::string jobName) override; virtual void postOnLedgerCloseThread(std::function&& f, std::string jobName) override; virtual void start() override; @@ -140,7 +142,7 @@ class ApplicationImpl : public Application virtual Hash const& getNetworkID() const override; - virtual AbstractLedgerTxnParent& getLedgerTxnRoot() override; + virtual AbstractLedgerTxnParent& getLedgerTxnRoot() const override; private: VirtualClock& mVirtualClock; @@ -165,6 +167,9 @@ class ApplicationImpl : public Application std::unique_ptr mOverlayIOContext; std::unique_ptr mOverlayWork; + std::unique_ptr mTxValidationIOContext; + std::unique_ptr mTxValidationWork; + std::unique_ptr mLedgerCloseIOContext; std::unique_ptr mLedgerCloseWork; @@ -217,6 +222,7 @@ class ApplicationImpl : public Application std::vector mWorkerThreads; std::optional mOverlayThread; + std::optional mTxValidationThread; std::optional mLedgerCloseThread; // Unlike mWorkerThreads (which are low priority), eviction scans require a @@ -243,6 +249,7 @@ class ApplicationImpl : public Application medida::Timer& mPostOnMainThreadDelay; medida::Timer& mPostOnBackgroundThreadDelay; medida::Timer& mPostOnOverlayThreadDelay; + medida::Timer& mPostOnTxValidationThreadDelay; medida::Timer& mPostOnLedgerCloseThreadDelay; VirtualClock::system_time_point mStartedOn; diff --git a/src/main/CommandHandler.cpp b/src/main/CommandHandler.cpp index 7e18453e75..cce439e07f 100644 --- a/src/main/CommandHandler.cpp +++ b/src/main/CommandHandler.cpp @@ -967,7 +967,7 @@ CommandHandler::tx(std::string const& params, std::string& retStr) { // Add it to our current set and make sure it is valid. auto addResult = - mApp.getHerder().recvTransaction(transaction, true); + mApp.getHerder().recvTransaction(transaction, true, false); root["status"] = TX_STATUS_STRING[static_cast(addResult.code)]; if (addResult.code == @@ -1398,7 +1398,7 @@ CommandHandler::testTx(std::string const& params, std::string& retStr) txFrame = fromAccount.tx({payment(toAccount, paymentAmount)}); } - auto addResult = mApp.getHerder().recvTransaction(txFrame, true); + auto addResult = mApp.getHerder().recvTransaction(txFrame, true, false); root["status"] = TX_STATUS_STRING[static_cast(addResult.code)]; if (addResult.code == TransactionQueue::AddResultCode::ADD_STATUS_ERROR) { diff --git a/src/main/Config.cpp b/src/main/Config.cpp index 0a8af21039..e11edc1e0c 100644 --- a/src/main/Config.cpp +++ b/src/main/Config.cpp @@ -162,6 +162,7 @@ Config::Config() : NODE_SEED(SecretKey::random()) CATCHUP_RECENT = 0; BACKGROUND_OVERLAY_PROCESSING = true; EXPERIMENTAL_PARALLEL_LEDGER_APPLY = false; + BACKGROUND_TX_VALIDATION = false; BUCKETLIST_DB_INDEX_PAGE_SIZE_EXPONENT = 14; // 2^14 == 16 kb BUCKETLIST_DB_INDEX_CUTOFF = 20; // 20 mb BUCKETLIST_DB_MEMORY_FOR_CACHING = 0; @@ -1083,6 +1084,8 @@ Config::processConfig(std::shared_ptr t) ARTIFICIALLY_DELAY_LEDGER_CLOSE_FOR_TESTING = std::chrono::milliseconds(readInt(item)); }}, + {"EXPERIMENTAL_BACKGROUND_TX_VALIDATION", + [&]() { BACKGROUND_TX_VALIDATION = 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 a65b3ecbca..0d9a2555cd 100644 --- a/src/main/Config.h +++ b/src/main/Config.h @@ -478,12 +478,15 @@ class Config : public std::enable_shared_from_this // index. size_t BUCKETLIST_DB_INDEX_CUTOFF; - // Enable parallel processing of overlay operations (experimental) + // Enable parallel processing of overlay operations bool BACKGROUND_OVERLAY_PROCESSING; // Enable parallel block application (experimental) bool EXPERIMENTAL_PARALLEL_LEDGER_APPLY; + // Enable parallel transaction validation (experimental) + bool BACKGROUND_TX_VALIDATION; + // 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 c8d3caba2b..c781909bc1 100644 --- a/src/main/test/CommandHandlerTests.cpp +++ b/src/main/test/CommandHandlerTests.cpp @@ -503,7 +503,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/overlay/OverlayManager.h b/src/overlay/OverlayManager.h index 5e8e0edf46..e0cb99d040 100644 --- a/src/overlay/OverlayManager.h +++ b/src/overlay/OverlayManager.h @@ -94,7 +94,7 @@ class OverlayManager // Process incoming transaction, pass it down to the transaction queue virtual void recvTransaction(StellarMessage const& msg, Peer::pointer peer, - Hash const& index) = 0; + std::shared_ptr index) = 0; // removes msgID from the floodgate's internal state // as it's not tracked anymore, calling "broadcast" with a (now forgotten) @@ -196,6 +196,9 @@ class OverlayManager // drops all connections virtual void shutdown() = 0; + // Update snapshots used for background transaction validation + virtual void updateSnapshots() = 0; + virtual bool isShuttingDown() const = 0; virtual void recordMessageMetric(StellarMessage const& stellarMsg, diff --git a/src/overlay/OverlayManagerImpl.cpp b/src/overlay/OverlayManagerImpl.cpp index 362702e742..f1a349b932 100644 --- a/src/overlay/OverlayManagerImpl.cpp +++ b/src/overlay/OverlayManagerImpl.cpp @@ -21,6 +21,7 @@ #include "overlay/SurveyDataManager.h" #include "overlay/TCPPeer.h" #include "overlay/TxDemandsManager.h" +#include "transactions/MutableTransactionResult.h" #include "util/GlobalChecks.h" #include "util/Logging.h" #include "util/Math.h" @@ -355,6 +356,22 @@ OverlayManagerImpl::start() // Start demand logic mTxDemandsManager.start(); + + // Create snapshots + updateSnapshots(); +} + +void +OverlayManagerImpl::updateSnapshots() +{ + ZoneScoped; + if (mApp.getConfig().BACKGROUND_TX_VALIDATION) + { + // Only need to update these snapshots if background transaction + // validation is enabled. + LOCK_GUARD(mSnapshotMutex, guard); + mLedgerSnapshot = std::make_unique(mApp); + } } uint32_t @@ -1169,7 +1186,7 @@ OverlayManagerImpl::checkScheduledAndCache( { return false; } - auto index = tracker->maybeGetHash().value(); + auto const& index = *tracker->maybeGetHash(); if (mScheduledMessages.exists(index)) { if (mScheduledMessages.get(index).lock()) @@ -1182,54 +1199,109 @@ OverlayManagerImpl::checkScheduledAndCache( return false; } +void +OverlayManagerImpl::handleTxAddResult(TransactionFrameBasePtr transaction, + TransactionQueue::AddResultCode addCode, + Peer::pointer peer, Hash const& index) +{ + ZoneScoped; + bool pulledRelevantTx = false; + if (!(addCode == TransactionQueue::AddResultCode::ADD_STATUS_PENDING || + addCode == TransactionQueue::AddResultCode::ADD_STATUS_DUPLICATE)) + { + forgetFloodedMsg(index); + CLOG_DEBUG(Overlay, + "Peer::recvTransaction Discarded transaction {} from {}", + hexAbbrev(transaction->getFullHash()), peer->toString()); + } + else + { + bool dup = + addCode == TransactionQueue::AddResultCode::ADD_STATUS_DUPLICATE; + if (!dup) + { + pulledRelevantTx = true; + } + CLOG_DEBUG(Overlay, + "Peer::recvTransaction Received {} transaction {} from {}", + (dup ? "duplicate" : "unique"), + hexAbbrev(transaction->getFullHash()), peer->toString()); + } + + auto const& om = getOverlayMetrics(); + auto& meter = + pulledRelevantTx ? om.mPulledRelevantTxs : om.mPulledIrrelevantTxs; + meter.Mark(); +} + void OverlayManagerImpl::recvTransaction(StellarMessage const& msg, - Peer::pointer peer, Hash const& index) + Peer::pointer peer, + std::shared_ptr index) { ZoneScoped; + releaseAssert(index != nullptr); auto transaction = TransactionFrameBase::makeTransactionFromWire( mApp.getNetworkID(), msg.transaction()); if (transaction) { // record that this peer sent us this transaction // add it to the floodmap so that this peer gets credit for it - recvFloodedMsgID(msg, peer, index); + recvFloodedMsgID(msg, peer, *index); mTxDemandsManager.recordTxPullLatency(transaction->getFullHash(), peer); - // add it to our current set - // and make sure it is valid - auto addResult = mApp.getHerder().recvTransaction(transaction, false); - bool pulledRelevantTx = false; - if (!(addResult.code == - TransactionQueue::AddResultCode::ADD_STATUS_PENDING || - addResult.code == - TransactionQueue::AddResultCode::ADD_STATUS_DUPLICATE)) + if (mApp.getConfig().BACKGROUND_TX_VALIDATION) { - forgetFloodedMsg(index); - CLOG_DEBUG(Overlay, - "Peer::recvTransaction Discarded transaction {} from {}", - hexAbbrev(transaction->getFullHash()), peer->toString()); + auto closeTime = mApp.getLedgerManager() + .getLastClosedLedgerHeader() + .header.scpValue.closeTime; + auto offset = + getUpperBoundCloseTimeOffset(mApp.getAppConnector(), closeTime); + mApp.postOnTxValidationThread( + [this, transaction, offset, peer, index]() { + ZoneScopedN("background checkValid"); + LOCK_GUARD(mSnapshotMutex, guard); + releaseAssert(mLedgerSnapshot); + + auto txResult = + transaction->checkValid(*mLedgerSnapshot, 0, 0, offset); + mApp.postOnMainThread( + [this, transaction, txResult, peer, index]() { + if (txResult->isSuccess()) + { + // add it to our current set + // and make sure it is valid + auto addCode = mApp.getHerder() + .recvTransaction(transaction, + false, true) + .code; + handleTxAddResult(transaction, addCode, peer, + *index); + } + else + { + // Record as an invalid transaction + handleTxAddResult( + transaction, + TransactionQueue::AddResultCode:: + ADD_STATUS_ERROR, + peer, *index); + } + }, + "handle pre-validated transaction"); + }, + "checkValid"); } else { - bool dup = addResult.code == - TransactionQueue::AddResultCode::ADD_STATUS_DUPLICATE; - if (!dup) - { - pulledRelevantTx = true; - } - CLOG_DEBUG( - Overlay, - "Peer::recvTransaction Received {} transaction {} from {}", - (dup ? "duplicate" : "unique"), - hexAbbrev(transaction->getFullHash()), peer->toString()); + // add it to our current set + // and make sure it is valid + auto addCode = mApp.getHerder() + .recvTransaction(transaction, false, false) + .code; + handleTxAddResult(transaction, addCode, peer, *index); } - - auto const& om = getOverlayMetrics(); - auto& meter = - pulledRelevantTx ? om.mPulledRelevantTxs : om.mPulledIrrelevantTxs; - meter.Mark(); } } diff --git a/src/overlay/OverlayManagerImpl.h b/src/overlay/OverlayManagerImpl.h index 60d9111762..e9d12f4ecb 100644 --- a/src/overlay/OverlayManagerImpl.h +++ b/src/overlay/OverlayManagerImpl.h @@ -8,6 +8,7 @@ #include "PeerAuth.h" #include "PeerDoor.h" #include "PeerManager.h" +#include "herder/TransactionQueue.h" #include "herder/TxSetFrame.h" #include "ledger/LedgerTxn.h" #include "overlay/Floodgate.h" @@ -115,7 +116,7 @@ class OverlayManagerImpl : public OverlayManager bool recvFloodedMsgID(StellarMessage const& msg, Peer::pointer peer, Hash const& msgID) override; void recvTransaction(StellarMessage const& msg, Peer::pointer peer, - Hash const& index) override; + std::shared_ptr index) override; void forgetFloodedMsg(Hash const& msgID) override; void recvTxDemand(FloodDemand const& dmd, Peer::pointer peer) override; bool @@ -164,6 +165,8 @@ class OverlayManagerImpl : public OverlayManager void start() override; void shutdown() override; + void updateSnapshots() override; + bool isShuttingDown() const override; void recordMessageMetric(StellarMessage const& stellarMsg, @@ -184,6 +187,18 @@ class OverlayManagerImpl : public OverlayManager RandomEvictionCache> mScheduledMessages; + // Ledger snapshot used for background transaction validation. Must be + // updated (replaced) after each new ledger via the `updateSnapshots` + // function. + std::unique_ptr mLedgerSnapshot; + +// Mutex to protect transaction validation snapshots +#ifdef USE_TRACY + mutable TracyLockable(std::mutex, mSnapshotMutex); +#else + std::mutex mutable mSnapshotMutex; +#endif + void triggerPeerResolution(); std::pair, bool> resolvePeers(std::vector const& peers); @@ -219,5 +234,10 @@ class OverlayManagerImpl : public OverlayManager bool checkScheduledAndCache( std::shared_ptr tracker) override; + + // Credit `peer` and record metrics about `transaction`. + void handleTxAddResult(TransactionFrameBasePtr transaction, + TransactionQueue::AddResultCode addCode, + Peer::pointer peer, Hash const& index); }; } diff --git a/src/overlay/Peer.cpp b/src/overlay/Peer.cpp index 193d7b01b7..96e2fa2c94 100644 --- a/src/overlay/Peer.cpp +++ b/src/overlay/Peer.cpp @@ -91,11 +91,11 @@ CapacityTrackedMessage::CapacityTrackedMessage(std::weak_ptr peer, self->beginMessageProcessing(mMsg); if (mMsg.type() == SCP_MESSAGE || mMsg.type() == TRANSACTION) { - mMaybeHash = xdrBlake2(msg); + mMaybeHash = std::make_shared(xdrBlake2(msg)); } } -std::optional +std::shared_ptr CapacityTrackedMessage::maybeGetHash() const { return mMaybeHash; @@ -1348,9 +1348,9 @@ Peer::recvTransaction(CapacityTrackedMessage const& msg) { ZoneScoped; releaseAssert(threadIsMain()); - releaseAssert(msg.maybeGetHash()); + releaseAssert(msg.maybeGetHash() != nullptr); mAppConnector.getOverlayManager().recvTransaction( - msg.getMessage(), shared_from_this(), msg.maybeGetHash().value()); + msg.getMessage(), shared_from_this(), msg.maybeGetHash()); } Hash @@ -1488,14 +1488,13 @@ Peer::recvSCPMessage(CapacityTrackedMessage const& msg) // add it to the floodmap so that this peer gets credit for it releaseAssert(msg.maybeGetHash()); mAppConnector.getOverlayManager().recvFloodedMsgID( - msg.getMessage(), shared_from_this(), msg.maybeGetHash().value()); + msg.getMessage(), shared_from_this(), *msg.maybeGetHash()); auto res = mAppConnector.getHerder().recvSCPEnvelope(envelope); if (res == Herder::ENVELOPE_STATUS_DISCARDED) { // the message was discarded, remove it from the floodmap as well - mAppConnector.getOverlayManager().forgetFloodedMsg( - msg.maybeGetHash().value()); + mAppConnector.getOverlayManager().forgetFloodedMsg(*msg.maybeGetHash()); } } diff --git a/src/overlay/Peer.h b/src/overlay/Peer.h index d1d8bcb726..f677580aae 100644 --- a/src/overlay/Peer.h +++ b/src/overlay/Peer.h @@ -503,12 +503,12 @@ class CapacityTrackedMessage : private NonMovableOrCopyable { std::weak_ptr const mWeakPeer; StellarMessage const mMsg; - std::optional mMaybeHash; + std::shared_ptr mMaybeHash; public: CapacityTrackedMessage(std::weak_ptr peer, StellarMessage const& msg); StellarMessage const& getMessage() const; ~CapacityTrackedMessage(); - std::optional maybeGetHash() const; + std::shared_ptr maybeGetHash() const; }; } diff --git a/src/overlay/test/FloodTests.cpp b/src/overlay/test/FloodTests.cpp index 00f76eb7c7..373950e0a2 100644 --- a/src/overlay/test/FloodTests.cpp +++ b/src/overlay/test/FloodTests.cpp @@ -162,7 +162,8 @@ TEST_CASE("Flooding", "[flood][overlay][acceptance]") } // this is basically a modified version of Peer::recvTransaction auto msg = tx1->toStellarMessage(); - auto addResult = inApp->getHerder().recvTransaction(tx1, false); + auto addResult = + inApp->getHerder().recvTransaction(tx1, false, false); REQUIRE(addResult.code == TransactionQueue::AddResultCode::ADD_STATUS_PENDING); inApp->getOverlayManager().broadcastMessage(msg, diff --git a/src/overlay/test/OverlayTests.cpp b/src/overlay/test/OverlayTests.cpp index 0e47b951f3..0903a61a67 100644 --- a/src/overlay/test/OverlayTests.cpp +++ b/src/overlay/test/OverlayTests.cpp @@ -2580,10 +2580,12 @@ TEST_CASE("overlay pull mode", "[overlay][pullmode]") tx->toStellarMessage()}); auto twoNodesRecvTx = [&]() { // Node0 and Node1 know about tx0 and will advertise it to Node2 - REQUIRE(apps[0]->getHerder().recvTransaction(tx, true).code == - TransactionQueue::AddResultCode::ADD_STATUS_PENDING); - REQUIRE(apps[1]->getHerder().recvTransaction(tx, true).code == - TransactionQueue::AddResultCode::ADD_STATUS_PENDING); + REQUIRE( + apps[0]->getHerder().recvTransaction(tx, true, false).code == + TransactionQueue::AddResultCode::ADD_STATUS_PENDING); + REQUIRE( + apps[1]->getHerder().recvTransaction(tx, true, false).code == + TransactionQueue::AddResultCode::ADD_STATUS_PENDING); }; SECTION("pull mode enabled on all") diff --git a/src/simulation/ApplyLoad.cpp b/src/simulation/ApplyLoad.cpp index f1297ed9d3..59fa2ad8e5 100644 --- a/src/simulation/ApplyLoad.cpp +++ b/src/simulation/ApplyLoad.cpp @@ -405,8 +405,8 @@ ApplyLoad::benchmark() { LedgerTxn ltx(mApp.getLedgerTxnRoot()); - auto res = tx.second->checkValid(mApp.getAppConnector(), ltx, 0, 0, - UINT64_MAX); + ExtendedLedgerSnapshot ls(ltx, mApp.getAppConnector(), false); + auto res = tx.second->checkValid(ls, 0, 0, UINT64_MAX); releaseAssert((res && res->isSuccess())); } diff --git a/src/simulation/LoadGenerator.cpp b/src/simulation/LoadGenerator.cpp index 1fcca87ec0..161ae7a001 100644 --- a/src/simulation/LoadGenerator.cpp +++ b/src/simulation/LoadGenerator.cpp @@ -1551,8 +1551,8 @@ LoadGenerator::execute(TransactionFrameBasePtr txf, LoadGenMode mode, // Skip certain checks for pregenerated transactions bool isPregeneratedTx = (mode == LoadGenMode::PAY_PREGENERATED); - auto addResult = - mApp.getHerder().recvTransaction(txf, true, isPregeneratedTx); + auto addResult = mApp.getHerder().recvTransaction( + txf, true, isPregeneratedTx, isPregeneratedTx); if (addResult.code != TransactionQueue::AddResultCode::ADD_STATUS_PENDING) { diff --git a/src/test/FuzzerImpl.cpp b/src/test/FuzzerImpl.cpp index 7c300973c3..1df22d7ab4 100644 --- a/src/test/FuzzerImpl.cpp +++ b/src/test/FuzzerImpl.cpp @@ -924,13 +924,11 @@ class FuzzTransactionFrame : public TransactionFrame SignatureChecker signatureChecker{ ltx.loadHeader().current().ledgerVersion, getContentsHash(), mEnvelope.v1().signatures}; - LedgerSnapshot ltxStmt(ltx); + ExtendedLedgerSnapshot ltxStmt(ltx, app.getAppConnector(), false); // if any ill-formed Operations, do not attempt transaction application auto isInvalidOperation = [&](auto const& op, auto& opResult) { - return !op->checkValid( - app.getAppConnector(), signatureChecker, - app.getAppConnector().getLastClosedSorobanNetworkConfig(), - ltxStmt, false, opResult, mTxResult->getSorobanData()); + return !op->checkValid(signatureChecker, ltxStmt, false, opResult, + mTxResult->getSorobanData()); }; auto const& ops = getOperations(); diff --git a/src/transactions/FeeBumpTransactionFrame.cpp b/src/transactions/FeeBumpTransactionFrame.cpp index 13133648fa..621859b70a 100644 --- a/src/transactions/FeeBumpTransactionFrame.cpp +++ b/src/transactions/FeeBumpTransactionFrame.cpp @@ -154,14 +154,12 @@ FeeBumpTransactionFrame::checkSignature(SignatureChecker& signatureChecker, } MutableTxResultPtr -FeeBumpTransactionFrame::checkValid(AppConnector& app, LedgerSnapshot const& ls, +FeeBumpTransactionFrame::checkValid(ExtendedLedgerSnapshot const& ls, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const { - if (!isTransactionXDRValidForProtocol( - ls.getLedgerHeader().current().ledgerVersion, app.getConfig(), - mEnvelope) || + if (!isTransactionXDRValidForCurrentProtocol(ls, mEnvelope) || !XDRProvidesValidFee()) { auto txResult = createSuccessResult(); @@ -189,7 +187,7 @@ FeeBumpTransactionFrame::checkValid(AppConnector& app, LedgerSnapshot const& ls, } auto innerTxResult = mInnerTx->checkValidWithOptionallyChargedFee( - app, ls, current, false, lowerBoundCloseTimeOffset, + ls, current, false, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); auto finalTxResult = createSuccessResultWithNewInnerTx( std::move(txResult), std::move(innerTxResult), mInnerTx); @@ -199,11 +197,9 @@ FeeBumpTransactionFrame::checkValid(AppConnector& app, LedgerSnapshot const& ls, bool FeeBumpTransactionFrame::checkSorobanResourceAndSetError( - AppConnector& app, SorobanNetworkConfig const& cfg, uint32_t ledgerVersion, - MutableTxResultPtr txResult) const + ExtendedLedgerSnapshot const& ls, MutableTxResultPtr txResult) const { - return mInnerTx->checkSorobanResourceAndSetError(app, cfg, ledgerVersion, - txResult); + return mInnerTx->checkSorobanResourceAndSetError(ls, txResult); } bool diff --git a/src/transactions/FeeBumpTransactionFrame.h b/src/transactions/FeeBumpTransactionFrame.h index 8d005612c5..144a16c26f 100644 --- a/src/transactions/FeeBumpTransactionFrame.h +++ b/src/transactions/FeeBumpTransactionFrame.h @@ -77,12 +77,12 @@ class FeeBumpTransactionFrame : public TransactionFrameBase MutableTxResultPtr txResult) const override; MutableTxResultPtr - checkValid(AppConnector& app, LedgerSnapshot const& ls, - SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, + checkValid(ExtendedLedgerSnapshot 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(ExtendedLedgerSnapshot const& ls, + MutableTxResultPtr txResult) const override; MutableTxResultPtr createSuccessResult() const override; diff --git a/src/transactions/OperationFrame.cpp b/src/transactions/OperationFrame.cpp index 00d2d46041..6c3c21cccb 100644 --- a/src/transactions/OperationFrame.cpp +++ b/src/transactions/OperationFrame.cpp @@ -144,12 +144,9 @@ OperationFrame::apply(AppConnector& app, SignatureChecker& signatureChecker, ZoneScoped; CLOG_TRACE(Tx, "{}", xdrToCerealString(mOperation, "Operation")); - LedgerSnapshot ltxState(ltx); - std::optional cfg = - isSoroban() ? std::make_optional(app.getSorobanNetworkConfigForApply()) - : std::nullopt; - bool applyRes = checkValid(app, signatureChecker, cfg, ltxState, true, res, - sorobanData); + ExtendedLedgerSnapshot ltxState(ltx, app, true); + bool applyRes = + checkValid(signatureChecker, ltxState, true, res, sorobanData); if (applyRes) { applyRes = doApply(app, ltx, sorobanBasePrngSeed, res, sorobanData); @@ -221,18 +218,15 @@ OperationFrame::getSourceID() const // make sure sig is correct // verifies that the operation is well formed (operation specific) bool -OperationFrame::checkValid(AppConnector& app, - SignatureChecker& signatureChecker, - std::optional const& cfg, - LedgerSnapshot const& ls, bool forApply, +OperationFrame::checkValid(SignatureChecker& signatureChecker, + ExtendedLedgerSnapshot const& ls, bool forApply, OperationResult& res, std::shared_ptr sorobanData) const { ZoneScoped; bool validationResult = false; - auto validate = [this, &res, forApply, &signatureChecker, &app, - &sorobanData, &validationResult, - &cfg](LedgerSnapshot const& ls) { + auto validate = [this, &res, forApply, &signatureChecker, &sorobanData, + &validationResult](ExtendedLedgerSnapshot const& ls) { if (!isOpSupported(ls.getLedgerHeader().current())) { res.code(opNOT_SUPPORTED); @@ -267,9 +261,11 @@ OperationFrame::checkValid(AppConnector& app, isSoroban()) { releaseAssertOrThrow(sorobanData); - releaseAssertOrThrow(cfg); - validationResult = doCheckValidForSoroban( - cfg.value(), app.getConfig(), ledgerVersion, res, *sorobanData); + auto const& sorobanConfig = ls.getSorobanNetworkConfig(); + + validationResult = + doCheckValidForSoroban(sorobanConfig, ls.getConfig(), + ledgerVersion, res, *sorobanData); } else { diff --git a/src/transactions/OperationFrame.h b/src/transactions/OperationFrame.h index a18778d54c..87ec92df0f 100644 --- a/src/transactions/OperationFrame.h +++ b/src/transactions/OperationFrame.h @@ -73,9 +73,8 @@ class OperationFrame AccountID getSourceID() const; - bool checkValid(AppConnector& app, SignatureChecker& signatureChecker, - std::optional const& cfg, - LedgerSnapshot const& ls, bool forApply, + bool checkValid(SignatureChecker& signatureChecker, + ExtendedLedgerSnapshot const& ls, bool forApply, OperationResult& res, std::shared_ptr sorobanData) const; diff --git a/src/transactions/TransactionFrame.cpp b/src/transactions/TransactionFrame.cpp index 53bfb5635d..04eacb887c 100644 --- a/src/transactions/TransactionFrame.cpp +++ b/src/transactions/TransactionFrame.cpp @@ -646,11 +646,12 @@ TransactionFrame::validateSorobanOpsConsistency() const } bool -TransactionFrame::validateSorobanResources(SorobanNetworkConfig const& config, - Config const& appConfig, - uint32_t protocolVersion, +TransactionFrame::validateSorobanResources(ExtendedLedgerSnapshot const& ls, SorobanTxData& sorobanData) const { + SorobanNetworkConfig const& config = ls.getSorobanNetworkConfig(); + Config const& appConfig = ls.getConfig(); + uint32_t protocolVersion = ls.getCurrentProtocolVersion(); auto const& resources = sorobanResources(); auto const& readEntries = resources.footprint.readOnly; auto const& writeEntries = resources.footprint.readWrite; @@ -984,8 +985,7 @@ TransactionFrame::isTooEarlyForAccount(LedgerHeaderWrapper const& header, bool TransactionFrame::commonValidPreSeqNum( - AppConnector& app, std::optional const& cfg, - LedgerSnapshot const& ls, bool chargeFee, + ExtendedLedgerSnapshot const& ls, bool chargeFee, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, std::optional sorobanResourceFee, MutableTxResultPtr txResult) const @@ -1055,9 +1055,7 @@ TransactionFrame::commonValidPreSeqNum( return false; } - releaseAssert(cfg); - if (!checkSorobanResourceAndSetError(app, cfg.value(), ledgerVersion, - txResult)) + if (!checkSorobanResourceAndSetError(ls, txResult)) { return false; } @@ -1067,7 +1065,7 @@ TransactionFrame::commonValidPreSeqNum( if (sorobanData.resourceFee > getFullFee()) { sorobanTxData.pushValidationTimeDiagnosticError( - app.getConfig(), SCE_STORAGE, SCEC_EXCEEDED_LIMIT, + ls.getConfig(), SCE_STORAGE, SCEC_EXCEEDED_LIMIT, "transaction `sorobanData.resourceFee` is higher than the " "full transaction fee", {makeU64SCVal(sorobanData.resourceFee), @@ -1081,7 +1079,7 @@ TransactionFrame::commonValidPreSeqNum( INT64_MAX - sorobanResourceFee->non_refundable_fee) { sorobanTxData.pushValidationTimeDiagnosticError( - app.getConfig(), SCE_STORAGE, SCEC_INVALID_INPUT, + ls.getConfig(), SCE_STORAGE, SCEC_INVALID_INPUT, "transaction resource fees cannot be added", {makeU64SCVal(sorobanResourceFee->refundable_fee), makeU64SCVal(sorobanResourceFee->non_refundable_fee)}); @@ -1094,7 +1092,7 @@ TransactionFrame::commonValidPreSeqNum( if (sorobanData.resourceFee < resourceFees) { sorobanTxData.pushValidationTimeDiagnosticError( - app.getConfig(), SCE_STORAGE, SCEC_EXCEEDED_LIMIT, + ls.getConfig(), SCE_STORAGE, SCEC_EXCEEDED_LIMIT, "transaction `sorobanData.resourceFee` is lower than the " "actual Soroban resource fee", {makeU64SCVal(sorobanData.resourceFee), @@ -1113,7 +1111,7 @@ TransactionFrame::commonValidPreSeqNum( if (!set.emplace(lk).second) { sorobanTxData.pushValidationTimeDiagnosticError( - app.getConfig(), SCE_STORAGE, SCEC_INVALID_INPUT, + ls.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.", @@ -1292,11 +1290,10 @@ TransactionFrame::isBadSeq(LedgerHeaderWrapper const& header, } TransactionFrame::ValidationType -TransactionFrame::commonValid(AppConnector& app, - std::optional const& cfg, - SignatureChecker& signatureChecker, - LedgerSnapshot const& ls, SequenceNumber current, - bool applying, bool chargeFee, +TransactionFrame::commonValid(SignatureChecker& signatureChecker, + ExtendedLedgerSnapshot const& ls, + SequenceNumber current, bool applying, + bool chargeFee, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, std::optional sorobanResourceFee, @@ -1307,9 +1304,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, + chargeFee, sorobanResourceFee, txResult, ¤t, + &res](ExtendedLedgerSnapshot const& ls) { if (applying && (lowerBoundCloseTimeOffset != 0 || upperBoundCloseTimeOffset != 0)) { @@ -1317,9 +1314,9 @@ TransactionFrame::commonValid(AppConnector& app, "Applying transaction with non-current closeTime"); } - if (!commonValidPreSeqNum( - app, cfg, ls, chargeFee, lowerBoundCloseTimeOffset, - upperBoundCloseTimeOffset, sorobanResourceFee, txResult)) + if (!commonValidPreSeqNum(ls, chargeFee, lowerBoundCloseTimeOffset, + upperBoundCloseTimeOffset, sorobanResourceFee, + txResult)) { return; } @@ -1524,8 +1521,8 @@ TransactionFrame::removeAccountSigner(AbstractLedgerTxn& ltxOuter, MutableTxResultPtr TransactionFrame::checkValidWithOptionallyChargedFee( - AppConnector& app, LedgerSnapshot const& ls, SequenceNumber current, - bool chargeFee, uint64_t lowerBoundCloseTimeOffset, + ExtendedLedgerSnapshot const& ls, SequenceNumber current, bool chargeFee, + uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const { ZoneScoped; @@ -1552,20 +1549,17 @@ TransactionFrame::checkValidWithOptionallyChargedFee( getSignatures(mEnvelope)}; std::optional sorobanResourceFee; - std::optional sorobanConfig; if (protocolVersionStartsFrom(ls.getLedgerHeader().current().ledgerVersion, SOROBAN_PROTOCOL_VERSION) && isSoroban()) { - sorobanConfig = - app.getLedgerManager().getLastClosedSorobanNetworkConfig(); sorobanResourceFee = computePreApplySorobanResourceFee( - ls.getLedgerHeader().current().ledgerVersion, sorobanConfig.value(), - app.getConfig()); + ls.getLedgerHeader().current().ledgerVersion, + ls.getSorobanNetworkConfig(), ls.getConfig()); } - bool res = commonValid(app, sorobanConfig, signatureChecker, ls, current, - false, chargeFee, lowerBoundCloseTimeOffset, - upperBoundCloseTimeOffset, sorobanResourceFee, + bool res = commonValid(signatureChecker, ls, current, false, chargeFee, + lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset, + sorobanResourceFee, txResult) == ValidationType::kMaybeValid; if (res) { @@ -1574,8 +1568,8 @@ TransactionFrame::checkValidWithOptionallyChargedFee( auto const& op = mOperations[i]; auto& opResult = txResult->getOpResultAt(i); - if (!op->checkValid(app, signatureChecker, sorobanConfig, ls, false, - opResult, txResult->getSorobanData())) + if (!op->checkValid(signatureChecker, ls, false, opResult, + txResult->getSorobanData())) { // it's OK to just fast fail here and not try to call // checkValid on all operations as the resulting object @@ -1595,7 +1589,7 @@ TransactionFrame::checkValidWithOptionallyChargedFee( } MutableTxResultPtr -TransactionFrame::checkValid(AppConnector& app, LedgerSnapshot const& ls, +TransactionFrame::checkValid(ExtendedLedgerSnapshot const& ls, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const @@ -1604,26 +1598,22 @@ 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(ls, mEnvelope)) { auto txResult = createSuccessResult(); txResult->setResultCode(txMALFORMED); return txResult; } - return checkValidWithOptionallyChargedFee(app, ls, current, true, + return checkValidWithOptionallyChargedFee(ls, current, true, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); } bool TransactionFrame::checkSorobanResourceAndSetError( - AppConnector& app, SorobanNetworkConfig const& cfg, uint32_t ledgerVersion, - MutableTxResultPtr txResult) const + ExtendedLedgerSnapshot const& ls, MutableTxResultPtr txResult) const { - if (!validateSorobanResources(cfg, app.getConfig(), ledgerVersion, - *txResult->getSorobanData())) + if (!validateSorobanResources(ls, *txResult->getSorobanData())) { txResult->setInnermostResultCode(txSOROBAN_INVALID); return false; @@ -1956,14 +1946,15 @@ TransactionFrame::apply(AppConnector& app, AbstractLedgerTxn& ltx, // we'll skip trying to apply operations but we'll still // process the sequence number if needed std::optional sorobanResourceFee; - std::optional sorobanConfig; + LedgerTxn ltxTx(ltx); + ExtendedLedgerSnapshot ltxStmt(ltxTx, app, true); if (protocolVersionStartsFrom(ledgerVersion, SOROBAN_PROTOCOL_VERSION) && isSoroban()) { - sorobanConfig = app.getSorobanNetworkConfigForApply(); sorobanResourceFee = computePreApplySorobanResourceFee( - ledgerVersion, *sorobanConfig, app.getConfig()); + ledgerVersion, ltxStmt.getSorobanNetworkConfig(), + app.getConfig()); auto& sorobanData = *txResult->getSorobanData(); sorobanData.setSorobanConsumedNonRefundableFee( @@ -1972,11 +1963,8 @@ TransactionFrame::apply(AppConnector& app, AbstractLedgerTxn& ltx, declaredSorobanResourceFee() - sorobanResourceFee->non_refundable_fee); } - LedgerTxn ltxTx(ltx); - LedgerSnapshot ltxStmt(ltxTx); - auto cv = - commonValid(app, sorobanConfig, *signatureChecker, ltxStmt, 0, true, - chargeFee, 0, 0, sorobanResourceFee, txResult); + auto cv = commonValid(*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..726f015995 100644 --- a/src/transactions/TransactionFrame.h +++ b/src/transactions/TransactionFrame.h @@ -94,9 +94,7 @@ 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(ExtendedLedgerSnapshot const& ls, bool chargeFee, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, std::optional sorobanResourceFee, @@ -105,11 +103,10 @@ class TransactionFrame : public TransactionFrameBase virtual bool isBadSeq(LedgerHeaderWrapper const& header, int64_t seqNum) const; - ValidationType commonValid(AppConnector& app, - std::optional const& cfg, - SignatureChecker& signatureChecker, - LedgerSnapshot const& ls, SequenceNumber current, - bool applying, bool chargeFee, + ValidationType commonValid(SignatureChecker& signatureChecker, + ExtendedLedgerSnapshot const& ls, + SequenceNumber current, bool applying, + bool chargeFee, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset, std::optional sorobanResourceFee, @@ -138,9 +135,7 @@ class TransactionFrame : public TransactionFrameBase bool extraSignersExist() const; bool validateSorobanOpsConsistency() const; - bool validateSorobanResources(SorobanNetworkConfig const& config, - Config const& appConfig, - uint32_t protocolVersion, + bool validateSorobanResources(ExtendedLedgerSnapshot const& ls, SorobanTxData& sorobanData) const; int64_t refundSorobanFee(AbstractLedgerTxn& ltx, AccountID const& feeSource, MutableTransactionResultBase& txResult) const; @@ -210,16 +205,16 @@ class TransactionFrame : public TransactionFrameBase bool checkExtraSigners(SignatureChecker& signatureChecker) const; MutableTxResultPtr checkValidWithOptionallyChargedFee( - AppConnector& app, LedgerSnapshot const& ls, SequenceNumber current, + ExtendedLedgerSnapshot const& ls, SequenceNumber current, bool chargeFee, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const; MutableTxResultPtr - checkValid(AppConnector& app, LedgerSnapshot const& ls, - SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, + checkValid(ExtendedLedgerSnapshot 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(ExtendedLedgerSnapshot const& ls, + MutableTxResultPtr txResult) const override; MutableTxResultPtr createSuccessResult() const override; diff --git a/src/transactions/TransactionFrameBase.h b/src/transactions/TransactionFrameBase.h index 94763cdeae..99f4ef939b 100644 --- a/src/transactions/TransactionFrameBase.h +++ b/src/transactions/TransactionFrameBase.h @@ -46,12 +46,12 @@ class TransactionFrameBase TransactionMetaFrame& meta, MutableTxResultPtr txResult, Hash const& sorobanBasePrngSeed = Hash{}) const = 0; virtual MutableTxResultPtr - checkValid(AppConnector& app, LedgerSnapshot const& ls, - SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, + checkValid(ExtendedLedgerSnapshot 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(ExtendedLedgerSnapshot const& ls, + MutableTxResultPtr txResult) const = 0; virtual MutableTxResultPtr createSuccessResult() const = 0; diff --git a/src/transactions/TransactionUtils.cpp b/src/transactions/TransactionUtils.cpp index 15a177906b..03ad14945d 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 const& 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(ExtendedLedgerSnapshot const& ls, + TransactionEnvelope const& envelope) { - uint32_t maxProtocol = cfg.CURRENT_LEDGER_PROTOCOL_VERSION; + uint32_t maxProtocol = ls.getConfig().CURRENT_LEDGER_PROTOCOL_VERSION; + uint32_t currProtocol = ls.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 2963fccf13..a69cdf562f 100644 --- a/src/transactions/TransactionUtils.h +++ b/src/transactions/TransactionUtils.h @@ -17,6 +17,7 @@ namespace stellar { class Application; +class AppConnector; class Config; class ConstLedgerTxnEntry; class ConstTrustLineWrapper; @@ -29,6 +30,7 @@ class SorobanNetworkConfig; class TransactionFrame; class TransactionFrameBase; class SorobanTxData; +class ExtendedLedgerSnapshot; struct ClaimAtom; struct LedgerHeader; struct LedgerKey; @@ -260,10 +262,12 @@ 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(ExtendedLedgerSnapshot const& ls, + TransactionEnvelope const& envelope); -uint64_t getUpperBoundCloseTimeOffset(Application& app, uint64_t lastCloseTime); +uint64_t getUpperBoundCloseTimeOffset(AppConnector const& 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 468b473568..27a980d2b5 100644 --- a/src/transactions/test/InvokeHostFunctionTests.cpp +++ b/src/transactions/test/InvokeHostFunctionTests.cpp @@ -421,8 +421,9 @@ TEST_CASE("basic contract invocation", "[tx][soroban]") addContractKeys); auto tx = invocation.createTx(&rootAccount); - auto result = - tx->checkValid(test.getApp().getAppConnector(), rootLtx, 0, 0, 0); + ExtendedLedgerSnapshot ls(rootLtx, test.getApp().getAppConnector(), + false); + auto result = tx->checkValid(ls, 0, 0, 0); REQUIRE(tx->getFullFee() == spec.getInclusionFee() + spec.getResourceFee()); @@ -760,8 +761,9 @@ TEST_CASE("Soroban footprint validation", "[tx][soroban]") MutableTxResultPtr result; { LedgerTxn ltx(test.getApp().getLedgerTxnRoot()); - result = - tx->checkValid(test.getApp().getAppConnector(), ltx, 0, 0, 0); + ExtendedLedgerSnapshot ls(ltx, test.getApp().getAppConnector(), + false); + result = tx->checkValid(ls, 0, 0, 0); } REQUIRE(result->isSuccess() == shouldBeValid); @@ -777,8 +779,9 @@ TEST_CASE("Soroban footprint validation", "[tx][soroban]") MutableTxResultPtr result; { LedgerTxn ltx(test.getApp().getLedgerTxnRoot()); - result = - tx->checkValid(test.getApp().getAppConnector(), ltx, 0, 0, 0); + ExtendedLedgerSnapshot ls(ltx, test.getApp().getAppConnector(), + false); + result = tx->checkValid(ls, 0, 0, 0); } REQUIRE(result->isSuccess() == shouldBeValid); if (!shouldBeValid) @@ -806,8 +809,9 @@ TEST_CASE("Soroban footprint validation", "[tx][soroban]") MutableTxResultPtr result; { LedgerTxn ltx(test.getApp().getLedgerTxnRoot()); - result = - tx->checkValid(test.getApp().getAppConnector(), ltx, 0, 0, 0); + ExtendedLedgerSnapshot ls(ltx, test.getApp().getAppConnector(), + false); + result = tx->checkValid(ls, 0, 0, 0); } REQUIRE(result->isSuccess() == shouldBeValid); if (!shouldBeValid) @@ -1457,7 +1461,8 @@ TEST_CASE("transaction validation diagnostics", "[tx][soroban]") MutableTxResultPtr result; { LedgerTxn ltx(test.getApp().getLedgerTxnRoot()); - result = tx->checkValid(test.getApp().getAppConnector(), ltx, 0, 0, 0); + ExtendedLedgerSnapshot ls(ltx, test.getApp().getAppConnector(), false); + result = tx->checkValid(ls, 0, 0, 0); } REQUIRE(!test.isTxValid(tx)); diff --git a/src/transactions/test/SorobanTxTestUtils.cpp b/src/transactions/test/SorobanTxTestUtils.cpp index 9a38f397bf..cf1308f1ec 100644 --- a/src/transactions/test/SorobanTxTestUtils.cpp +++ b/src/transactions/test/SorobanTxTestUtils.cpp @@ -810,7 +810,8 @@ SorobanTest::invokeArchivalOp(TransactionFrameBaseConstPtr tx, MutableTxResultPtr result; { LedgerTxn ltx(getApp().getLedgerTxnRoot()); - result = tx->checkValid(getApp().getAppConnector(), ltx, 0, 0, 0); + ExtendedLedgerSnapshot ls(ltx, getApp().getAppConnector(), false); + result = tx->checkValid(ls, 0, 0, 0); } REQUIRE(result->isSuccess()); int64_t initBalance = getRoot().getBalance(); @@ -1090,7 +1091,8 @@ bool SorobanTest::isTxValid(TransactionFrameBaseConstPtr tx) { LedgerTxn ltx(getApp().getLedgerTxnRoot()); - auto ret = tx->checkValid(getApp().getAppConnector(), ltx, 0, 0, 0); + ExtendedLedgerSnapshot ls(ltx, getApp().getAppConnector(), false); + auto ret = tx->checkValid(ls, 0, 0, 0); return ret->isSuccess(); } @@ -1100,8 +1102,8 @@ SorobanTest::invokeTx(TransactionFrameBaseConstPtr tx, { { LedgerTxn ltx(getApp().getLedgerTxnRoot()); - REQUIRE(tx->checkValid(getApp().getAppConnector(), ltx, 0, 0, 0) - ->isSuccess()); + ExtendedLedgerSnapshot ls(ltx, getApp().getAppConnector(), false); + REQUIRE(tx->checkValid(ls, 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..b78ab3fe13 100644 --- a/src/transactions/test/TransactionTestFrame.cpp +++ b/src/transactions/test/TransactionTestFrame.cpp @@ -95,20 +95,20 @@ TransactionTestFrame::checkValid(AppConnector& app, AbstractLedgerTxn& ltxOuter, uint64_t upperBoundCloseTimeOffset) const { LedgerTxn ltx(ltxOuter); - auto ls = LedgerSnapshot(ltx); + auto ls = ExtendedLedgerSnapshot(ltx, app, false); mTransactionTxResult = mTransactionFrame->checkValid( - app, ls, current, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); + ls, current, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); return mTransactionTxResult; } MutableTxResultPtr -TransactionTestFrame::checkValid(AppConnector& app, LedgerSnapshot const& ls, +TransactionTestFrame::checkValid(ExtendedLedgerSnapshot const& ls, SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const { mTransactionTxResult = mTransactionFrame->checkValid( - app, ls, current, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); + ls, current, lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset); return mTransactionTxResult; } @@ -142,11 +142,9 @@ TransactionTestFrame::checkValidForTesting(AppConnector& app, bool TransactionTestFrame::checkSorobanResourceAndSetError( - AppConnector& app, SorobanNetworkConfig const& cfg, uint32_t ledgerVersion, - MutableTxResultPtr txResult) const + ExtendedLedgerSnapshot const& ls, MutableTxResultPtr txResult) const { - auto ret = mTransactionFrame->checkSorobanResourceAndSetError( - app, cfg, ledgerVersion, txResult); + auto ret = mTransactionFrame->checkSorobanResourceAndSetError(ls, txResult); mTransactionTxResult = txResult; return ret; } diff --git a/src/transactions/test/TransactionTestFrame.h b/src/transactions/test/TransactionTestFrame.h index 4f6f577057..c164732b8a 100644 --- a/src/transactions/test/TransactionTestFrame.h +++ b/src/transactions/test/TransactionTestFrame.h @@ -67,12 +67,12 @@ class TransactionTestFrame : public TransactionFrameBase uint64_t lowerBoundCloseTimeOffset, uint64_t upperBoundCloseTimeOffset) const; MutableTxResultPtr - checkValid(AppConnector& app, LedgerSnapshot const& ls, - SequenceNumber current, uint64_t lowerBoundCloseTimeOffset, + checkValid(ExtendedLedgerSnapshot 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(ExtendedLedgerSnapshot const& ls, + MutableTxResultPtr txResult) const override; MutableTxResultPtr createSuccessResult() const override; diff --git a/src/transactions/test/TxEnvelopeTests.cpp b/src/transactions/test/TxEnvelopeTests.cpp index 11ebeaaf1a..ad3f0cfe3e 100644 --- a/src/transactions/test/TxEnvelopeTests.cpp +++ b/src/transactions/test/TxEnvelopeTests.cpp @@ -2075,8 +2075,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")