ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-04 19:25:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improved error message on mistyped command [RIPD-1527]:

Browse Source 
Previously if you mistyped the "submit_multisigned" command as
"submit_multisign", the returned message was "Internal error".  Not
very helpful.  It turns out this was caused by a small amount of
code in RPCCall.cpp.  Removing that code improves two situations:

1. It improves the situation with a mistyped command.  Now the
   command returns "Unknown method" and provides the string of
   the mistyped command.

2. The "transaction_entry", if properly entered in its command
   line form, would fire an assert.  That assert is now removed.

In the process, it was discovered that the command line form of
the "transaction_entry" command has not worked correctly for at
least a year.  Therefore support for that the command line form
of "transaction_entry" is added along with appropriate unit
tests.
This commit is contained in:
Scott Schurr
2017-11-22 18:39:08 -08:00
committed by seelabs
parent 8a02b76d73
commit 722917e9a6
3 changed files with 139 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
src/ripple/net/impl/RPCCall.cpp
View File

@@ -879,6 +879,31 @@ private:
return rpcError (rpcINVALID_PARAMS);
}
// transaction_entry <tx_hash> <ledger_hash/ledger_index>
Json::Value parseTransactionEntry (Json::Value const& jvParams)
{
// Parameter count should have already been verified.
assert (jvParams.size() == 2);
std::string const txHash = jvParams[0u].asString();
if (txHash.length() != 64)
return rpcError (rpcINVALID_PARAMS);
Json::Value jvRequest;
jvRequest[jss::tx_hash] = txHash;
jvParseLedger (jvRequest, jvParams[1u].asString());
// jvParseLedger inserts a "ledger_index" of 0 if it doesn't
// find a match.
if (jvRequest.isMember(jss::ledger_index) &&
jvRequest[jss::ledger_index] == 0)
return rpcError (rpcINVALID_PARAMS);
return jvRequest;
}
// tx <transaction_id>
Json::Value parseTx (Json::Value const& jvParams)
{
@@ -1073,7 +1098,7 @@ public:
{ "server_info", &RPCParser::parseAsIs, 0, 0 },
{ "server_state", &RPCParser::parseAsIs, 0, 0 },
{ "stop", &RPCParser::parseAsIs, 0, 0 },
// { "transaction_entry", &RPCParser::parseTransactionEntry, -1, -1 },
{ "transaction_entry", &RPCParser::parseTransactionEntry, 2, 2 },
{ "tx", &RPCParser::parseTx, 1, 2 },
{ "tx_account", &RPCParser::parseTxAccount, 1, 7 },
{ "tx_history", &RPCParser::parseTxHistory, 1, 1 },
@@ -1323,12 +1348,6 @@ rpcClient(std::vector<std::string> const& args,
jvParams.append(jvRequest[i]);
}
if (jvRequest.isMember(jss::params))
{
auto const& params = jvRequest[jss::params];
assert(params.size() == 1);
jvParams.append(params[0u]);
}
{
boost::asio::io_service isService;
RPCCall::fromNetwork (

12
src/test/rpc/JSONRPC_test.cpp
View File

@@ -1840,6 +1840,17 @@ R"({
class JSONRPC_test : public beast::unit_test::suite
{
public:
void testBadRpcCommand ()
{
test::jtx::Env env(*this);
Json::Value const result {
env.rpc ("bad_command", R"({"MakingThisUp": 0})")};
BEAST_EXPECT (result[jss::result][jss::error] == "unknownCmd");
BEAST_EXPECT (
result[jss::result][jss::request][jss::command] == "bad_command");
}
void testAutoFillFees ()
{
test::jtx::Env env(*this);
@@ -2345,6 +2356,7 @@ public:
void run ()
{
testBadRpcCommand ();
testAutoFillFees ();
testAutoFillEscalatedFees ();
testTransactionRPC ();

113
src/test/rpc/TransactionEntry_test.cpp
View File

@@ -69,6 +69,76 @@ class TransactionEntry_test : public beast::unit_test::suite
BEAST_EXPECT(result[jss::error] == "transactionNotFound");
BEAST_EXPECT(result[jss::status] == "error");
}
std::string const txHash {
"E2FE8D4AF3FCC3944DDF6CD8CDDC5E3F0AD50863EF8919AFEF10CB6408CD4D05"};
// Command line format
{
// No arguments
Json::Value const result {env.rpc ("transaction_entry")};
BEAST_EXPECT(result[jss::ledger_hash].asString().empty());
BEAST_EXPECT(result[jss::error] == "badSyntax");
BEAST_EXPECT(result[jss::status] == "error");
}
{
// One argument
Json::Value const result {env.rpc ("transaction_entry", txHash)};
BEAST_EXPECT(result[jss::error] == "badSyntax");
BEAST_EXPECT(result[jss::status] == "error");
}
{
// First argument with too few characters
Json::Value const result {env.rpc (
"transaction_entry", txHash.substr (1), "closed")};
BEAST_EXPECT(result[jss::error] == "invalidParams");
BEAST_EXPECT(result[jss::status] == "error");
}
{
// First argument with too many characters
Json::Value const result {env.rpc (
"transaction_entry", txHash + "A", "closed")};
BEAST_EXPECT(result[jss::error] == "invalidParams");
BEAST_EXPECT(result[jss::status] == "error");
}
{
// Second argument not valid
Json::Value const result {env.rpc (
"transaction_entry", txHash, "closer")};
BEAST_EXPECT(result[jss::error] == "invalidParams");
BEAST_EXPECT(result[jss::status] == "error");
}
{
// Ledger index of 0 is not valid
Json::Value const result {env.rpc (
"transaction_entry", txHash, "0")};
BEAST_EXPECT(result[jss::error] == "invalidParams");
BEAST_EXPECT(result[jss::status] == "error");
}
{
// Three arguments
Json::Value const result {env.rpc (
"transaction_entry", txHash, "closed", "extra")};
BEAST_EXPECT(result[jss::error] == "badSyntax");
BEAST_EXPECT(result[jss::status] == "error");
}
{
// Valid structure, but transaction not found.
Json::Value const result {env.rpc (
"transaction_entry", txHash, "closed")};
BEAST_EXPECT(
! result[jss::result][jss::ledger_hash].asString().empty());
BEAST_EXPECT(
result[jss::result][jss::error] == "transactionNotFound");
BEAST_EXPECT(result[jss::result][jss::status] == "error");
}
}
void testRequest()
@@ -78,24 +148,28 @@ class TransactionEntry_test : public beast::unit_test::suite
Env env {*this};
auto check_tx = [this, &env]
(int index, std::string txhash, std::string type = "")
(int index, std::string const txhash, std::string const type = "")
{
Json::Value resIndex, resHash;
// first request using ledger_index to lookup
Json::Value const resIndex {[&env, index, &txhash] ()
{
Json::Value params {Json::objectValue};
params[jss::ledger_index] = index;
params[jss::tx_hash] = txhash;
resIndex = env.client()
return env.client()
.invoke("transaction_entry", params)[jss::result];
if(! BEAST_EXPECTS(resIndex.isMember(jss::tx_json), txhash))
return;
BEAST_EXPECT(resIndex[jss::tx_json][jss::hash] == txhash);
if(! type.empty())
BEAST_EXPECTS(
resIndex[jss::tx_json][jss::TransactionType] == type,
txhash + " is " +
resIndex[jss::tx_json][jss::TransactionType].asString());
}()};
if(! BEAST_EXPECTS(resIndex.isMember(jss::tx_json), txhash))
return;
BEAST_EXPECT(resIndex[jss::tx_json][jss::hash] == txhash);
if(! type.empty())
{
BEAST_EXPECTS(
resIndex[jss::tx_json][jss::TransactionType] == type,
txhash + " is " +
resIndex[jss::tx_json][jss::TransactionType].asString());
}
// second request using ledger_hash to lookup and verify
@@ -104,10 +178,25 @@ class TransactionEntry_test : public beast::unit_test::suite
Json::Value params {Json::objectValue};
params[jss::ledger_hash] = resIndex[jss::ledger_hash];
params[jss::tx_hash] = txhash;
resHash = env.client()
Json::Value const resHash = env.client()
.invoke("transaction_entry", params)[jss::result];
BEAST_EXPECT(resHash == resIndex);
}
// Use the command line form with the index.
{
Json::Value const clIndex {env.rpc (
"transaction_entry", txhash, std::to_string (index))};
BEAST_EXPECT (clIndex["result"] == resIndex);
}
// Use the command line form with the ledger_hash.
{
Json::Value const clHash {env.rpc (
"transaction_entry", txhash,
resIndex[jss::ledger_hash].asString())};
BEAST_EXPECT (clHash["result"] == resIndex);
}
};
Account A1 {"A1"};