diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 5a65d2d4cbd..a6d97ddf888 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -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 diff --git a/src/ripple/app/ledger/TransactionMaster.h b/src/ripple/app/ledger/TransactionMaster.h index f19ecd55fa4..5357aac6c42 100644 --- a/src/ripple/app/ledger/TransactionMaster.h +++ b/src/ripple/app/ledger/TransactionMaster.h @@ -23,11 +23,12 @@ #include #include #include +#include +#include namespace ripple { class Application; -class Transaction; class STTx; // Tracks all transactions in memory @@ -45,10 +46,21 @@ class TransactionMaster std::shared_ptr 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 + fetch (uint256 const& , ClosedInterval const& range, error_code_i& ec); + std::shared_ptr fetch (std::shared_ptr 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); diff --git a/src/ripple/app/ledger/impl/TransactionMaster.cpp b/src/ripple/app/ledger/impl/TransactionMaster.cpp index 3d15658da5b..d6f7ab425b4 100644 --- a/src/ripple/app/ledger/impl/TransactionMaster.cpp +++ b/src/ripple/app/ledger/impl/TransactionMaster.cpp @@ -21,7 +21,6 @@ #include #include #include -#include #include namespace ripple { @@ -68,10 +67,29 @@ TransactionMaster::fetch (uint256 const& txnID, error_code_i& ec) return txn; } +boost::variant +TransactionMaster::fetch (uint256 const& txnID, ClosedInterval const& range, + error_code_i& ec) +{ + using pointer = Transaction::pointer; + + auto txn = mCache.fetch (txnID); + + if (txn) + return txn; + + boost::variant v = Transaction::load ( + txnID, mApp, range, ec); + + if (v.which () == 0 && boost::get (v)) + mCache.canonicalize (txnID, boost::get (v)); + + return v; +} + std::shared_ptr TransactionMaster::fetch (std::shared_ptr const& item, - SHAMapTreeNode::TNType type, - bool checkDisk, std::uint32_t uCommitLedger) + SHAMapTreeNode::TNType type, std::uint32_t uCommitLedger) { std::shared_ptr txn; auto iTx = fetch_from_cache (item->key()); diff --git a/src/ripple/app/misc/Transaction.h b/src/ripple/app/misc/Transaction.h index 695e3c51a01..1e180f5962c 100644 --- a/src/ripple/app/misc/Transaction.h +++ b/src/ripple/app/misc/Transaction.h @@ -25,7 +25,9 @@ #include #include #include +#include #include +#include namespace ripple { @@ -63,7 +65,6 @@ class Transaction using pointer = std::shared_ptr; using ref = const pointer&; -public: Transaction ( std::shared_ptr const&, std::string&, Application&) noexcept; @@ -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 + load (uint256 const& id, Application& app, ClosedInterval const& range, error_code_i& ec); private: + + static boost::variant + load (uint256 const& id, Application& app, boost::optional> const& range, + error_code_i& ec); + uint256 mTransactionID; LedgerIndex mInLedger = 0; diff --git a/src/ripple/app/misc/impl/Transaction.cpp b/src/ripple/app/misc/impl/Transaction.cpp index bc7b6f94e89..4ba0b3b2b10 100644 --- a/src/ripple/app/misc/impl/Transaction.cpp +++ b/src/ripple/app/misc/impl/Transaction.cpp @@ -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 (load (id, app, boost::none, ec)); +} + +boost::variant +Transaction::load (uint256 const& id, Application& app, ClosedInterval const& range, + error_code_i& ec) +{ + using op = boost::optional>; + + return load (id, app, op {range}, ec); +} + +boost::variant +Transaction::load (uint256 const& id, Application& app, boost::optional> 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 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 diff --git a/src/ripple/net/impl/RPCCall.cpp b/src/ripple/net/impl/RPCCall.cpp index a010791614d..da4e2caa43b 100644 --- a/src/ripple/net/impl/RPCCall.cpp +++ b/src/ripple/net/impl/RPCCall.cpp @@ -996,19 +996,26 @@ class RPCParser return jvRequest; } - // tx 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 @@ 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 }, diff --git a/src/ripple/protocol/ErrorCodes.h b/src/ripple/protocol/ErrorCodes.h index 2c3be54edcb..c6c2f1d0209 100644 --- a/src/ripple/protocol/ErrorCodes.h +++ b/src/ripple/protocol/ErrorCodes.h @@ -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.= }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/impl/ErrorCodes.cpp b/src/ripple/protocol/impl/ErrorCodes.cpp index 7da60b332c8..c58ef9962d0 100644 --- a/src/ripple/protocol/impl/ErrorCodes.cpp +++ b/src/ripple/protocol/impl/ErrorCodes.cpp @@ -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 diff --git a/src/ripple/protocol/jss.h b/src/ripple/protocol/jss.h index a6137027051..31c7dd70501 100644 --- a/src/ripple/protocol/jss.h +++ b/src/ripple/protocol/jss.h @@ -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 diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index e2344b51d6b..c5e302fcf72 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/src/ripple/rpc/handlers/Tx.cpp @@ -19,7 +19,6 @@ #include #include -#include #include #include #include @@ -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(txid), ec); + ClosedInterval 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 (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 v = + context.app.getMasterTransaction().fetch( + from_hex_text(txid), range, ec); + + if (v.which () == 1) + { + auto jvResult = Json::Value (Json::objectValue); + + jvResult[jss::searched_all] = boost::get (v); + + return rpcError (rpcTXN_NOT_FOUND, jvResult); + } + else + txn = boost::get (v); + } + else + txn = context.app.getMasterTransaction().fetch( + from_hex_text(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) diff --git a/src/test/rpc/RPCCall_test.cpp b/src/test/rpc/RPCCall_test.cpp index 05c9a23bb88..66de1670443 100644 --- a/src/test/rpc/RPCCall_test.cpp +++ b/src/test/rpc/RPCCall_test.cpp @@ -6138,6 +6138,8 @@ static RPCCallTestData const rpcCallTestArray [] = "tx", "transaction_hash_is_not_validated", "binary", + "1", + "2", "extra" }, RPCCallTestData::no_exception, diff --git a/src/test/rpc/Transaction_test.cpp b/src/test/rpc/Transaction_test.cpp new file mode 100644 index 00000000000..de7b167cffd --- /dev/null +++ b/src/test/rpc/Transaction_test.cpp @@ -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 +#include +#include +#include +#include +#include + +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> 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 diff --git a/src/test/unity/rpc_test_unity.cpp b/src/test/unity/rpc_test_unity.cpp index a3bc197c613..90d9f6ea571 100644 --- a/src/test/unity/rpc_test_unity.cpp +++ b/src/test/unity/rpc_test_unity.cpp @@ -49,6 +49,7 @@ #include #include #include +#include #include #include #include