Skip to content

Commit

Permalink
Provide additional info with txnNotFound errors.
Browse files Browse the repository at this point in the history
* The `tx` command now supports min_ledger and max_ledger fields.
* If the requested transaction isn't found and these fields are
  provided, the error response indicates whether or not every
  ledger in the the provided range was searched.

This fixes #2924
  • Loading branch information
undertome authored and nbougalis committed Nov 28, 2019
1 parent 11cf27e commit 47501b7
Show file tree
Hide file tree
Showing 13 changed files with 458 additions and 32 deletions.
1 change: 1 addition & 0 deletions Builds/CMake/RippledCore.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,7 @@ else ()
src/test/rpc/ServerInfo_test.cpp
src/test/rpc/Status_test.cpp
src/test/rpc/Subscribe_test.cpp
src/test/rpc/Transaction_test.cpp
src/test/rpc/TransactionEntry_test.cpp
src/test/rpc/TransactionHistory_test.cpp
src/test/rpc/ValidatorRPC_test.cpp
Expand Down
18 changes: 15 additions & 3 deletions src/ripple/app/ledger/TransactionMaster.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@
#include <ripple/protocol/ErrorCodes.h>
#include <ripple/shamap/SHAMapItem.h>
#include <ripple/shamap/SHAMapTreeNode.h>
#include <ripple/basics/RangeSet.h>
#include <ripple/app/misc/Transaction.h>

namespace ripple {

class Application;
class Transaction;
class STTx;

// Tracks all transactions in memory
Expand All @@ -45,10 +46,21 @@ class TransactionMaster
std::shared_ptr<Transaction>
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.
*/
boost::variant<Transaction::pointer, bool>
fetch (uint256 const& , ClosedInterval<uint32_t> const& range, error_code_i& ec);

std::shared_ptr<STTx const>
fetch (std::shared_ptr<SHAMapItem> const& item,
SHAMapTreeNode::TNType type, bool checkDisk,
std::uint32_t uCommitLedger);
SHAMapTreeNode::TNType type, std::uint32_t uCommitLedger);

// return value: true = we had the transaction already
bool inLedger (uint256 const& hash, std::uint32_t ledger);
Expand Down
24 changes: 21 additions & 3 deletions src/ripple/app/ledger/impl/TransactionMaster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include <ripple/app/misc/Transaction.h>
#include <ripple/app/main/Application.h>
#include <ripple/protocol/STTx.h>
#include <ripple/basics/Log.h>
#include <ripple/basics/chrono.h>

namespace ripple {
Expand Down Expand Up @@ -68,10 +67,29 @@ TransactionMaster::fetch (uint256 const& txnID, error_code_i& ec)
return txn;
}

boost::variant<Transaction::pointer, bool>
TransactionMaster::fetch (uint256 const& txnID, ClosedInterval<uint32_t> const& range,
error_code_i& ec)
{
using pointer = Transaction::pointer;

auto txn = mCache.fetch (txnID);

if (txn)
return txn;

boost::variant<Transaction::pointer, bool> v = Transaction::load (
txnID, mApp, range, ec);

if (v.which () == 0 && boost::get<pointer> (v))
mCache.canonicalize (txnID, boost::get<pointer> (v));

return v;
}

