FIXES XRPLF#2597: historical transaction retrieval no longer needs to…

… pass current validity check; introduced additional RPC error code for database deserialization error.

src/ripple/app/ledger/ConsensusTransSetSF.cpp

@@ -81,7 +81,7 @@ ConsensusTransSetSF::getNode (SHAMapHash const& nodeHash) const
     if (m_nodeCache.retrieve (nodeHash, nodeData))
         return nodeData;
 
-    auto txn = app_.getMasterTransaction().fetch(nodeHash.as_uint256(), false);
+    auto txn = app_.getMasterTransaction().fetch_from_cache (nodeHash.as_uint256());
 
     if (txn)
     {

src/ripple/app/ledger/TransactionMaster.h

@@ -20,6 +20,7 @@
 #ifndef RIPPLE_APP_LEDGER_TRANSACTIONMASTER_H_INCLUDED
 #define RIPPLE_APP_LEDGER_TRANSACTIONMASTER_H_INCLUDED
 
+#include <ripple/protocol/ErrorCodes.h>
 #include <ripple/shamap/SHAMapItem.h>
 #include <ripple/shamap/SHAMapTreeNode.h>
 
@@ -39,7 +40,10 @@ class TransactionMaster
     TransactionMaster& operator= (TransactionMaster const&) = delete;
 
     std::shared_ptr<Transaction>
-    fetch (uint256 const& , bool checkDisk);
+    fetch_from_cache (uint256 const&);
+
+    std::shared_ptr<Transaction>
+    fetch (uint256 const& , error_code_i& ec);
 
     std::shared_ptr<STTx const>
     fetch (std::shared_ptr<SHAMapItem> const& item,

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

@@ -45,14 +45,20 @@ bool TransactionMaster::inLedger (uint256 const& hash, std::uint32_t ledger)
 }
 
 std::shared_ptr<Transaction>
-TransactionMaster::fetch (uint256 const& txnID, bool checkDisk)
+TransactionMaster::fetch_from_cache (uint256 const& txnID)
 {
-    auto txn = mCache.fetch (txnID);
+    return mCache.fetch (txnID);
+}
+
+std::shared_ptr<Transaction>
+TransactionMaster::fetch (uint256 const& txnID, error_code_i& ec)
+{
+    auto txn = fetch_from_cache (txnID);
 
-    if (!checkDisk || txn)
+    if (txn)
         return txn;
 
-    txn = Transaction::load (txnID, mApp);
+    txn = Transaction::load (txnID, mApp, ec);
 
     if (!txn)
         return txn;
@@ -68,7 +74,7 @@ TransactionMaster::fetch (std::shared_ptr<SHAMapItem> const& item,
         bool checkDisk, std::uint32_t uCommitLedger)
 {
     std::shared_ptr<STTx const>  txn;
-    auto iTx = fetch (item->key(), false);
+    auto iTx = fetch_from_cache (item->key());
 
     if (!iTx)
     {

src/ripple/app/misc/Transaction.h

@@ -20,6 +20,7 @@
 #ifndef RIPPLE_APP_MISC_TRANSACTION_H_INCLUDED
 #define RIPPLE_APP_MISC_TRANSACTION_H_INCLUDED
 
+#include <ripple/protocol/ErrorCodes.h>
 #include <ripple/protocol/Protocol.h>
 #include <ripple/protocol/STTx.h>
 #include <ripple/protocol/TER.h>
@@ -74,14 +75,6 @@ class Transaction
         Blob const& rawTxn,
         Application& app);
 
-    static
-    Transaction::pointer
-    transactionFromSQLValidated (
-        boost::optional<std::uint64_t> const& ledgerSeq,
-        boost::optional<std::string> const& status,
-        Blob const& rawTxn,
-        Application& app);
-
     static
     TransStatus
     sqlTransactionStatus(boost::optional<std::string> const& status);
@@ -156,7 +149,7 @@ class Transaction
 
     Json::Value getJson (JsonOptions options, bool binary = false) const;
 
-    static Transaction::pointer load (uint256 const& id, Application& app);
+    static Transaction::pointer load (uint256 const& id, Application& app, error_code_i& ec);
 
 private:
     uint256         mTransactionID;

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

@@ -25,6 +25,7 @@
 #include <ripple/app/main/Application.h>
 #include <ripple/app/misc/HashRouter.h>
 #include <ripple/basics/safe_cast.h>
+#include <ripple/protocol/ErrorCodes.h>
 #include <ripple/protocol/Feature.h>
 #include <ripple/protocol/jss.h>
 #include <boost/optional.hpp>
@@ -99,25 +100,7 @@ Transaction::pointer Transaction::transactionFromSQL (
     return tr;
 }
 
-Transaction::pointer Transaction::transactionFromSQLValidated(
-    boost::optional<std::uint64_t> const& ledgerSeq,
-    boost::optional<std::string> const& status,
-    Blob const& rawTxn,
-    Application& app)
-{
-    auto ret = transactionFromSQL(ledgerSeq, status, rawTxn, app);
-
-    if (checkValidity(app.getHashRouter(),
-            *ret->getSTransaction(), app.
-                getLedgerMaster().getValidatedRules(),
-                    app.config()).first !=
-                        Validity::Valid)
-        return {};
-
-    return ret;
-}
-
-Transaction::pointer Transaction::load(uint256 const& id, Application& app)
+Transaction::pointer Transaction::load(uint256 const& id, Application& app, error_code_i& ec)
 {
     std::string sql = "SELECT LedgerSeq,Status,RawTxn "
             "FROM Transactions WHERE TransID='";
@@ -135,13 +118,29 @@ Transaction::pointer Transaction::load(uint256 const& id, Application& app)
         *db << sql, soci::into (ledgerSeq), soci::into (status),
                 soci::into (sociRawTxnBlob, rti);
         if (!db->got_data () || rti != soci::i_ok)
+        {
+            ec = rpcTXN_NOT_FOUND;
             return {};
+        }
 
         convert(sociRawTxnBlob, rawTxn);
     }
 
-    return Transaction::transactionFromSQLValidated (
-        ledgerSeq, status, rawTxn, app);
+    std::shared_ptr<Transaction> txn;
+    try
+    {
+        txn = Transaction::transactionFromSQL(
+                ledgerSeq, status, rawTxn, app);
+    }
+    catch (std::exception& e)
+    {
+        JLOG(app.journal("Ledger").warn())
+        << "Unable to deserialize transaction from raw SQL value. Error: "
+        << e.what();
+        ec = rpcDB_DESERIALIZATION;
+    }
+
+    return txn;
 }
 
 // options 1 to include the date of the transaction

src/ripple/protocol/ErrorCodes.h

@@ -132,7 +132,8 @@ enum error_code_i
     rpcNOT_IMPL              = 74,
     rpcNOT_SUPPORTED         = 75,
     rpcBAD_KEY_TYPE          = 76,
-    rpcLAST = rpcBAD_KEY_TYPE      // rpcLAST should always equal the last code.
+    rpcDB_DESERIALIZATION    = 77,
+    rpcLAST = rpcDB_DESERIALIZATION   // rpcLAST should always equal the last code.
 };
 
 //------------------------------------------------------------------------------

src/ripple/protocol/impl/ErrorCodes.cpp

@@ -88,6 +88,7 @@ constexpr static ErrorInfo unorderedErrorInfos[]
     {rpcTXN_NOT_FOUND,         "txnNotFound",         "Transaction not found."},
     {rpcUNKNOWN_COMMAND,       "unknownCmd",          "Unknown method."},
     {rpcSENDMAX_MALFORMED,     "sendMaxMalformed",    "SendMax amount malformed."},
+    {rpcDB_DESERIALIZATION,    "dbDeserialization",   "Database deserialization error."},
 };
 
 // C++ does not allow you to return an array from a function.  You must

src/ripple/rpc/handlers/Tx.cpp

@@ -97,11 +97,12 @@ Json::Value doTx (RPC::Context& context)
     if (!isHexTxID (txid))
         return rpcError (rpcNOT_IMPL);
 
+    error_code_i ec = rpcSUCCESS;
     auto txn = context.app.getMasterTransaction ().fetch (
-        from_hex_text<uint256>(txid), true);
+        from_hex_text<uint256>(txid), ec);
 
     if (!txn)
-        return rpcError (rpcTXN_NOT_FOUND);
+        return rpcError (ec);
 
     Json::Value ret = txn->getJson (JsonOptions::include_date, binary);