Provide additional info with txnNotFound errors.

* 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
This commit is contained in:
Devon White
2019-11-01 13:15:18 -04:00
committed by Nik Bougalis
parent 11cf27e006
commit 47501b7f99
13 changed files with 458 additions and 32 deletions

View File

@@ -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

View File

@@ -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
@@ -45,10 +46,21 @@ public:
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);

View File

@@ -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 {
@@ -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());

View File

@@ -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 {
@@ -63,7 +65,6 @@ public:
using pointer = std::shared_ptr<Transaction>;
using ref = const pointer&;
public:
Transaction (
std::shared_ptr<STTx const> const&, std::string&, Application&) noexcept;
@@ -149,9 +150,18 @@ public:
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;

View File

@@ -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 ("';");
@@ -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

View File

@@ -996,19 +996,26 @@ private:
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;
}
@@ -1182,7 +1189,7 @@ public:
{ "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 },

View File

@@ -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.=
};
//------------------------------------------------------------------------------

View File

@@ -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."},
@@ -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

View File

@@ -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

View File

@@ -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>
@@ -97,12 +96,66 @@ 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;
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 (ec);
return rpcError (rpcTXN_NOT_FOUND);
Json::Value ret = txn->getJson (JsonOptions::include_date, binary);

View File

@@ -6138,6 +6138,8 @@ static RPCCallTestData const rpcCallTestArray [] =
"tx",
"transaction_hash_is_not_validated",
"binary",
"1",
"2",
"extra"
},
RPCCallTestData::no_exception,

View File