std::shared_ptr<STTx const>
TransactionMaster::fetch (std::shared_ptr<SHAMapItem> const& item,
SHAMapTreeNode::TNType type,
bool checkDisk, std::uint32_t uCommitLedger)
SHAMapTreeNode::TNType type, std::uint32_t uCommitLedger)
{
std::shared_ptr<STTx const> txn;
auto iTx = fetch_from_cache (item->key());
Expand Down
14 changes: 12 additions & 2 deletions src/ripple/app/misc/Transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
#include <ripple/protocol/STTx.h>
#include <ripple/protocol/TER.h>
#include <ripple/beast/utility/Journal.h>
#include <ripple/basics/RangeSet.h>
#include <boost/optional.hpp>
#include <boost/variant.hpp>

namespace ripple {

Expand Down Expand Up @@ -63,7 +65,6 @@ class Transaction
using pointer = std::shared_ptr<Transaction>;
using ref = const pointer&;

public:
Transaction (
std::shared_ptr<STTx const> const&, std::string&, Application&) noexcept;

Expand Down Expand Up @@ -149,9 +150,18 @@ class Transaction

Json::Value getJson (JsonOptions options, bool binary = false) const;

static Transaction::pointer load (uint256 const& id, Application& app, error_code_i& ec);
static pointer
load (uint256 const& id, Application& app, error_code_i& ec);

static boost::variant<Transaction::pointer, bool>
load (uint256 const& id, Application& app, ClosedInterval<uint32_t> const& range, error_code_i& ec);

private:

static boost::variant<Transaction::pointer, bool>
load (uint256 const& id, Application& app, boost::optional<ClosedInterval<uint32_t>> const& range,
error_code_i& ec);

uint256 mTransactionID;

LedgerIndex mInLedger = 0;
Expand Down
59 changes: 47 additions & 12 deletions src/ripple/app/misc/impl/Transaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,27 @@ Transaction::pointer Transaction::transactionFromSQL (
return tr;
}

Transaction::pointer Transaction::load(uint256 const& id, Application& app, error_code_i& ec)
Transaction::pointer Transaction::load (uint256 const& id, Application& app, error_code_i& ec)
{
return boost::get<pointer> (load (id, app, boost::none, ec));
}

boost::variant<Transaction::pointer, bool>
Transaction::load (uint256 const& id, Application& app, ClosedInterval<uint32_t> const& range,
error_code_i& ec)
{
using op = boost::optional<ClosedInterval<uint32_t>>;

return load (id, app, op {range}, ec);
}

boost::variant<Transaction::pointer, bool>
Transaction::load (uint256 const& id, Application& app, boost::optional<ClosedInterval<uint32_t>> const& range,
error_code_i& ec)
{
std::string sql = "SELECT LedgerSeq,Status,RawTxn "
"FROM Transactions WHERE TransID='";
"FROM Transactions WHERE TransID='";

sql.append (to_string (id));
sql.append ("';");

Expand All @@ -117,30 +134,48 @@ Transaction::pointer Transaction::load(uint256 const& id, Application& app, erro

*db << sql, soci::into (ledgerSeq), soci::into (status),
soci::into (sociRawTxnBlob, rti);
if (!db->got_data () || rti != soci::i_ok)

auto const got_data = db->got_data ();

if ((!got_data || rti != soci::i_ok) && !range)
return nullptr;

if (!got_data)
{
ec = rpcTXN_NOT_FOUND;
return {};
uint64_t count = 0;

*db << "SELECT COUNT(DISTINCT LedgerSeq) FROM Transactions WHERE LedgerSeq BETWEEN "
<< range->first ()
<< " AND "
<< range->last ()
<< ";",
soci::into (count, rti);

if (!db->got_data () || rti != soci::i_ok)
return false;

return count == (range->last () - range->first () + 1);
}

convert(sociRawTxnBlob, rawTxn);
convert (sociRawTxnBlob, rawTxn);
}

std::shared_ptr<Transaction> txn;
try
{
txn = Transaction::transactionFromSQL(
ledgerSeq, status, rawTxn, app);
return 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();
<< "Unable to deserialize transaction from raw SQL value. Error: "
<< e.what();

ec = rpcDB_DESERIALIZATION;
}

return txn;
return nullptr;
}

// options 1 to include the date of the transaction
Expand Down
15 changes: 11 additions & 4 deletions src/ripple/net/impl/RPCCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -996,19 +996,26 @@ class RPCParser
return jvRequest;
}


// tx <transaction_id>
Json::Value parseTx (Json::Value const& jvParams)
{
Json::Value jvRequest{Json::objectValue};

if (jvParams.size () > 1)
if (jvParams.size () == 2 || jvParams.size () == 4)
{
if (jvParams[1u].asString () == jss::binary)
jvRequest[jss::binary] = true;
}

jvRequest["transaction"] = jvParams[0u].asString ();
if (jvParams.size () >= 3)
{
const auto offset = jvParams.size () == 3 ? 0 : 1;

jvRequest[jss::min_ledger] = jvParams[1u + offset].asString ();
jvRequest[jss::max_ledger] = jvParams[2u + offset].asString ();
}

jvRequest[jss::transaction] = jvParams[0u].asString ();
return jvRequest;
}

Expand Down Expand Up @@ -1182,7 +1189,7 @@ class RPCParser
{ "crawl_shards", &RPCParser::parseAsIs, 0, 2 },
{ "stop", &RPCParser::parseAsIs, 0, 0 },
{ "transaction_entry", &RPCParser::parseTransactionEntry, 2, 2 },
{ "tx", &RPCParser::parseTx, 1, 2 },
{ "tx", &RPCParser::parseTx, 1, 4 },
{ "tx_account", &RPCParser::parseTxAccount, 1, 7 },
{ "tx_history", &RPCParser::parseTxHistory, 1, 1 },
{ "unl_list", &RPCParser::parseAsIs, 0, 0 },
Expand Down
4 changes: 3 additions & 1 deletion src/ripple/protocol/ErrorCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,9 @@ enum error_code_i
rpcNOT_SUPPORTED = 75,
rpcBAD_KEY_TYPE = 76,
rpcDB_DESERIALIZATION = 77,
rpcLAST = rpcDB_DESERIALIZATION // rpcLAST should always equal the last code.
rpcEXCESSIVE_LGR_RANGE = 78,
rpcINVALID_LGR_RANGE = 79,
rpcLAST = rpcINVALID_LGR_RANGE // rpcLAST should always equal the last code.=
};

