Load transaction metadata from SQLite:

When processing the `tx` command, we will now load both the transaction
and its metadata directly from SQLite.

Previously the `tx` RPC call was querying SQLite for the transaction
and then separately querying the key-value store for the metadata.

src/ripple/app/ledger/Ledger.cpp

@@ -954,8 +954,6 @@ saveValidatedLedger(
             (void)_;
             uint256 transactionID = acceptedLedgerTx->getTransactionID();
 
-            app.getMasterTransaction().inLedger(transactionID, seq);
-
             std::string const txnId(to_string(transactionID));
             std::string const txnSeq(
                 std::to_string(acceptedLedgerTx->getTxnSeq()));
@@ -1012,6 +1010,8 @@ saveValidatedLedger(
                     acceptedLedgerTx->getTxn()->getMetaSQL(
                         seq, acceptedLedgerTx->getEscMeta()) +
                     ";");
+
+            app.getMasterTransaction().inLedger(transactionID, seq);
         }
 
         tr.commit();

src/ripple/app/ledger/TransactionMaster.h

@@ -44,19 +44,23 @@ public:
     std::shared_ptr<Transaction>
     fetch_from_cache(uint256 const&);
 
-    std::shared_ptr<Transaction>
+    std::variant<
+        std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>,
+        TxSearched>
     fetch(uint256 const&, error_code_i& ec);
 
     /**
      * Fetch transaction from the cache or database.
      *
-     * @return A boost::variant that contains either a
+     * @return A std::variant that contains either a
-     *         shared_pointer to the retrieved transaction or a
+     *         pair of shared_pointer to the retrieved transaction
-     *         bool indicating whether or not the all ledgers in
+     *         and its metadata or an enum indicating whether or not
-     *         the provided range were present in the database
+     *         the all ledgers in the provided range were present in
-     *         while the search was conducted.
+     *         the database while the search was conducted.
      */
-    boost::variant<Transaction::pointer, bool>
+    std::variant<
+        std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>,
+        TxSearched>
     fetch(
         uint256 const&,
         ClosedInterval<uint32_t> const& range,

src/ripple/app/ledger/impl/TransactionMaster.cpp

@@ -54,44 +54,55 @@ TransactionMaster::fetch_from_cache(uint256 const& txnID)
     return mCache.fetch(txnID);
 }
 
-std::shared_ptr<Transaction>
+std::variant<
+    std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>,
+    TxSearched>
 TransactionMaster::fetch(uint256 const& txnID, error_code_i& ec)
 {
-    auto txn = fetch_from_cache(txnID);
+    using TxPair =
+        std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>;
+
+    if (auto txn = fetch_from_cache(txnID); txn && !txn->isValidated())
+        return std::pair{std::move(txn), nullptr};
+
+    auto v = Transaction::load(txnID, mApp, ec);
+
+    if (std::holds_alternative<TxSearched>(v))
+        return v;
+
+    auto [txn, txnMeta] = std::get<TxPair>(v);
 
     if (txn)
-        return txn;
+        mCache.canonicalize_replace_client(txnID, txn);
 
-    txn = Transaction::load(txnID, mApp, ec);
+    return std::pair{std::move(txn), std::move(txnMeta)};
-
-    if (!txn)
-        return txn;
-
-    mCache.canonicalize_replace_client(txnID, txn);
-
-    return txn;
 }
 
-boost::variant<Transaction::pointer, bool>
+std::variant<
+    std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>,
+    TxSearched>
 TransactionMaster::fetch(
     uint256 const& txnID,
     ClosedInterval<uint32_t> const& range,
     error_code_i& ec)
 {
-    using pointer = Transaction::pointer;
+    using TxPair =
+        std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>;
 
-    auto txn = mCache.fetch(txnID);
+    if (auto txn = fetch_from_cache(txnID); txn && !txn->isValidated())
+        return std::pair{std::move(txn), nullptr};
+
+    auto v = Transaction::load(txnID, mApp, range, ec);
+
+    if (std::holds_alternative<TxSearched>(v))
+        return v;
+
+    auto [txn, txnMeta] = std::get<TxPair>(v);
 
     if (txn)
-        return txn;
+        mCache.canonicalize_replace_client(txnID, txn);
 
-    boost::variant<Transaction::pointer, bool> v =
+    return std::pair{std::move(txn), std::move(txnMeta)};
-        Transaction::load(txnID, mApp, range, ec);
-
-    if (v.which() == 0 && boost::get<pointer>(v))
-        mCache.canonicalize_replace_client(txnID, boost::get<pointer>(v));
-
-    return v;
 }
 
 std::shared_ptr<STTx const>

