From 4d9bddc7aed4ff47bfe46a8c35be0f45129650e9 Mon Sep 17 00:00:00 2001 From: marta-lokhova Date: Thu, 11 Jul 2024 18:50:08 -0700 Subject: [PATCH] Deprecate obsolete tx fee history table --- src/database/Database.cpp | 7 +- src/ledger/LedgerManagerImpl.cpp | 18 +--- src/test/TxTests.cpp | 45 +++------- src/test/TxTests.h | 23 +++-- src/transactions/TransactionSQL.cpp | 83 ++----------------- src/transactions/TransactionSQL.h | 9 +- src/transactions/readme.md | 4 +- .../test/InvokeHostFunctionTests.cpp | 4 +- src/transactions/test/MergeTests.cpp | 9 +- src/transactions/test/PaymentTests.cpp | 3 +- src/transactions/test/TxEnvelopeTests.cpp | 4 +- 11 files changed, 50 insertions(+), 159 deletions(-) diff --git a/src/database/Database.cpp b/src/database/Database.cpp index 8b259570bb..e06f1ff016 100644 --- a/src/database/Database.cpp +++ b/src/database/Database.cpp @@ -63,7 +63,7 @@ bool Database::gDriversRegistered = false; // smallest schema version supported static unsigned long const MIN_SCHEMA_VERSION = 21; -static unsigned long const SCHEMA_VERSION = 21; +static unsigned long const SCHEMA_VERSION = 22; // These should always match our compiled version precisely, since we are // using a bundled version to get access to carray(). But in case someone @@ -213,10 +213,7 @@ Database::applySchemaUpgrade(unsigned long vers) switch (vers) { case 22: - // Add schema upgrades for version 21 -> 22 here. - // Currently, this is empty as the schema is already at version 21 and - // there are no upgrades to apply. However, this case is left here to - // allow for future upgrades to be added. + deprecateTransactionFeeHistory(*this); break; default: throw std::runtime_error("Unknown DB schema version"); diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp index 3b2bda54e7..5ced4af443 100644 --- a/src/ledger/LedgerManagerImpl.cpp +++ b/src/ledger/LedgerManagerImpl.cpp @@ -1350,7 +1350,6 @@ LedgerManagerImpl::processFeesSeqNums( { LedgerTxn ltx(ltxOuter); auto header = ltx.loadHeader().current(); - auto ledgerSeq = header.ledgerSeq; std::map accToMaxSeq; bool mergeSeen = false; @@ -1384,18 +1383,7 @@ LedgerManagerImpl::processFeesSeqNums( ledgerCloseMeta->setLastTxProcessingFeeProcessingChanges( changes); } - // Note to future: when we eliminate the txhistory and txfeehistory - // tables, the following step can be removed. - // - // Also note: for historical reasons the history tables number - // txs counting from 1, not 0. We preserve this for the time being - // in case anyone depends on it. ++index; - if (mApp.getConfig().MODE_STORES_HISTORY_MISC) - { - storeTransactionFee(mApp.getDatabase(), ledgerSeq, tx, changes, - index); - } ltxTx.commit(); } @@ -1475,7 +1463,7 @@ LedgerManagerImpl::prefetchTransactionData( } else { - tx->insertKeysForTxApply(classicKeys, {nullptr}); + tx->insertKeysForTxApply(classicKeys, nullptr); } } // Prefetch classic and soroban keys separately for greater visibility @@ -1582,8 +1570,8 @@ LedgerManagerImpl::applyTransactions( // Then finally store the results and meta into the txhistory table. // if we're running in a mode that has one. // - // Note to future: when we eliminate the txhistory and txfeehistory - // tables, the following step can be removed. + // Note to future: when we eliminate the txhistory for archiving, the + // next step can be removed. // // Also note: for historical reasons the history tables number // txs counting from 1, not 0. We preserve this for the time being diff --git a/src/test/TxTests.cpp b/src/test/TxTests.cpp index f806cf5720..2f66a2d87d 100644 --- a/src/test/TxTests.cpp +++ b/src/test/TxTests.cpp @@ -463,7 +463,7 @@ checkLiquidityPool(Application& app, PoolID const& poolID, int64_t reserveA, REQUIRE(cp.poolSharesTrustLineCount == poolSharesTrustLineCount); } -TxSetResultMeta +TransactionResultSet closeLedgerOn(Application& app, uint32 ledgerSeq, int day, int month, int year, std::vector const& txs, bool strictOrder) { @@ -471,7 +471,7 @@ closeLedgerOn(Application& app, uint32 ledgerSeq, int day, int month, int year, strictOrder); } -TxSetResultMeta +TransactionResultSet closeLedgerOn(Application& app, int day, int month, int year, std::vector const& txs, bool strictOrder) { @@ -480,7 +480,7 @@ closeLedgerOn(Application& app, int day, int month, int year, strictOrder); } -TxSetResultMeta +TransactionResultSet closeLedger(Application& app, std::vector const& txs, bool strictOrder) { @@ -493,7 +493,7 @@ closeLedger(Application& app, std::vector const& txs, return closeLedgerOn(app, nextLedgerSeq, lastCloseTime, txs, strictOrder); } -TxSetResultMeta +TransactionResultSet closeLedgerOn(Application& app, uint32 ledgerSeq, TimePoint closeTime, std::vector const& txs, bool strictOrder) { @@ -544,23 +544,11 @@ closeLedgerOn(Application& app, uint32 ledgerSeq, TimePoint closeTime, } app.getHerder().externalizeValue(txSet.first, ledgerSeq, closeTime, emptyUpgradeSteps); - auto z1 = getTransactionHistoryResults(app.getDatabase(), ledgerSeq); - auto z2 = getTransactionFeeMeta(app.getDatabase(), ledgerSeq); - REQUIRE(app.getLedgerManager().getLastClosedLedgerNum() == ledgerSeq); - - TxSetResultMeta res; - std::transform( - z1.results.begin(), z1.results.end(), z2.begin(), - std::back_inserter(res), - [](TransactionResultPair const& r1, LedgerEntryChanges const& r2) { - return std::make_pair(r1, r2); - }); - - return res; + return getTransactionHistoryResults(app.getDatabase(), ledgerSeq); } -TxSetResultMeta +TransactionResultSet closeLedger(Application& app, TxSetXDRFrameConstPtr txSet) { auto lastCloseTime = app.getLedgerManager() @@ -570,7 +558,7 @@ closeLedger(Application& app, TxSetXDRFrameConstPtr txSet) return closeLedgerOn(app, nextLedgerSeq, lastCloseTime, txSet); } -TxSetResultMeta +TransactionResultSet closeLedgerOn(Application& app, uint32 ledgerSeq, time_t closeTime, TxSetXDRFrameConstPtr txSet) { @@ -578,19 +566,10 @@ closeLedgerOn(Application& app, uint32 ledgerSeq, time_t closeTime, emptyUpgradeSteps); auto z1 = getTransactionHistoryResults(app.getDatabase(), ledgerSeq); - auto z2 = getTransactionFeeMeta(app.getDatabase(), ledgerSeq); REQUIRE(app.getLedgerManager().getLastClosedLedgerNum() == ledgerSeq); - TxSetResultMeta res; - std::transform( - z1.results.begin(), z1.results.end(), z2.begin(), - std::back_inserter(res), - [](TransactionResultPair const& r1, LedgerEntryChanges const& r2) { - return std::make_pair(r1, r2); - }); - - return res; + return z1; } SecretKey @@ -1634,17 +1613,17 @@ getFirstResultCode(TransactionFrame const& tx) } void -checkTx(int index, TxSetResultMeta& r, TransactionResultCode expected) +checkTx(int index, TransactionResultSet& r, TransactionResultCode expected) { - REQUIRE(r[index].first.result.result.code() == expected); + REQUIRE(r.results[index].result.result.code() == expected); }; void -checkTx(int index, TxSetResultMeta& r, TransactionResultCode expected, +checkTx(int index, TransactionResultSet& r, TransactionResultCode expected, OperationResultCode code) { checkTx(index, r, expected); - REQUIRE(r[index].first.result.result.results()[0].code() == code); + REQUIRE(r.results[index].result.result.results()[0].code() == code); }; static void diff --git a/src/test/TxTests.h b/src/test/TxTests.h index 990c394693..921fbd30f3 100644 --- a/src/test/TxTests.h +++ b/src/test/TxTests.h @@ -22,9 +22,6 @@ class TestAccount; namespace txtest { -typedef std::vector> - TxSetResultMeta; - struct ExpectedOpResult { OperationResult mOperationResult; @@ -86,27 +83,28 @@ void checkLiquidityPool(Application& app, PoolID const& poolID, int64_t totalPoolShares, int64_t poolSharesTrustLineCount); -TxSetResultMeta +TransactionResultSet closeLedger(Application& app, std::vector const& txs = {}, bool strictOrder = false); -TxSetResultMeta +TransactionResultSet closeLedgerOn(Application& app, int day, int month, int year, std::vector const& txs = {}, bool strictOrder = false); -TxSetResultMeta +TransactionResultSet closeLedgerOn(Application& app, uint32 ledgerSeq, TimePoint closeTime, std::vector const& txs = {}, bool strictOrder = false); -TxSetResultMeta closeLedger(Application& app, TxSetXDRFrameConstPtr txSet); +TransactionResultSet closeLedger(Application& app, TxSetXDRFrameConstPtr txSet); -TxSetResultMeta closeLedgerOn(Application& app, uint32 ledgerSeq, - time_t closeTime, TxSetXDRFrameConstPtr txSet); +TransactionResultSet closeLedgerOn(Application& app, uint32 ledgerSeq, + time_t closeTime, + TxSetXDRFrameConstPtr txSet); -TxSetResultMeta +TransactionResultSet closeLedgerOn(Application& app, uint32 ledgerSeq, int day, int month, int year, std::vector const& txs = {}, bool strictOrder = false); @@ -287,9 +285,10 @@ OperationResult const& getFirstResult(TransactionFrame const& tx); OperationResultCode getFirstResultCode(TransactionFrame const& tx); // methods to check results based off meta data -void checkTx(int index, TxSetResultMeta& r, TransactionResultCode expected); +void checkTx(int index, TransactionResultSet& r, + TransactionResultCode expected); -void checkTx(int index, TxSetResultMeta& r, TransactionResultCode expected, +void checkTx(int index, TransactionResultSet& r, TransactionResultCode expected, OperationResultCode code); TransactionFrameBasePtr diff --git a/src/transactions/TransactionSQL.cpp b/src/transactions/TransactionSQL.cpp index ade31314a2..ff37172218 100644 --- a/src/transactions/TransactionSQL.cpp +++ b/src/transactions/TransactionSQL.cpp @@ -435,38 +435,6 @@ storeTxSet(Database& db, uint32_t ledgerSeq, TxSetXDRFrame const& txSet) } } -void -storeTransactionFee(Database& db, uint32_t ledgerSeq, - TransactionFrameBasePtr const& tx, - LedgerEntryChanges const& changes, uint32_t txIndex) -{ - ZoneScoped; - std::string txChanges = decoder::encode_b64(xdr::xdr_to_opaque(changes)); - - std::string txIDString = binToHex(tx->getContentsHash()); - - auto prep = db.getPreparedStatement( - "INSERT INTO txfeehistory " - "( txid, ledgerseq, txindex, txchanges) VALUES " - "(:id, :seq, :txindex, :txchanges)"); - - auto& st = prep.statement(); - st.exchange(soci::use(txIDString)); - st.exchange(soci::use(ledgerSeq)); - st.exchange(soci::use(txIndex)); - st.exchange(soci::use(txChanges)); - st.define_and_bind(); - { - auto timer = db.getInsertTimer("txfeehistory"); - st.execute(true); - } - - if (st.get_affected_rows() != 1) - { - throw std::runtime_error("Could not update data in SQL"); - } -} - TransactionResultSet getTransactionHistoryResults(Database& db, uint32 ledgerSeq) { @@ -498,35 +466,6 @@ getTransactionHistoryResults(Database& db, uint32 ledgerSeq) return res; } -std::vector -getTransactionFeeMeta(Database& db, uint32 ledgerSeq) -{ - ZoneScoped; - std::vector res; - std::string changes64; - auto prep = - db.getPreparedStatement("SELECT txchanges FROM txfeehistory " - "WHERE ledgerseq = :lseq ORDER BY txindex ASC"); - auto& st = prep.statement(); - - st.exchange(soci::into(changes64)); - st.exchange(soci::use(ledgerSeq)); - st.define_and_bind(); - st.execute(true); - while (st.got_data()) - { - std::vector changesRaw; - decoder::decode_b64(changes64, changesRaw); - - xdr::xdr_get g1(&changesRaw.front(), &changesRaw.back() + 1); - res.emplace_back(); - xdr_argpack_archive(g1, res.back()); - - st.fetch(); - } - return res; -} - size_t copyTransactionsToStream(Application& app, soci::session& sess, uint32_t ledgerSeq, uint32_t ledgerCount, @@ -630,14 +569,19 @@ createTxSetHistoryTable(Database& db) ")"; } +void +deprecateTransactionFeeHistory(Database& db) +{ + ZoneScoped; + db.getSession() << "DROP TABLE IF EXISTS txfeehistory"; +} + void dropTransactionHistory(Database& db, Config const& cfg) { ZoneScoped; db.getSession() << "DROP TABLE IF EXISTS txhistory"; - db.getSession() << "DROP TABLE IF EXISTS txfeehistory"; - // txmeta only supported when BucketListDB is not enabled std::string txMetaColumn = cfg.isUsingBucketListDB() ? "" : "txmeta TEXT NOT NULL,"; @@ -654,15 +598,6 @@ dropTransactionHistory(Database& db, Config const& cfg) db.getSession() << "CREATE INDEX histbyseq ON txhistory (ledgerseq);"; - db.getSession() << "CREATE TABLE txfeehistory (" - "txid CHARACTER(64) NOT NULL," - "ledgerseq INT NOT NULL CHECK (ledgerseq >= 0)," - "txindex INT NOT NULL," - "txchanges TEXT NOT NULL," - "PRIMARY KEY (ledgerseq, txindex)" - ")"; - db.getSession() << "CREATE INDEX histfeebyseq ON txfeehistory (ledgerseq);"; - createTxSetHistoryTable(db); } @@ -675,8 +610,6 @@ deleteOldTransactionHistoryEntries(Database& db, uint32_t ledgerSeq, "txhistory", "ledgerseq"); DatabaseUtils::deleteOldEntriesHelper(db.getSession(), ledgerSeq, count, "txsethistory", "ledgerseq"); - DatabaseUtils::deleteOldEntriesHelper(db.getSession(), ledgerSeq, count, - "txfeehistory", "ledgerseq"); } void @@ -687,8 +620,6 @@ deleteNewerTransactionHistoryEntries(Database& db, uint32_t ledgerSeq) "txhistory", "ledgerseq"); DatabaseUtils::deleteNewerEntriesHelper(db.getSession(), ledgerSeq, "txsethistory", "ledgerseq"); - DatabaseUtils::deleteNewerEntriesHelper(db.getSession(), ledgerSeq, - "txfeehistory", "ledgerseq"); } } diff --git a/src/transactions/TransactionSQL.h b/src/transactions/TransactionSQL.h index 51cc021b59..b63439d709 100644 --- a/src/transactions/TransactionSQL.h +++ b/src/transactions/TransactionSQL.h @@ -21,16 +21,9 @@ void storeTransaction(Database& db, uint32_t ledgerSeq, void storeTxSet(Database& db, uint32_t ledgerSeq, TxSetXDRFrame const& txSet); -void storeTransactionFee(Database& db, uint32_t ledgerSeq, - TransactionFrameBasePtr const& tx, - LedgerEntryChanges const& changes, uint32_t txIndex); - TransactionResultSet getTransactionHistoryResults(Database& db, uint32 ledgerSeq); -std::vector getTransactionFeeMeta(Database& db, - uint32 ledgerSeq); - size_t copyTransactionsToStream(Application& app, soci::session& sess, uint32_t ledgerSeq, uint32_t ledgerCount, XDROutputFileStream& txOut, @@ -38,6 +31,8 @@ size_t copyTransactionsToStream(Application& app, soci::session& sess, void createTxSetHistoryTable(Database& db); +void deprecateTransactionFeeHistory(Database& db); + void dropTransactionHistory(Database& db, Config const& cfg); void deleteOldTransactionHistoryEntries(Database& db, uint32_t ledgerSeq, diff --git a/src/transactions/readme.md b/src/transactions/readme.md index 4e6f18f094..af9ffc7eec 100644 --- a/src/transactions/readme.md +++ b/src/transactions/readme.md @@ -77,7 +77,7 @@ the result set to "Failed". ## Result When transactions are applied (success or not), the result is saved in the -"txhistory" and "txfeehistory" tables in the database. +"txhistory" table in the database. # Operations @@ -137,8 +137,6 @@ of the operation or in the case of failure records why in structured form. Results are queued in the txhistory table for other components to derive data: historical module for uploading it for long term storage, but also for API servers to consume externally. -The txfeehistory table is additional meta data that tracks changes to the ledger -done before transactions are applied. ## List of operations See `src/xdr/Stellar-transaction.x` for a detailed list of all operations and results. diff --git a/src/transactions/test/InvokeHostFunctionTests.cpp b/src/transactions/test/InvokeHostFunctionTests.cpp index 2c1e6caf10..dceec20523 100644 --- a/src/transactions/test/InvokeHostFunctionTests.cpp +++ b/src/transactions/test/InvokeHostFunctionTests.cpp @@ -1265,9 +1265,9 @@ TEST_CASE_VERSIONS("refund is sent to fee-bump source", auto const feeCharged = afterV20 ? txFeeWithRefund : 1'040'971; REQUIRE( - r.at(0).first.result.result.innerResultPair().result.feeCharged == + r.results.at(0).result.result.innerResultPair().result.feeCharged == feeCharged - 100); - REQUIRE(r.at(0).first.result.feeCharged == feeCharged); + REQUIRE(r.results.at(0).result.feeCharged == feeCharged); REQUIRE(feeBumper.getBalance() == feeBumperStartingBalance - txFeeWithRefund); diff --git a/src/transactions/test/MergeTests.cpp b/src/transactions/test/MergeTests.cpp index cf4898f937..f22d96e4e7 100644 --- a/src/transactions/test/MergeTests.cpp +++ b/src/transactions/test/MergeTests.cpp @@ -653,7 +653,8 @@ TEST_CASE_VERSIONS("merge", "[tx][merge]") auto r = closeLedger(*app, {tx1, txMinSeqNumSrc}, true); - REQUIRE(r[0].first.result.result.results()[0] + REQUIRE(r.results[0] + .result.result.results()[0] .tr() .accountMergeResult() .code() == ACCOUNT_MERGE_SEQNUM_TOO_FAR); @@ -667,7 +668,8 @@ TEST_CASE_VERSIONS("merge", "[tx][merge]") {a1.op(accountMerge(b1))}, {a1}); auto r = closeLedger(*app, {tx1, txMinSeqNumSrc}, true); - REQUIRE(r[0].first.result.result.results()[0] + REQUIRE(r.results[0] + .result.result.results()[0] .tr() .accountMergeResult() .code() == ACCOUNT_MERGE_SEQNUM_TOO_FAR); @@ -686,7 +688,8 @@ TEST_CASE_VERSIONS("merge", "[tx][merge]") auto r = closeLedger(*app, {tx1, tx2}, true); checkTx(0, r, txSUCCESS); - REQUIRE(r[1].first.result.result.results()[0] + REQUIRE(r.results[1] + .result.result.results()[0] .tr() .accountMergeResult() .code() == ACCOUNT_MERGE_SEQNUM_TOO_FAR); diff --git a/src/transactions/test/PaymentTests.cpp b/src/transactions/test/PaymentTests.cpp index 449b3ec95d..c6e3e6db35 100644 --- a/src/transactions/test/PaymentTests.cpp +++ b/src/transactions/test/PaymentTests.cpp @@ -288,7 +288,8 @@ TEST_CASE_VERSIONS("payment", "[tx][payment]") auto r = closeLedger(*app, {tx1, tx2}, /* strictOrder */ true); checkTx(0, r, txSUCCESS); checkTx(1, r, txFAILED); - REQUIRE(r[1].first.result.result.results()[0] + REQUIRE(r.results[1] + .result.result.results()[0] .tr() .paymentResult() .code() == PAYMENT_UNDERFUNDED); diff --git a/src/transactions/test/TxEnvelopeTests.cpp b/src/transactions/test/TxEnvelopeTests.cpp index 4e2a8ebadc..82cacc3406 100644 --- a/src/transactions/test/TxEnvelopeTests.cpp +++ b/src/transactions/test/TxEnvelopeTests.cpp @@ -2388,9 +2388,9 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]") checkTx(0, r, txSUCCESS); checkTx(1, r, txFAILED); REQUIRE(PaymentOpFrame::getInnerCode( - r[1].first.result.result.results()[0]) == + r.results[1].result.result.results()[0]) == PAYMENT_SUCCESS); - REQUIRE(r[1].first.result.result.results()[1].code() == + REQUIRE(r.results[1].result.result.results()[1].code() == opBAD_AUTH); }); }