//------------------------------------------------------------------------------
Expand Down
6 changes: 4 additions & 2 deletions src/ripple/protocol/impl/ErrorCodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,18 @@ constexpr static ErrorInfo unorderedErrorInfos[]
{rpcCHANNEL_MALFORMED, "channelMalformed", "Payment channel is malformed."},
{rpcCHANNEL_AMT_MALFORMED, "channelAmtMalformed", "Payment channel amount is malformed."},
{rpcCOMMAND_MISSING, "commandMissing", "Missing command entry."},
{rpcDB_DESERIALIZATION, "dbDeserialization", "Database deserialization error."},
{rpcDST_ACT_MALFORMED, "dstActMalformed", "Destination account is malformed."},
{rpcDST_ACT_MISSING, "dstActMissing", "Destination account not provided."},
{rpcDST_ACT_NOT_FOUND, "dstActNotFound", "Destination account not found."},
{rpcDST_AMT_MALFORMED, "dstAmtMalformed", "Destination amount/currency/issuer is malformed."},
{rpcDST_AMT_MISSING, "dstAmtMissing", "Destination amount/currency/issuer is missing."},
{rpcDST_ISR_MALFORMED, "dstIsrMalformed", "Destination issuer is malformed."},
{rpcEXCESSIVE_LGR_RANGE, "excessiveLgrRange", "Ledger range exceeds 1000."},
{rpcFORBIDDEN, "forbidden", "Bad credentials."},
{rpcHIGH_FEE, "highFee", "Current transaction fee exceeds your limit."},
{rpcINTERNAL, "internal", "Internal error."},
{rpcINVALID_LGR_RANGE, "invalidLgrRange", "Ledger range is invalid."},
{rpcINVALID_PARAMS, "invalidParams", "Invalid parameters."},
{rpcJSON_RPC, "json_rpc", "JSON-RPC transport error."},
{rpcLGR_IDXS_INVALID, "lgrIdxsInvalid", "Ledger indexes invalid."},
Expand Down Expand Up @@ -87,8 +90,7 @@ constexpr static ErrorInfo unorderedErrorInfos[]
{rpcTOO_BUSY, "tooBusy", "The server is too busy to help you now."},
{rpcTXN_NOT_FOUND, "txnNotFound", "Transaction not found."},
{rpcUNKNOWN_COMMAND, "unknownCmd", "Unknown method."},
{rpcSENDMAX_MALFORMED, "sendMaxMalformed", "SendMax amount malformed."},
{rpcDB_DESERIALIZATION, "dbDeserialization", "Database deserialization error."},
{rpcSENDMAX_MALFORMED, "sendMaxMalformed", "SendMax amount malformed."}
};

