diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index e26f5caee..524f81c84 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/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(); diff --git a/src/ripple/app/ledger/TransactionMaster.h b/src/ripple/app/ledger/TransactionMaster.h index 9d3e06466..6c3f13bff 100644 --- a/src/ripple/app/ledger/TransactionMaster.h +++ b/src/ripple/app/ledger/TransactionMaster.h @@ -44,19 +44,23 @@ public: std::shared_ptr fetch_from_cache(uint256 const&); - std::shared_ptr + std::variant< + std::pair, std::shared_ptr>, + TxSearched> fetch(uint256 const&, error_code_i& ec); /** * Fetch transaction from the cache or database. * - * @return A boost::variant that contains either a - * shared_pointer to the retrieved transaction or a - * bool indicating whether or not the all ledgers in - * the provided range were present in the database - * while the search was conducted. + * @return A std::variant that contains either a + * pair of shared_pointer to the retrieved transaction + * and its metadata or an enum indicating whether or not + * the all ledgers in the provided range were present in + * the database while the search was conducted. */ - boost::variant + std::variant< + std::pair, std::shared_ptr>, + TxSearched> fetch( uint256 const&, ClosedInterval const& range, diff --git a/src/ripple/app/ledger/impl/TransactionMaster.cpp b/src/ripple/app/ledger/impl/TransactionMaster.cpp index 7138a3cd9..f83b1aa92 100644 --- a/src/ripple/app/ledger/impl/TransactionMaster.cpp +++ b/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 +std::variant< + std::pair, std::shared_ptr>, + TxSearched> TransactionMaster::fetch(uint256 const& txnID, error_code_i& ec) { - auto txn = fetch_from_cache(txnID); + using TxPair = + std::pair, std::shared_ptr>; + + 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(v)) + return v; + + auto [txn, txnMeta] = std::get(v); if (txn) - return txn; + mCache.canonicalize_replace_client(txnID, txn); - txn = Transaction::load(txnID, mApp, ec); - - if (!txn) - return txn; - - mCache.canonicalize_replace_client(txnID, txn); - - return txn; + return std::pair{std::move(txn), std::move(txnMeta)}; } -boost::variant +std::variant< + std::pair, std::shared_ptr>, + TxSearched> TransactionMaster::fetch( uint256 const& txnID, ClosedInterval const& range, error_code_i& ec) { - using pointer = Transaction::pointer; + using TxPair = + std::pair, std::shared_ptr>; - 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(v)) + return v; + + auto [txn, txnMeta] = std::get(v); if (txn) - return txn; + mCache.canonicalize_replace_client(txnID, txn); - boost::variant v = - Transaction::load(txnID, mApp, range, ec); - - if (v.which() == 0 && boost::get(v)) - mCache.canonicalize_replace_client(txnID, boost::get(v)); - - return v; + return std::pair{std::move(txn), std::move(txnMeta)}; } std::shared_ptr diff --git a/src/ripple/app/misc/Transaction.h b/src/ripple/app/misc/Transaction.h index 455bc4fae..4dbe2f06b 100644 --- a/src/ripple/app/misc/Transaction.h +++ b/src/ripple/app/misc/Transaction.h @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -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, @@ -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>, + TxSearched> load(uint256 const& id, Application& app, error_code_i& ec); - static boost::variant + static std::variant< + std::pair, std::shared_ptr>, + TxSearched> load( uint256 const& id, Application& app, @@ -311,7 +324,9 @@ public: error_code_i& ec); private: - static boost::variant + static std::variant< + std::pair, std::shared_ptr>, + TxSearched> load( uint256 const& id, Application& app, diff --git a/src/ripple/app/misc/impl/Transaction.cpp b/src/ripple/app/misc/impl/Transaction.cpp index 645eb79ad..aa9044095 100644 --- a/src/ripple/app/misc/impl/Transaction.cpp +++ b/src/ripple/app/misc/impl/Transaction.cpp @@ -105,13 +105,17 @@ Transaction::transactionFromSQL( return tr; } -Transaction::pointer +std::variant< + std::pair, std::shared_ptr>, + TxSearched> Transaction::load(uint256 const& id, Application& app, error_code_i& ec) { - return boost::get(load(id, app, boost::none, ec)); + return load(id, app, boost::none, ec); } -boost::variant +std::variant< + std::pair, std::shared_ptr>, + TxSearched> Transaction::load( uint256 const& id, Application& app, @@ -123,7 +127,9 @@ Transaction::load( return load(id, app, op{range}, ec); } -boost::variant +std::variant< + std::pair, std::shared_ptr>, + 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 ledgerSeq; boost::optional status; - Blob rawTxn; + Blob rawTxn, rawMeta; { auto db = app.getTxnDB().checkoutDb(); - soci::blob sociRawTxnBlob(*db); - soci::indicator rti; + soci::blob sociRawTxnBlob(*db), sociRawMetaBlob(*db); + 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) - return nullptr; + if ((!got_data || txn != soci::i_ok || meta != soci::i_ok) && !range) + 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(ledgerSeq.value()); + + auto txMeta = std::make_shared(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 diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index 0b8df1486..e5ab4b8dd 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/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, 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 txn; auto ec{rpcSUCCESS}; - result.searchedAll = SearchedAll::unknown; + using TxPair = + std::pair, std::shared_ptr>; + + result.searchedAll = TxSearched::unknown; + + std::variant v; if (args.ledgerRange) { - boost::variant, bool> v = - context.app.getMasterTransaction().fetch(args.hash, range, ec); - - if (v.which() == 1) - { - result.searchedAll = - boost::get(v) ? SearchedAll::yes : SearchedAll::no; - return {result, rpcTXN_NOT_FOUND}; - } - else - { - txn = boost::get>(v); - } + v = context.app.getMasterTransaction().fetch(args.hash, range, ec); } else { - txn = context.app.getMasterTransaction().fetch(args.hash, ec); + v = context.app.getMasterTransaction().fetch(args.hash, ec); } + if (auto e = std::get_if(&v)) + { + result.searchedAll = *e; + return {result, rpcTXN_NOT_FOUND}; + } + + auto [txn, meta] = std::get(v); + if (ec == rpcDB_DESERIALIZATION) { return {result, ec}; @@ -162,39 +160,12 @@ doTxHelp(RPC::Context& context, TxArgs const& args) std::shared_ptr 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) - { - ok = true; - SerialIter it(item->slice()); - it.skip(it.getVLDataLength()); // skip transaction - 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( - txn->getID(), ledger->seq(), *rawMeta); - } - } - if (ok) - { - result.validated = isValidated( - context.ledgerMaster, ledger->info().seq, ledger->info().hash); - } + if (ledger && meta) + { + result.meta = meta; + 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" - : "false"))}; + (result.searchedAll == TxSearched::all ? "true" + : "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 diff --git a/src/test/app/Ticket_test.cpp b/src/test/app/Ticket_test.cpp index a311bb305..f9b3e5f4e 100644 --- a/src/test/app/Ticket_test.cpp +++ b/src/test/app/Ticket_test.cpp @@ -791,16 +791,27 @@ class Ticket_test : public beast::unit_test::suite boost::optional ticketSeq, TxType txType) { error_code_i txErrCode{rpcSUCCESS}; - std::shared_ptr const tx{ - Transaction::load(txID, env.app(), txErrCode)}; - BEAST_EXPECT(txErrCode == rpcSUCCESS); - BEAST_EXPECT(tx->getLedger() == ledgerSeq); - std::shared_ptr const& sttx{tx->getSTransaction()}; - BEAST_EXPECT((*sttx)[sfSequence] == txSeq); - if (ticketSeq) - BEAST_EXPECT((*sttx)[sfTicketSequence] == *ticketSeq); - BEAST_EXPECT((*sttx)[sfTransactionType] == txType); + using TxPair = std:: + pair, std::shared_ptr>; + std::variant maybeTx = + Transaction::load(txID, env.app(), txErrCode); + + BEAST_EXPECT(txErrCode == rpcSUCCESS); + if (auto txPtr = std::get_if(&maybeTx)) + { + std::shared_ptr& tx = txPtr->first; + BEAST_EXPECT(tx->getLedger() == ledgerSeq); + std::shared_ptr 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