src/ripple/app/misc/Transaction.h

@@ -22,6 +22,7 @@
 
 #include <ripple/basics/RangeSet.h>
 #include <ripple/beast/utility/Journal.h>
+#include <ripple/ledger/TxMeta.h>
 #include <ripple/protocol/ErrorCodes.h>
 #include <ripple/protocol/Protocol.h>
 #include <ripple/protocol/STTx.h>
@@ -52,6 +53,8 @@ enum TransStatus {
     INCOMPLETE = 8   // needs more signatures
 };
 
+enum class TxSearched { all, some, unknown };
+
 // This class is for constructing and examining transactions.
 // Transactions are static so manipulation functions are unnecessary.
 class Transaction : public std::enable_shared_from_this<Transaction>,
@@ -100,6 +103,12 @@ public:
         return mInLedger;
     }
 
+    bool
+    isValidated() const
+    {
+        return mInLedger != 0;
+    }
+
     TransStatus
     getStatus() const
     {
@@ -300,10 +309,14 @@ public:
     Json::Value
     getJson(JsonOptions options, bool binary = false) const;
 
-    static pointer
+    static std::variant<
+        std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>,
+        TxSearched>
     load(uint256 const& id, Application& app, error_code_i& ec);
 
-    static boost::variant<Transaction::pointer, bool>
+    static std::variant<
+        std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>,
+        TxSearched>
     load(
         uint256 const& id,
         Application& app,
@@ -311,7 +324,9 @@ public:
         error_code_i& ec);
 
 private:
-    static boost::variant<Transaction::pointer, bool>
+    static std::variant<
+        std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>,
+        TxSearched>
     load(
         uint256 const& id,
         Application& app,

src/ripple/app/misc/impl/Transaction.cpp

@@ -105,13 +105,17 @@ Transaction::transactionFromSQL(
     return tr;
 }
 
-Transaction::pointer
+std::variant<
+    std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>,
+    TxSearched>
 Transaction::load(uint256 const& id, Application& app, error_code_i& ec)
 {
-    return boost::get<pointer>(load(id, app, boost::none, ec));
+    return load(id, app, boost::none, ec);
 }
 
-boost::variant<Transaction::pointer, bool>
+std::variant<
+    std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>,
+    TxSearched>
 Transaction::load(
     uint256 const& id,
     Application& app,
@@ -123,7 +127,9 @@ Transaction::load(
     return load(id, app, op{range}, ec);
 }
 
-boost::variant<Transaction::pointer, bool>
+std::variant<
+    std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>,
+    TxSearched>
 Transaction::load(
     uint256 const& id,
     Application& app,
@@ -131,7 +137,7 @@ Transaction::load(
     error_code_i& ec)
 {
     std::string sql =
-        "SELECT LedgerSeq,Status,RawTxn "
+        "SELECT LedgerSeq,Status,RawTxn,TxnMeta "
         "FROM Transactions WHERE TransID='";
 
     sql.append(to_string(id));
@@ -139,23 +145,24 @@ Transaction::load(
 
     boost::optional<std::uint64_t> ledgerSeq;
     boost::optional<std::string> status;
-    Blob rawTxn;
+    Blob rawTxn, rawMeta;
     {
         auto db = app.getTxnDB().checkoutDb();
-        soci::blob sociRawTxnBlob(*db);
+        soci::blob sociRawTxnBlob(*db), sociRawMetaBlob(*db);
-        soci::indicator rti;
+        soci::indicator txn, meta;
 
         *db << sql, soci::into(ledgerSeq), soci::into(status),
-            soci::into(sociRawTxnBlob, rti);
+            soci::into(sociRawTxnBlob, txn), soci::into(sociRawMetaBlob, meta);
 
         auto const got_data = db->got_data();
 
-        if ((!got_data || rti != soci::i_ok) && !range)
+        if ((!got_data || txn != soci::i_ok || meta != soci::i_ok) && !range)
-            return nullptr;
+            return TxSearched::unknown;
 
         if (!got_data)
         {
             uint64_t count = 0;
+            soci::indicator rti;
 
             *db << "SELECT COUNT(DISTINCT LedgerSeq) FROM Transactions WHERE "
                    "LedgerSeq BETWEEN "
@@ -163,17 +170,31 @@ Transaction::load(
                 soci::into(count, rti);
 
             if (!db->got_data() || rti != soci::i_ok)
-                return false;
+                return TxSearched::some;
 
-            return count == (range->last() - range->first() + 1);
+            return count == (range->last() - range->first() + 1)
+                ? TxSearched::all
+                : TxSearched::some;
         }
 
         convert(sociRawTxnBlob, rawTxn);
+        convert(sociRawMetaBlob, rawMeta);
     }
 
     try
     {
-        return Transaction::transactionFromSQL(ledgerSeq, status, rawTxn, app);
+        auto txn =
+            Transaction::transactionFromSQL(ledgerSeq, status, rawTxn, app);
+
+        if (!ledgerSeq)
+            return std::pair{std::move(txn), nullptr};
+
+        std::uint32_t inLedger =
+            rangeCheckedCast<std::uint32_t>(ledgerSeq.value());
+
+        auto txMeta = std::make_shared<TxMeta>(id, inLedger, rawMeta);
+
+        return std::pair{std::move(txn), std::move(txMeta)};
     }
     catch (std::exception& e)
     {
@@ -184,7 +205,7 @@ Transaction::load(
         ec = rpcDB_DESERIALIZATION;
     }
 
-    return nullptr;
+    return TxSearched::unknown;
 }
 
 // options 1 to include the date of the transaction

src/ripple/rpc/handlers/Tx.cpp

@@ -81,14 +81,12 @@ getMetaHex(Ledger const& ledger, uint256 const& transID, std::string& hex)
     return true;
 }
 
-enum class SearchedAll { no, yes, unknown };
-
 struct TxResult
 {
     Transaction::pointer txn;
     std::variant<std::shared_ptr<TxMeta>, Blob> meta;
     bool validated = false;
-    SearchedAll searchedAll;
+    TxSearched searchedAll;
 };
 
 struct TxArgs
@@ -119,31 +117,31 @@ doTxHelp(RPC::Context& context, TxArgs const& args)
             args.ledgerRange->first, args.ledgerRange->second);
     }
 
-    std::shared_ptr<Transaction> txn;
     auto ec{rpcSUCCESS};
 
-    result.searchedAll = SearchedAll::unknown;
+    using TxPair =
+        std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>;
+
+    result.searchedAll = TxSearched::unknown;
+
+    std::variant<TxPair, TxSearched> v;
     if (args.ledgerRange)
     {
-        boost::variant<std::shared_ptr<Transaction>, bool> v =
+        v = context.app.getMasterTransaction().fetch(args.hash, range, ec);
-            context.app.getMasterTransaction().fetch(args.hash, range, ec);
-
-        if (v.which() == 1)
-        {
-            result.searchedAll =
-                boost::get<bool>(v) ? SearchedAll::yes : SearchedAll::no;
-            return {result, rpcTXN_NOT_FOUND};
-        }
-        else
-        {
-            txn = boost::get<std::shared_ptr<Transaction>>(v);
-        }
     }
     else
     {
-        txn = context.app.getMasterTransaction().fetch(args.hash, ec);
+        v = context.app.getMasterTransaction().fetch(args.hash, ec);
     }
 
+    if (auto e = std::get_if<TxSearched>(&v))
+    {
+        result.searchedAll = *e;
+        return {result, rpcTXN_NOT_FOUND};
+    }
+
+    auto [txn, meta] = std::get<TxPair>(v);
+
     if (ec == rpcDB_DESERIALIZATION)
     {
         return {result, ec};
@@ -162,39 +160,12 @@ doTxHelp(RPC::Context& context, TxArgs const& args)
 
     std::shared_ptr<Ledger const> ledger =
         context.ledgerMaster.getLedgerBySeq(txn->getLedger());
-    // get meta data
-    if (ledger)
-    {
-        bool ok = false;
-        if (args.binary)
-        {
-            SHAMapTreeNode::TNType type;
-            auto const item = ledger->txMap().peekItem(txn->getID(), type);
 
-            if (item && type == SHAMapTreeNode::tnTRANSACTION_MD)
+    if (ledger && meta)
-            {
+    {
-                ok = true;
+        result.meta = meta;
-                SerialIter it(item->slice());
+        result.validated = isValidated(
-                it.skip(it.getVLDataLength());  // skip transaction
+            context.ledgerMaster, ledger->info().seq, ledger->info().hash);
-                Blob blob = it.getVL();
-                result.meta = std::move(blob);
-            }
-        }
-        else
-        {
-            auto rawMeta = ledger->txRead(txn->getID()).second;
-            if (rawMeta)
-            {
-                ok = true;
-                result.meta = std::make_shared<TxMeta>(
-                    txn->getID(), ledger->seq(), *rawMeta);
-            }
-        }
-        if (ok)
-        {
-            result.validated = isValidated(
-                context.ledgerMaster, ledger->info().seq, ledger->info().hash);
-        }
     }
 
     return {result, rpcSUCCESS};
@@ -214,14 +185,14 @@ populateProtoResponse(
     if (error.toErrorCode() != rpcSUCCESS)
     {
         if (error.toErrorCode() == rpcTXN_NOT_FOUND &&
-            result.searchedAll != SearchedAll::unknown)
+            result.searchedAll != TxSearched::unknown)
         {
             status = {
                 grpc::StatusCode::NOT_FOUND,
                 "txn not found. searched_all = " +
                     to_string(
-                        (result.searchedAll == SearchedAll::yes ? "true"
+                        (result.searchedAll == TxSearched::all ? "true"
-                                                                : "false"))};
+                                                               : "false"))};
         }
         else
         {
@@ -308,11 +279,11 @@ populateJsonResponse(
     if (error.toErrorCode() != rpcSUCCESS)
     {
         if (error.toErrorCode() == rpcTXN_NOT_FOUND &&
-            result.searchedAll != SearchedAll::unknown)
+            result.searchedAll != TxSearched::unknown)
         {
             response = Json::Value(Json::objectValue);
             response[jss::searched_all] =
-                (result.searchedAll == SearchedAll::yes);
+                (result.searchedAll == TxSearched::all);
             error.inject(response);
         }
         else

src/test/app/Ticket_test.cpp

@@ -791,16 +791,27 @@ class Ticket_test : public beast::unit_test::suite
                                  boost::optional<std::uint32_t> ticketSeq,
                                  TxType txType) {
             error_code_i txErrCode{rpcSUCCESS};
-            std::shared_ptr<Transaction> const tx{
-                Transaction::load(txID, env.app(), txErrCode)};
-            BEAST_EXPECT(txErrCode == rpcSUCCESS);
-            BEAST_EXPECT(tx->getLedger() == ledgerSeq);
 
-            std::shared_ptr<STTx const> const& sttx{tx->getSTransaction()};
+            using TxPair = std::
-            BEAST_EXPECT((*sttx)[sfSequence] == txSeq);
+                pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>;
-            if (ticketSeq)
+            std::variant<TxPair, TxSearched> maybeTx =
-                BEAST_EXPECT((*sttx)[sfTicketSequence] == *ticketSeq);
+                Transaction::load(txID, env.app(), txErrCode);
-            BEAST_EXPECT((*sttx)[sfTransactionType] == txType);
+
+            BEAST_EXPECT(txErrCode == rpcSUCCESS);
+            if (auto txPtr = std::get_if<TxPair>(&maybeTx))
+            {
+                std::shared_ptr<Transaction>& tx = txPtr->first;
+                BEAST_EXPECT(tx->getLedger() == ledgerSeq);
+                std::shared_ptr<STTx const> const& sttx = tx->getSTransaction();
+                BEAST_EXPECT((*sttx)[sfSequence] == txSeq);
+                if (ticketSeq)
+                    BEAST_EXPECT((*sttx)[sfTicketSequence] == *ticketSeq);
+                BEAST_EXPECT((*sttx)[sfTransactionType] == txType);
+            }
+            else
+            {
+                fail("Expected transaction was not found");
+            }
         };
 
         //   txID ledgerSeq txSeq ticketSeq          txType