diff --git a/src/ripple/app/rdb/backend/RWDBDatabase.h b/src/ripple/app/rdb/backend/RWDBDatabase.h index 4f97d1468..70d0e4ee8 100644 --- a/src/ripple/app/rdb/backend/RWDBDatabase.h +++ b/src/ripple/app/rdb/backend/RWDBDatabase.h @@ -28,9 +28,8 @@ private: struct AccountTxData { - AccountTxs transactions; - std::map> - ledgerTxMap; // ledgerSeq -> txSeq -> index in transactions + std::map> + ledgerTxMap; // ledgerSeq -> vector of transactions }; Application& app_; @@ -166,14 +165,6 @@ public: { txIt = accountData.ledgerTxMap.erase(txIt); } - accountData.transactions.erase( - std::remove_if( - accountData.transactions.begin(), - accountData.transactions.end(), - [ledgerSeq](const AccountTx& tx) { - return tx.second->getLgrSeq() < ledgerSeq; - }), - accountData.transactions.end()); } } std::size_t @@ -196,7 +187,10 @@ public: std::size_t count = 0; for (const auto& [_, accountData] : accountTxMap_) { - count += accountData.transactions.size(); + for (const auto& [_, txVector] : accountData.ledgerTxMap) + { + count += txVector.size(); + } } return count; } @@ -296,10 +290,7 @@ public: accountTxMap_[account] = AccountTxData(); auto& accountData = accountTxMap_[account]; - accountData.transactions.push_back(accTx); - accountData - .ledgerTxMap[seq][acceptedLedgerTx->getTxnSeq()] = - accountData.transactions.size() - 1; + accountData.ledgerTxMap[seq].push_back(accTx); } app_.getMasterTransaction().inLedger( @@ -466,11 +457,9 @@ public: for (const auto& [_, accountData] : accountTxMap_) { size += sizeof(AccountID) + sizeof(AccountTxData); - size += accountData.transactions.size() * sizeof(AccountTx); - for (const auto& [_, innerMap] : accountData.ledgerTxMap) + for (const auto& [_, txVector] : accountData.ledgerTxMap) { - size += sizeof(uint32_t) + - innerMap.size() * (sizeof(uint32_t) + sizeof(size_t)); + size += sizeof(uint32_t) + txVector.size() * sizeof(AccountTx); } } return size / 1024; @@ -499,11 +488,9 @@ public: for (const auto& [_, accountData] : accountTxMap_) { size += sizeof(AccountID) + sizeof(AccountTxData); - size += accountData.transactions.size() * sizeof(AccountTx); - for (const auto& [_, innerMap] : accountData.ledgerTxMap) + for (const auto& [_, txVector] : accountData.ledgerTxMap) { - size += sizeof(uint32_t) + - innerMap.size() * (sizeof(uint32_t) + sizeof(size_t)); + size += sizeof(uint32_t) + txVector.size() * sizeof(AccountTx); } } return size / 1024; @@ -608,14 +595,13 @@ public: (options.bUnlimited || result.size() < options.limit); ++txIt) { - for (const auto& [txSeq, txIndex] : txIt->second) + for (const auto& accountTx : txIt->second) { if (skipped < options.offset) { ++skipped; continue; } - AccountTx const accountTx = accountData.transactions[txIndex]; std::uint32_t const inLedger = rangeCheckedCast( accountTx.second->getLgrSeq()); accountTx.first->setStatus(COMMITTED); @@ -660,8 +646,7 @@ public: ++skipped; continue; } - AccountTx const accountTx = - accountData.transactions[innerRIt->second]; + AccountTx const accountTx = *innerRIt; std::uint32_t const inLedger = rangeCheckedCast( accountTx.second->getLgrSeq()); accountTx.first->setLedger(inLedger); @@ -695,14 +680,14 @@ public: (options.bUnlimited || result.size() < options.limit); ++txIt) { - for (const auto& [txSeq, txIndex] : txIt->second) + for (const auto& accountTx : txIt->second) { if (skipped < options.offset) { ++skipped; continue; } - const auto& [txn, txMeta] = accountData.transactions[txIndex]; + const auto& [txn, txMeta] = accountTx; result.emplace_back( txn->getSTransaction()->getSerializer().peekData(), txMeta->getAsObject().getSerializer().peekData(), @@ -746,8 +731,7 @@ public: ++skipped; continue; } - const auto& [txn, txMeta] = - accountData.transactions[innerRIt->second]; + const auto& [txn, txMeta] = *innerRIt; result.emplace_back( txn->getSTransaction()->getSerializer().peekData(), txMeta->getAsObject().getSerializer().peekData(), @@ -819,11 +803,9 @@ public: for (; txIt != txEnd; ++txIt) { std::uint32_t const ledgerSeq = txIt->first; - for (auto seqIt = txIt->second.begin(); - seqIt != txIt->second.end(); - ++seqIt) + std::uint32_t txnSeq = 0; + for (const auto& accountTx : txIt->second) { - const auto& [txnSeq, index] = *seqIt; if (lookingForMarker) { if (findLedger == ledgerSeq && findSeq == txnSeq) @@ -831,7 +813,10 @@ public: lookingForMarker = false; } else + { + ++txnSeq; continue; + } } else if (numberOfResults == 0) { @@ -840,12 +825,10 @@ public: return {newmarker, total}; } - Blob rawTxn = accountData.transactions[index] - .first->getSTransaction() + Blob rawTxn = accountTx.first->getSTransaction() ->getSerializer() .peekData(); - Blob rawMeta = accountData.transactions[index] - .second->getAsObject() + Blob rawMeta = accountTx.second->getAsObject() .getSerializer() .peekData(); @@ -859,6 +842,7 @@ public: std::move(rawMeta)); --numberOfResults; ++total; + ++txnSeq; } } } @@ -874,11 +858,11 @@ public: for (; rtxIt != rtxEnd; ++rtxIt) { std::uint32_t const ledgerSeq = rtxIt->first; + std::uint32_t txnSeq = rtxIt->second.size() - 1; for (auto innerRIt = rtxIt->second.rbegin(); innerRIt != rtxIt->second.rend(); ++innerRIt) { - const auto& [txnSeq, index] = *innerRIt; if (lookingForMarker) { if (findLedger == ledgerSeq && findSeq == txnSeq) @@ -886,7 +870,10 @@ public: lookingForMarker = false; } else + { + --txnSeq; continue; + } } else if (numberOfResults == 0) { @@ -895,12 +882,11 @@ public: return {newmarker, total}; } - Blob rawTxn = accountData.transactions[index] - .first->getSTransaction() + const auto& accountTx = *innerRIt; + Blob rawTxn = accountTx.first->getSTransaction() ->getSerializer() .peekData(); - Blob rawMeta = accountData.transactions[index] - .second->getAsObject() + Blob rawMeta = accountTx.second->getAsObject() .getSerializer() .peekData(); @@ -914,6 +900,7 @@ public: std::move(rawMeta)); --numberOfResults; ++total; + --txnSeq; } } } diff --git a/src/test/app/SHAMapStore_test.cpp b/src/test/app/SHAMapStore_test.cpp index 8a3ca0f89..9f4269667 100644 --- a/src/test/app/SHAMapStore_test.cpp +++ b/src/test/app/SHAMapStore_test.cpp @@ -206,6 +206,8 @@ public: auto const firstSeq = waitForReady(env); auto lastRotated = firstSeq - 1; + // Create ledgers 4-11 with transactions (8 ledgers) + // These transactions will survive the first rotation but not the second for (auto i = firstSeq + 1; i < deleteInterval + firstSeq; ++i) { env.fund(XRP(10000), noripple("test" + std::to_string(i))); @@ -218,6 +220,47 @@ public: SQLiteDatabase* const db = dynamic_cast(&env.app().getRelationalDatabase()); + + // Simple helper to show what's in the database + auto showDBState = [&env, &db]( + const std::string& when, + const std::string& expectation = "") { + auto [ledgerCount, firstLedger, lastLedger] = + db->getLedgerCountMinMax(); + auto txCount = db->getTransactionCount(); + auto accTxCount = db->getAccountTransactionCount(); + + std::cout << "\n" << when << ":" << std::endl; + std::cout << " Ledgers: " << firstLedger << "-" << lastLedger + << " (keeping " << ledgerCount << " ledgers)" + << std::endl; + std::cout << " Transactions: " << txCount; + std::cout << ", Account Transactions: " << accTxCount; + if (!expectation.empty()) + { + std::cout << " " << expectation; + } + std::cout << std::endl; + + // Also show RPC counts to see in-memory objects + auto const result = env.rpc("get_counts")[jss::result]; + if (result.isMember("ripple::STTx") || + result.isMember("ripple::Transaction")) + { + std::cout << " In-memory objects: " + << "STTx=" + << (result.isMember("ripple::STTx") + ? result["ripple::STTx"].asInt() + : 0) + << ", Transaction=" + << (result.isMember("ripple::Transaction") + ? result["ripple::Transaction"].asInt() + : 0) + << std::endl; + } + }; + + showDBState("Initial state"); BEAST_EXPECT(*db->getTransactionsMinLedgerSeq() == 3); for (auto i = 3; i < deleteInterval + lastRotated; ++i) @@ -229,10 +272,18 @@ public: getHash(ledgers[i]).length()); } + // After creating 8 more ledgers with transactions + showDBState("Before first rotation"); + + // Verify expected state ledgerCheck(env, deleteInterval + 1, 2); transactionCheck(env, deleteInterval); accountTransactionCheck(env, 2 * deleteInterval); + // Additional verification + BEAST_EXPECT(db->getTransactionCount() == deleteInterval); + BEAST_EXPECT(db->getAccountTransactionCount() == 2 * deleteInterval); + { // Closing one more ledger triggers a rotate env.close(); @@ -248,11 +299,21 @@ public: lastRotated = store.getLastRotated(); BEAST_EXPECT(lastRotated == 11); - // That took care of the fake hashes + showDBState( + "After FIRST rotation", + "(EXPECTED: still have 8 txns from ledgers 3-11)"); + + // First rotation: Deleted ledgers 1-2, kept 3-11 + // Transactions from ledgers 3-11 should STILL EXIST ledgerCheck(env, deleteInterval + 1, 3); - transactionCheck(env, deleteInterval); + transactionCheck(env, deleteInterval); // Still have transactions! accountTransactionCheck(env, 2 * deleteInterval); + // Additional verification + BEAST_EXPECT(db->getTransactionCount() == deleteInterval); + BEAST_EXPECT(db->getAccountTransactionCount() == 2 * deleteInterval); + BEAST_EXPECT(*db->getTransactionsMinLedgerSeq() == 3); + // The last iteration of this loop should trigger a rotate for (auto i = lastRotated - 1; i < lastRotated + deleteInterval - 1; ++i) @@ -276,9 +337,18 @@ public: BEAST_EXPECT(store.getLastRotated() == deleteInterval + lastRotated); + showDBState( + "After SECOND rotation", + "(EXPECTED: 0 txns - ledgers 3-11 were deleted)"); + ledgerCheck(env, deleteInterval + 1, lastRotated); - transactionCheck(env, 0); - accountTransactionCheck(env, 0); + transactionCheck(env, 0); // NOW transactions should be gone! + accountTransactionCheck(env, 0); // Account transactions too! + + // Additional verification - all transactions should be gone + BEAST_EXPECT(db->getTransactionCount() == 0); + BEAST_EXPECT(db->getAccountTransactionCount() == 0); + BEAST_EXPECT(!db->getTransactionsMinLedgerSeq().has_value()); } void