// C++ does not allow you to return an array from a function. You must
Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/jss.h
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ JSS ( rt_accounts ); // in: Subscribe, Unsubscribe
JSS ( running_duration_us );
JSS ( sanity ); // out: PeerImp
JSS ( search_depth ); // in: RipplePathFind
JSS ( searched_all ); // out: Tx
JSS ( secret ); // in: TransactionSign,
// ValidationCreate, ValidationSeed,
// channel_authorize
Expand Down
63 changes: 58 additions & 5 deletions src/ripple/rpc/handlers/Tx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

#include <ripple/app/ledger/LedgerMaster.h>
#include <ripple/app/ledger/TransactionMaster.h>
#include <ripple/app/main/Application.h>
#include <ripple/app/misc/NetworkOPs.h>
#include <ripple/app/misc/Transaction.h>
#include <ripple/net/RPCErr.h>
Expand Down Expand Up @@ -97,13 +96,67 @@ 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), ec);
ClosedInterval<uint32_t> range;

if (!txn)
auto rangeProvided = context.params.isMember (jss::min_ledger) &&
context.params.isMember (jss::max_ledger);

if (rangeProvided)
{
try
{
auto const& min = context.params[jss::min_ledger].asUInt ();
auto const& max = context.params[jss::max_ledger].asUInt ();

constexpr uint16_t MAX_RANGE = 1000;

if (max < min)
return rpcError (rpcINVALID_LGR_RANGE);

if (max - min > MAX_RANGE)
return rpcError (rpcEXCESSIVE_LGR_RANGE);

range = ClosedInterval<uint32_t> (min, max);
}
catch (...)
{
// One of the calls to `asUInt ()` failed.
return rpcError (rpcINVALID_LGR_RANGE);
}
}

using pointer = Transaction::pointer;

auto ec {rpcSUCCESS};
pointer txn;

if (rangeProvided)
{
boost::variant<pointer, bool> v =
context.app.getMasterTransaction().fetch(
from_hex_text<uint256>(txid), range, ec);

if (v.which () == 1)
{
auto jvResult = Json::Value (Json::objectValue);

jvResult[jss::searched_all] = boost::get<bool> (v);

return rpcError (rpcTXN_NOT_FOUND, jvResult);
}
else
txn = boost::get<pointer> (v);
}
else
txn = context.app.getMasterTransaction().fetch(
from_hex_text<uint256>(txid), ec);

if (ec == rpcDB_DESERIALIZATION)
return rpcError (ec);

if (!txn)
return rpcError (rpcTXN_NOT_FOUND);

Json::Value ret = txn->getJson (JsonOptions::include_date, binary);

if (txn->getLedger () == 0)
Expand Down
2 changes: 2 additions & 0 deletions src/test/rpc/RPCCall_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6138,6 +6138,8 @@ static RPCCallTestData const rpcCallTestArray [] =
"tx",
"transaction_hash_is_not_validated",
"binary",
"1",
"2",
"extra"
},
RPCCallTestData::no_exception,
Expand Down
Loading

0 comments on commit 47501b7

Please sign in to comment.