@@ -0,0 +1,282 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2012-2017 Ripple Labs Inc.
Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
//==============================================================================
#include <test/jtx.h>
#include <test/jtx/Env.h>
#include <test/jtx/envconfig.h>
#include <ripple/protocol/jss.h>
#include <ripple/core/DatabaseCon.h>
#include <ripple/protocol/ErrorCodes.h>
namespace ripple {
class Transaction_test : public beast::unit_test::suite
{
void
testRangeRequest()
{
testcase("Test Range Request");
using namespace test::jtx;
const char* COMMAND = jss::tx.c_str();
const char* BINARY = jss::binary.c_str();
const char* NOT_FOUND = RPC::get_error_info(rpcTXN_NOT_FOUND).token;
const char* INVALID = RPC::get_error_info(rpcINVALID_LGR_RANGE).token;
const char* EXCESSIVE = RPC::get_error_info(rpcEXCESSIVE_LGR_RANGE).token;
Env env(*this);
auto const alice = Account("alice");
env.fund(XRP(1000), alice);
env.close();
std::vector<std::shared_ptr<STTx const>> txns;
auto const startLegSeq = env.current()->info().seq;
for (int i = 0; i < 750; ++i)
{
env(noop(alice));
txns.emplace_back(env.tx());
env.close();
}
auto const endLegSeq = env.closed()->info().seq;
// Find the existing transactions
for (auto&& tx : txns)
{
auto const result = env.rpc(
COMMAND,
to_string(tx->getTransactionID()),
BINARY,
to_string(startLegSeq),
to_string(endLegSeq));
BEAST_EXPECT(result[jss::result][jss::status] == jss::success);
}
auto const tx = env.jt(noop(alice), seq(env.seq(alice))).stx;
for (int deltaEndSeq = 0; deltaEndSeq < 2; ++deltaEndSeq)
{
auto const result = env.rpc(
COMMAND,
to_string(tx->getTransactionID()),
BINARY,
to_string(startLegSeq),
to_string(endLegSeq + deltaEndSeq));
BEAST_EXPECT(
result[jss::result][jss::status] == jss::error &&
result[jss::result][jss::error] == NOT_FOUND);
if (deltaEndSeq)
BEAST_EXPECT(!result[jss::result][jss::searched_all].asBool());
else
BEAST_EXPECT(result[jss::result][jss::searched_all].asBool());
}
// Find transactions outside of provided range.
for (auto&& tx : txns)
{
auto const result = env.rpc(
COMMAND,
to_string(tx->getTransactionID()),
BINARY,
to_string(endLegSeq + 1),
to_string(endLegSeq + 100));
BEAST_EXPECT(result[jss::result][jss::status] == jss::success);
BEAST_EXPECT(!result[jss::result][jss::searched_all].asBool());
}
const auto deletedLedger = (startLegSeq + endLegSeq) / 2;
{
// Remove one of the ledgers from the database directly
auto db = env.app().getTxnDB().checkoutDb();
*db << "DELETE FROM Transactions WHERE LedgerSeq == "
<< deletedLedger << ";";
}
for (int deltaEndSeq = 0; deltaEndSeq < 2; ++deltaEndSeq)
{
auto const result = env.rpc(
COMMAND,
to_string(tx->getTransactionID()),
BINARY,
to_string(startLegSeq),
to_string(endLegSeq + deltaEndSeq));
BEAST_EXPECT(
result[jss::result][jss::status] == jss::error &&
result[jss::result][jss::error] == NOT_FOUND);
BEAST_EXPECT(!result[jss::result][jss::searched_all].asBool());
}
// Provide range without providing the `binary`
// field. (Tests parameter parsing)
{
auto const result = env.rpc(
COMMAND,
to_string(tx->getTransactionID()),
to_string(startLegSeq),
to_string(endLegSeq));
BEAST_EXPECT(
result[jss::result][jss::status] == jss::error &&
result[jss::result][jss::error] == NOT_FOUND);
BEAST_EXPECT(!result[jss::result][jss::searched_all].asBool());
}
// Provide range without providing the `binary`
// field. (Tests parameter parsing)
{
auto const result = env.rpc(
COMMAND,
to_string(tx->getTransactionID()),
to_string(startLegSeq),
to_string(deletedLedger - 1));
BEAST_EXPECT(
result[jss::result][jss::status] == jss::error &&
result[jss::result][jss::error] == NOT_FOUND);
BEAST_EXPECT(result[jss::result][jss::searched_all].asBool());
}
// Provide range without providing the `binary`
// field. (Tests parameter parsing)
{
auto const result = env.rpc(
COMMAND,
to_string(txns[0]->getTransactionID()),
to_string(startLegSeq),
to_string(deletedLedger - 1));
BEAST_EXPECT(result[jss::result][jss::status] == jss::success);
BEAST_EXPECT(!result[jss::result].isMember(jss::searched_all));
}
// Provide an invalid range: (min > max)
{
auto const result = env.rpc(
COMMAND,
to_string(tx->getTransactionID()),
BINARY,
to_string(deletedLedger - 1),
to_string(startLegSeq));
BEAST_EXPECT(
result[jss::result][jss::status] == jss::error &&
result[jss::result][jss::error] == INVALID);
BEAST_EXPECT(!result[jss::result].isMember(jss::searched_all));
}
// Provide an invalid range: (min < 0)
{
auto const result = env.rpc(
COMMAND,
to_string(tx->getTransactionID()),
BINARY,
to_string(-1),
to_string(deletedLedger - 1));
BEAST_EXPECT(
result[jss::result][jss::status] == jss::error &&
result[jss::result][jss::error] == INVALID);
BEAST_EXPECT(!result[jss::result].isMember(jss::searched_all));
}
// Provide an invalid range: (min < 0, max < 0)
{
auto const result = env.rpc(
COMMAND,
to_string(tx->getTransactionID()),
BINARY,
to_string(-20),
to_string(-10));
BEAST_EXPECT(
result[jss::result][jss::status] == jss::error &&
result[jss::result][jss::error] == INVALID);
BEAST_EXPECT(!result[jss::result].isMember(jss::searched_all));
}
// Provide an invalid range: (only one value)
{
auto const result = env.rpc(
COMMAND,
to_string(tx->getTransactionID()),
BINARY,
to_string(20));
BEAST_EXPECT(
result[jss::result][jss::status] == jss::error &&
result[jss::result][jss::error] == INVALID);
BEAST_EXPECT(!result[jss::result].isMember(jss::searched_all));
}
// Provide an invalid range: (only one value)
{
auto const result = env.rpc(
COMMAND,
to_string(tx->getTransactionID()),
to_string(20));
// Since we only provided one value for the range,
// the interface parses it as a false binary flag,
// as single-value ranges are not accepted. Since
// the error this causes differs depending on the platform
// we don't call out a specific error here.
BEAST_EXPECT(result[jss::result][jss::status] == jss::error);
BEAST_EXPECT(!result[jss::result].isMember(jss::searched_all));
}
// Provide an invalid range: (max - min > 1000)
{
auto const result = env.rpc(
COMMAND,
to_string(tx->getTransactionID()),
BINARY,
to_string(startLegSeq),
to_string(startLegSeq + 1001));
BEAST_EXPECT(
result[jss::result][jss::status] == jss::error &&
result[jss::result][jss::error] == EXCESSIVE);
BEAST_EXPECT(!result[jss::result].isMember(jss::searched_all));
}
}
public:
void run () override
{
testRangeRequest();
}
};
BEAST_DEFINE_TESTSUITE (Transaction, rpc, ripple);
} // ripple

View File

@@ -49,6 +49,7 @@
#include <test/rpc/ServerInfo_test.cpp>
#include <test/rpc/Status_test.cpp>
#include <test/rpc/Subscribe_test.cpp>
#include <test/rpc/Transaction_test.cpp>
#include <test/rpc/TransactionEntry_test.cpp>
#include <test/rpc/TransactionHistory_test.cpp>
#include <test/rpc/ValidatorRPC_test.cpp>